8000 Local dev props by tmat · Pull Request #48739 · dotnet/sdk · GitHub
[go: up one dir, main page]

Skip to content

Local dev props #48739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Local dev props #48739

wants to merge 1 commit into from

Conversation

tmat
Copy link
Member
@tmat tmat commented Apr 27, 2025

Adds infrastructure that makes it easier to consume locally built Roslyn packages.

Adding LocalDev.Build.props and LocalDev.Packages.props to the repo root redirects package restore to .packages dir and sets version of Roslyn packages (or any other package) to locally built one.

Based on @sharwell's work in another repository.

@Copilot Copilot AI review requested due to automatic review settings April 27, 2025 16:30
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds documentation to simplify the process of consuming locally built Roslyn packages.

  • Provides step-by-step instructions for checking out the Roslyn repository, copying necessary property files, building Roslyn, and restoring packages.
  • Introduces a tip on speeding up package restore by clearing the cache and selectively applying local development settings.
Files not reviewed (4)
  • Directory.Build.props: Language not supported
  • Directory.Packages.props: Language not supported
  • documentation/local-dev/roslyn/LocalDev.Build.props: Language not supported
  • documentation/local-dev/roslyn/LocalDev.Packages.props: Language not supported

<!-- If \LocalDev.Packages.props exists, it will be loaded and merged with Directory.Packages.props, allowing for local adhoc build customizations. -->
<PropertyGroup>
<!-- Use the local version of Roslyn -->
<RoslynVersion>4.14.0-dev</RoslynVersion>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to update these versions?

Copy link
Member Author
@tmat tmat Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How? These are locally built versions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. I was thinking that if it's built locally, we could perhaps overwrite something in this file as part of the build process? That only really works if it's in a fixed location and we just conditionally import it rather than having it normally live in documentation as here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially fish out the version from the other repo, using RoslynRoot set in LocalDev.Build.props. Setting RoslynVersion in LocalDev.Packages.props is easy enough, one time step though.


<!-- Import any local dev packages if they exist. -->
<PropertyGroup>
<LocalDevPackagesPath>$(MSBuildThisFileDirectory)LocalDev.Packages.props</LocalDevPackagesPath>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems weird to me to have LocalDev.Packages.props and LocalDev.Build.props start in 'documentation' (which shouldn't have code, in my opinion) and need someone to copy them to the root. Perhaps we could have both always in the root (or, better yet, root/LocalDevTools or just root/eng) and import them conditionally if -p:SomeProperty=true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They need to be available when building from VS.
Also, these are just templates. They need to be updated to point to the local Roslyn repo and set the local version. That information is specific to the dev machine.


4. Build Roslyn with:

`..\roslyn\Build.cmd -restore -pack`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a simpler approach (which is what I do currently and it works great):

  1. Build Roslyn: .\build.cmd -pack /p:VersionSuffix=$(git rev-parse --short HEAD)
  2. In SDK's Versions.props: find+replace (simple VSCode action) current roslyn version with the built version
  3. Restore/build SDK.

That's it. No need to clean .packages. No need to keep the list of Version properties in LocalDev.Packages.props in sync with Versions.props (if we add more roslyn dependencies over time). Works in other repos too (like runtime or razor).

Copy link
Member Author
@tmat tmat Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not simpler. I don't need to build and pack the entire Roslyn from command line, which takes a long time. I can pack just one project from VS. I don't need to update version every time I make a change. I only need to delete Roslyn packages from .packages dir and restore. Ideally, I would not want to do that either. I wonder if we can set the VersionSuffix automatically to what you do when building locally. Then in LocalDev.Packages.props we would use x.y.z-dev-* pattern.

No need to keep the list of Version properties in LocalDev.Packages.props

If you update the version in Versions.props you need to revert that change before committing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't need to build and pack the entire Roslyn from command line, which takes a long time. I can pack just one project from VS.

I see, the doc says build.cmd -pack so I assumed that's what you are doing.

I don't need to update version every time I make a change. I only need to delete Roslyn packages from .packages dir and restore.

Both are a single step, I don't think one is more difficult than the other.

But it's true that re-building just a single package with my approach isn't feasible. Your approach looks better in this scenario. Thanks for explaining.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue with using $(git rev-parse --short HEAD) is that you need to remember to commit each time you want to switch to the other repo.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think setting VersionSuffix to current time-based value would make it simpler still. Might explore that later on.


5. Clear the customized packages cache in `.packages`

`git clean -dxf .packages/`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using git clean and not simply rm -r?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way works. I just delete the directory from file explorer. @sharwell authored the original doc.

@marcpopMSFT
Copy link
Member

SDK Triage: This doesn't impact us and we can't think of a good reason not to have it. I'm not sure how many folks outside of tmat will ultimately use this but there's limited harm in having it in. We discussed if the files should be in the doc folder or not but it's better to be it next to the readme than elsewhere so this is fine. So once y 8000 ou have signoff, you're clear to merge from our team.

@marcpopMSFT marcpopMSFT self-requested a review May 20, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0