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.

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

dotnet build /p:ContinuousIntegrationBuild=true doesn't work #16325

Open
PureKrome opened this issue Mar 12, 2021 · 45 comments
Open

dotnet build /p:ContinuousIntegrationBuild=true doesn't work #16325

PureKrome opened this issue Mar 12, 2021 · 45 comments
Milestone

Comments

@PureKrome
Copy link

PureKrome commented Mar 12, 2021

(I hope this is the correct place to raise this question/issue. apologies if it isn't)

Issue

Trying to building a class library on a CI/CD server that is deterministic using the ContinuousIntegrationBuild=true property argument on the cli doesn't seem to actually do this?

Assumption:

  • dotnet build should allow this msbuild property
  • I can build/confirm this on my localhost development machine for testing, before it is 'pushed up' to a CI server.

Steps to repo.

- mkdir deterministic-nuget
- cd deterministic-nuget
- dotnet new classlib
- dotnet build -c RELEASE /p:ContinuousIntegrationBuild=true -p:version=1.2.3-pre.1
- dotnet pack -c RELEASE --no-build -p:packageVersion=1.2.3-pre.1 -o C:\temp\nuget-testing\

now goto C:\temp\nuget-testing and double click on the .nupkg file that is there. NuGet Package Explorer opens up and this is what it looks like:

image

I'm not sure if this is an issue with NuGet or dotnet build CLI or NGPE?

Info

  • On my own windows 10 dev machine.
❯ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.201
 Commit:    a09bd5c86c

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19042
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.201\

Host (useful for support):
  Version: 5.0.4
  Commit:  f27d337295

.NET SDKs installed:
  2.2.207 [C:\Program Files\dotnet\sdk]
  3.1.406 [C:\Program Files\dotnet\sdk]
  5.0.100 [C:\Program Files\dotnet\sdk]
  5.0.201 [C:\Program Files\dotnet\sdk]
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Mar 12, 2021
@sfoslund sfoslund added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Mar 15, 2021
@sfoslund sfoslund removed their assignment Mar 15, 2021
@marcpopMSFT
Copy link
Member

@jaredpar Do you know about ContinuousIntegrationBuild or who we should bring in to take a look?

@jaredpar
Copy link
Member

I'm not seeing this behavior locally using 5.0.102. I added a -bl option at the dotnet build command in the repro and verified that Deterministic is being passed to the compiler

image

@PureKrome
Copy link
Author

@jaredpar so the compiler is doing the correct thing. It's possible that N.P.E is doing the wrong thing.

Is there a way I can locally check that a dll which I've built with the /p:ContinuousIntegrationBuild=true flag, shows deterministic = true, like that screenshot above?

@jaredpar
Copy link
Member

@PureKrome recent versions of ILSpy will display whether or not the binary was built deterministically. You can also look at the binary log to see if it's being passed to the compiler.

@PureKrome
Copy link
Author

Cheers @jaredpar - time to google what all that means / how to do all of that :)

@marcpopMSFT marcpopMSFT removed the needs team triage Requires a full team discussion label Apr 7, 2021
@marcpopMSFT marcpopMSFT added this to the Discussion milestone Apr 7, 2021
@dominikjeske
Copy link

I also have same issue - I'm using ContinuousIntegrationBuild but NGPE is showing that package is not deterministic. I have checked IL Spy and indeed it says: "This assembly was compiled using the /deterministic option." so maybe it is a bug in NGPE

@SteveDesmond-ca
Copy link
Contributor

SteveDesmond-ca commented Sep 9, 2021

I am currently fighting with this, specifically with GitHub Actions not respecting the -p:ContinuousIntegrationBuild=true flag/argument on dotnet pack.

Debugging into both ILSpy and NGPE, I can see they're looking at different things:

  • ILSpy is looking for any debug directory of type DebugDirectoryEntryType.Reproducible
  • NGPE is confirming all source paths in the PDB start with /_

