Skip to content
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.

Sign up for GitHub

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

Jump to bottom

RepositoryBranch logic conflicts with .NET 9 SDK #46

Closed
MattKotsenas opened this issue Aug 21, 2024 · 1 comment · Fixed by #50
Closed

RepositoryBranch logic conflicts with .NET 9 SDK #46

MattKotsenas opened this issue Aug 21, 2024 · 1 comment · Fixed by #50
Assignees
@MattKotsenas

Comments

@MattKotsenas
Copy link
Member

We have custom logic to set the repository branch name based on CI provider environment variables (see

reproducible-builds/src/DotNet.ReproducibleBuilds/DotNet.ReproducibleBuilds.targets

Line 16 in 97f3184

<PropertyGroup Condition="'$(RepositoryBranch)' == '' and '$(PublishRepositoryUrl)' == 'true'">
)

However, our logic strips the refs/heads/ and refs/tags/ from the branch name, which conflicts with NuGet/NuGet.Client#5923 available in the .NET 9 SDK, which gives the full ref name.

I think we should update our logic to also use the full ref name:

  1. for consistency with the SDK
  2. for reproducibility (so packages are tagged the same locally and in CI)
@MattKotsenas MattKotsenas self-assigned this Aug 21, 2024
@baronfel
Copy link
Member

We dug into this a bit and we should make this change. There are some users who are explicitly setting values like main, master, or using tools like GitVersion to get the repository branch - .NET 9's changes won't overwrite these values, and we should have a goal in this repo of upstreaming things that prove their value in this repo. This is a success story!

@MattKotsenas MattKotsenas mentioned this issue Aug 27, 2024
Merged
baronfel added a commit that referenced this issue Aug 27, 2024
@baronfel
Add tests for package (#47)
e0ee68a 
In preparation for #46, add some test to validate the current package
behavior. This doesn't test everything; future PRs should ensure the
code they're changing is tested.

The test flow is as follows:

1. Include `DotNet.ReproducibleBuilds`'s own .props and .targets files
into the test project's output
2. Create a temp directory
3. Create a `{project}.*.props` and `{project}.*.targets` file in the
`obj/` folder so that the .NET SDK will import the props and targets at
the correct time (this is basically to do a NuGet restore of the project
without needing to create the package or deal with package resolution)
4. Create a temp project and assert the behavior
@MattKotsenas MattKotsenas mentioned this issue Aug 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MattKotsenas MattKotsenas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Align RepositoryBranch logic with .NET 9 MattKotsenas/reproducible-builds
2 participants
@MattKotsenas @baronfel