@SteveDesmond-ca
Copy link
Contributor

SteveDesmond-ca commented Sep 9, 2021

I've narrowed this down to "seemingly nondeterministic behavior" with dotnet pack and when it decides it needs to rebuild the project. If I run dotnet test beforehand without the -p flag, and run dotnet pack afterwards with the -p flag, sometimes the NuGet package will be deterministic, and sometimes it won't.

My workaround for now will be to have the workflow dotnet test with the default Debug config, and dotnet pack with the Release config, but I really think the proper default behavior for dotnet pack should be to always build from scratch if the CI flag is set -- that's literally the designed purpose of it: create deterministic builds.

@PureKrome
Copy link
Author

@SteveDesmond-ca - mate. REALLY appreciate your work/effort on debugging this issue.

I love things right .. so this has been killing me for ages :(

My workaround for now will be to have the workflow dotnet test with the default Debug config, and dotnet pack with the Release config,

Interesting you're saying this. I've been sorta already doing this.

I actually have a CI "matrix" where it does CI in -both- DEBUG and RELEASE modes.

  • For DEBUG, it only does the TESTS. (I don't do any #if RELEASE code .. so this isn't an issue for me)
  • For RELEASE it only does the pack / deploy

this way, the 'CI RUN' will always have the one binary everything is worked against. So if this is the 'DEBUG CI run', then it does most steps against the debug version of the run.

versus

a single run which does different CONFIGURATION builds during it.

Now back to what you said:

and dotnet pack with the Release config

I thought was doing that above ... and it wasn't working :( I thought that the RELEASE config is used for the BUILD step. So if we do dotnet pack and the compiler says to itself "oh wait, i can't find any dlls, so I need to actually, secretly do a dotnet build, first .. then it will do that with whatever configuration variable was provided.

so with my example above, i thought:

  • dotnet build -c RELEASE ... => builds the project in RELEASE mode configuration.
  • dotnet pack _but no -c RELEASE param was provided => the dll's are already provided so if a -c XXX was provided it's ignored.

so i'm all confused :(

@DamianEdwards
Copy link
Member

DamianEdwards commented Sep 10, 2021

I've just hit this too on https://github.com/DamianEdwards/MinimalValidation. I haven't been able to workaround it by being explicit when calling dotnet pack either.

@adadurov
Copy link

adadurov commented Nov 29, 2021

I noticed that paths were getting 'normalized' when I build a project with Microsoft.SourceLink.GitLab package installed and as soon as I uninstalled SourceLink, ContinuousIntegrationBuild=true has no effect.
Didn't have time to attempt debugging.

@manuelbl
Copy link

manuelbl commented Dec 9, 2021

Added "Microsoft.SourceLink.GitHub" dependency, and it works...

@curt-w
Copy link

curt-w commented Apr 1, 2022

I've narrowed this down to "seemingly nondeterministic behavior" with dotnet pack and when it decides it needs to rebuild the project. If I run dotnet test beforehand without the -p flag, and run dotnet pack afterwards with the -p flag, sometimes the NuGet package will be deterministic, and sometimes it won't.

My workaround for now will be to have the workflow dotnet test with the default Debug config, and dotnet pack with the Release config, but I really think the proper default behavior for dotnet pack should be to always build from scratch if the CI flag is set -- that's literally the designed purpose of it: create deterministic builds.

I'm discovering the same, unfortunately I found this after hours of troubleshooting / coming to the conclusion.

My pipeline consists of 1) build (/p:ContinuousIntegrationBuild=true), 2) test, 3) pack (--no-build); I added --no-build arguments to the test step which seems to have resolved my issue; the test step must have been affecting the results of step 1 / build.

@Suchiman
Copy link

Suchiman commented Sep 9, 2022

I've added /p:ContinuousIntegrationBuild=true to my build and didn't see any changes.
Investigating the compiler command line i've noticed that while /deterministic+ was already being passed (default in SDK style projects AFAIK), adding ContinuousIntegrationBuild resulted in this additional flag being passed

/pathmap:"C:\Users\TFS\.nuget\packages\=/_/,C:\Program Files\dotnet\sdk\NuGetFallbackFolder\=/_1/,"

I would have expected to find C:\Builds\Agent_Jones\_work\35\s in there and not nuget paths.

@KalleOlaviNiemitalo
Copy link

nuget paths in pathmap make the result more reproducible by hiding the C:\Users\TFS path from debug info so that the result can be the same even if built with a different user account.

@Suchiman
Copy link

Suchiman commented Sep 9, 2022

Ok so after some more tinkering, adding an explicit /p:Deterministic to the build, the build outputted the following error:

error : SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true

I then had to drop a Directory.Build.props into the root of my repository with the following contents

<Project>
  <ItemGroup>
    <SourceRoot Include="$(MSBuildThisFileDirectory)"/>
  </ItemGroup>
</Project>

and that + /p:ContinuousIntegrationBuild=true /p:Deterministic made it work.

EDIT:
Welp, except i spoke too soon, the command line to csc now contains

/pathmap:"C:\Builds\Agent_Jones\_work\35\s\/=/_/,C:\Users\TFS\.nuget\packages\=/_1/,C:\Program Files\dotnet\sdk\NuGetFallbackFolder\=/_2/,"

so that looks good but stacktraces still start with C:\Builds\Agent_Jones\_work\35\s\... sigh a problem for after the weekend

EDIT 2:
Removed trailing / from the SourceRoot include which fixed it

@MihaMarkic
Copy link

Looks like the issue is still with us.

@cremor
Copy link

cremor commented Jan 24, 2023

See also dotnet/roslyn#55860

@iBrotNano
Copy link

I could observe that a lot of reasons are showing this behaviour. In my case a referencing NuGet has a build.props file by itself. My own configuration got overwritten by it. Analyzing the logs was a useful hind for me. Thanks to @jaredpar . dotnet build -bl generates it and with https://msbuildlog.com/#:~:text=Double-click%20the%20.binlog%20file%20to%20open%20it%20in,log%20of%20any%20verbosity%20given%20the%20.binlog%20file. you can open it. I searched for Deterministic = and found this line Property reassignment: $(Deterministic)="false" (previous value: "True") at C:\Users\iBrotNano\.nuget\packages\shouldly\4.1.0\build\Shouldly.props (7,5).

@TomJMaher
Copy link

Interesting @TomJMaher ! If you notice in my opening post, I have something very similar:

- dotnet build -c RELEASE /p:ContinuousIntegrationBuild=true -p:version=1.2.3-pre.1
- dotnet pack --no-build -p:packageVersion=1.2.3-pre.1 -o C:\temp\nuget-testing\

The difference being, I'm doing pack and you're doing publish.

Not sure if this is related or not? Would love to see what the MS team think ...

Yes it was actually that, that made me realise I'd missed off the --no-build option from the Publish. I'd got it on the Test but forgot to include it on the Publish.

@TomJMaher
Copy link

I'm pretty sure the commands are doing as told. I mentioned the same thing here #16325 (comment) that adding "no-build" type flags to every dotnet step in my pipeline resolved the issues.

Basically I do an explicit restore, then an explicit build, then all my test pack publish (all other dotnet commands) explicitly state "no restore" and "no build" so those operations don't change the deterministic build I created in the first step.

I've gotten all my libraries publishing deterministic with source link and everything working - opening the Nuget in package explorer shows a healthy package with all green checkmarks for the deterministic, source link, etc. So all in all it does work / is achievable.

From one perspective it would be better if each dotnet command (restore, build, test, publish, pack et al) did just that, one atomic action. However from an ease of use stand point I can see why running dotnet test would also restore and build. After all every time a user wants to mod their code and retest it would be a pain to run 3 separate commands vs just 1. Plus the use cases of the more verbose variants of the commands with the additional options are probably more likely for automation where someone isn't having to manually type them out each time.

I think it's a fine balance and despite it costing me a couple of hours in trouble shooting I would say that they've got it the right way round.

@PureKrome
Copy link
Author

@TomJMaher thanks for the input. I agree heaps with your thoughts, also.

But was is weird (frustrating) is that .. when I manually say --no build ... it's not working .. when it should be working.

By that, I mean NGPE still reports the package with an ❌ for deterministic. But based on what we're all saying (using the --no-build for the steps after dotnet build) .. it's not doing that.

@cremor
Copy link

cremor commented Mar 19, 2023

Have you seen (and tried) the comment above about <SourceRoot>?
See also dotnet/roslyn#55860

@PureKrome
Copy link
Author

Have you seen (and tried) #16325 (comment) about ?

Nope .. because I feel like doing changes like that are undocumented hacks, right now. Either the concept/documentation is wrong or there's a potential bug.

@cremor
Copy link

cremor commented Mar 19, 2023

Yes, there is a bug. That's what dotnet/roslyn#55860 is about.
So you can either wait until this bug is fixed, or use <SourceRoot> as a workaround.

@stijnherreman
Copy link

I've narrowed this down to "seemingly nondeterministic behavior" with dotnet pack and when it decides it needs to rebuild the project. If I run dotnet test beforehand without the -p flag, and run dotnet pack afterwards with the -p flag, sometimes the NuGet package will be deterministic, and sometimes it won't.

Same here, but always instead of sometimes.

- name: Test
  run: dotnet test tests/Foo.Tests --configuration Release
- name: Pack
  run: dotnet pack src/Foo --configuration Release /p:ContinuousIntegrationBuild=true --output artifacts -bl

binlog:

Skipping target "CoreCompile" because all output files are up-to-date with respect to the input files.

Not sure if that's intended or a bug. When disabling the dotnet test step or when adding /p:ContinuousIntegrationBuild=true to it, I do get a deterministic binary.

@baronfel
Copy link
Member

OK, after reading this thread I think there's a bit of misunderstanding about the Deterministic/ContinuousIntegrationBuild properties and the actual impacts they have on the build. Let's work backwards a bit from @stijnherreman's example. The CoreCompile target mentioned above will only rebuild if any of its inputs have changed. What are those inputs?

<Target Name="CoreCompile"
          Inputs="$(MSBuildAllProjects);
                  @(Compile);
                  @(_CoreCompileResourceInputs);
                  $(ApplicationIcon);
                  $(KeyOriginatorFile);
                  @(ReferencePathWithRefAssemblies);
                  @(CompiledLicenseFile);
                  @(LinkResource);
                  @(EmbeddedDocumentation);
                  $(Win32Resource);
                  $(Win32Manifest);
                  @(CustomAdditionalCompileInputs);
                  $(ResolvedCodeAnalysisRuleSet);
                  @(AdditionalFiles);
                  @(EmbeddedFiles);
                  @(Analyzer);
                  @(EditorConfigFiles)" ...>
...
</Target>

There's a lot of stuff in here, but one thing to note is that all of these are files - MSBuild is a file-based, timestamp-based dependency engine. What this means is that unless any of these inputs directly or indirectly depends on Deterministic or ContinuousIntegrationBuild to determine their path, those properties won't trigger a recompilation.

So how do those properties get used in a typical .NET SDK build (as determined by a binlog of a dotnet build of a fresh console app on 8.0.100)?

  • Deterministic
    • This is mostly used as a property that gets passed to the Csc or Fsc Tasks - which are in the CoreCompile Target. This means that if CoreCompile doesn't run, this has no impact. There's some secondary usage, but nothing relevant to us here
  • ContinuousIntegrationBuild
    • This is mostly used in sourcelink when deciding if the generated source paths should have a build-machine-specific prefix stripped off of them

There may be custom build targets out in the ecosystem that also use these, but that's all that I can see in the SDK's targets.

What does this mean?

ContinuousIntegrationBuild doesn't have a direct impact on if compilation occurs any differently, but it does impact the contents of the sourcelink.json file, which is an input to the compiler. @jaredpar should the $(SourceLink) file be an input to CoreCompile?

Deterministic does impact the behavior of the compiler, but it's not directly tied to an input of CoreCompile. This is harder because properties cannot be inputs to a Target unless they are actually file paths - @jaredpar should we do some thing where compiler-impacting properties are written to an intermediate file and that file is used as an input to the compilation?

cc @dsplaisted @rainersigwald for thoughts/analysis.

@baronfel baronfel added Area-NetSDK needs team triage Requires a full team discussion labels Feb 13, 2024
@rainersigwald
Copy link
Member

should we do some thing where compiler-impacting properties are written to an intermediate file and that file is used as an input to the compilation?

We do this already for some, this is a reasonable addition.

https://github.com/dotnet/msbuild/blob/299e0514835a1588e6ef21b1da748462dec706b8/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3796-L3811

@marcpopMSFT
Copy link
Member

So in summary, we add a CoreCompileCache include for the deterministic build property and add source link as an input to CoreCompile (assuming Jared says yet).

This will make some builds more accurate so worth doing in 9 as it seems straightforward.

@marcpopMSFT marcpopMSFT modified the milestones: Discussion, 9.0.1xx Feb 13, 2024
@marcpopMSFT marcpopMSFT removed the needs team triage Requires a full team discussion label Feb 13, 2024
@jaredpar
Copy link
Member

@jaredpar should the $(SourceLink) file be an input to CoreCompile?

Seems reasonable.

My only pushback would be do we have plans to add a warning / analyzer for this? The $(SourceLink) is very easy to spot as an input to a task within a target. Shouldn't there be a diagnostic telling us this is suspect?

We do this already for some, this is a reasonable addition.

Is there a reason this couldn't be a core MSBuild primitive? Essentially a simple way for a group of non-file properties to be properly tracked as input to a target?

@baronfel
Copy link
Member

Is there a reason this couldn't be a core MSBuild primitive? Essentially a simple way for a group of non-file properties to be properly tracked as input to a target?

I believe this is dotnet/msbuild#701

My only pushback would be do we have plans to add a warning / analyzer for this? The $(SourceLink) is very easy to spot as an input to a task within a target. Shouldn't there be a diagnostic telling us this is suspect?

Added dotnet/msbuild#9741 to track this, it's a great idea.

@cremor
Copy link

cremor commented Feb 14, 2024

ContinuousIntegrationBuild doesn't have a direct impact on if compilation occurs any differently, but it does impact the contents of the sourcelink.json file, which is an input to the compiler.

It also effects the PDB files (removes absolute path to source files). This also happens if the project doesn't use source link.

@baronfel
Copy link
Member

@cremor can you help me find in the props/targets where that linkage is? I didn't see it when I was digging yesterday.

@cremor
Copy link

cremor commented Feb 14, 2024

Sorry, I don't know how the .NET SDK works internally. I only know that it has this effect because that's the reason I use the property.

Documentation here: https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#continuousintegrationbuild
It might have something to do with the note mentioned there (so with dotnet/roslyn#55860).

@rainersigwald
Copy link
Member

I believe it's the sourcelink.json input to the compiler that causes the compiler to emit different .pdb output

@KalleOlaviNiemitalo
Copy link

It's the PathMap parameter of the Csc task, not the SourceLink parameter.

baronfel added a commit to dotnet/msbuild that referenced this issue Feb 14, 2024
This is one part of dotnet/sdk#16325, identifying two areas where inputs to the CoreCompile Target (via the Csc/Fsc/etc Tasks) aren't tracked consistently.
@baronfel
Copy link
Member

I've sent PRs for all of these actions to MSBuild, Roslyn, and FSharp so this should be well on its way to fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests