Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Rename OfflineBuild property to DotNetBuildOffline#39926

Merged
ericstj merged 2 commits into
dotnet:masterfrom
ericstj:renameOfflineBuild
Aug 2, 2019
Merged

Rename OfflineBuild property to DotNetBuildOffline#39926
ericstj merged 2 commits into
dotnet:masterfrom
ericstj:renameOfflineBuild

Conversation

@ericstj

@ericstj ericstj commented Jul 31, 2019

Copy link
Copy Markdown
Member

This will also need to go to release/3.0

@ericstj ericstj added this to the 3.0 milestone Jul 31, 2019
@ericstj
ericstj requested review from dseefeld and safern July 31, 2019 19:14
@ericstj ericstj self-assigned this Jul 31, 2019
@ericstj ericstj mentioned this pull request Jul 31, 2019
Closed
@dseefeld

dseefeld commented Jul 31, 2019

Copy link
Copy Markdown
Contributor

@ericstj I created a patch in source-build locally with this change in it to test building source-build and the resulting tarball. On tarball build, I'm seeing an error:

/src/tb-release3.0/src/corefx.1719a3fe2a5c81b67a4909787da4a02fb0d0d419/eng/notSupported.SourceBuild.targets(17,5): error : Error NotSupportedSourceFile '/src/tb-release3.0/src/generatedSrc/Microsoft.Win32.Registry.AccessControl/netcoreapp-Linux/Microsoft.Win32.Registry.AccessControl.notsupported.cs' did not exist under DotNetSourceBuildIntermediatePath. [/src/tb-release3.0/src/corefx.1719a3fe2a5c81b67a4909787da4a02fb0d0d419/src/Microsoft.Win32.Registry.AccessControl/src/Microsoft.Win32.Registry.AccessControl.csproj]

Any suggestions on how to troubleshoot this?

@ericstj

ericstj commented Jul 31, 2019

Copy link
Copy Markdown
Member Author

@dseefeld are you doing two pass build? The first pass specifying DotNetBuildFromSource=true;DotNetSourceBuildIntermediatePath=somePath. The second pass specifying DotNetBuildFromSource=true;DotNetSourceBuildIntermediatePath=somePath;DotNetBuildOffline=true?

If so, then the thing to check would be that the files are being written correctly to somepath and that you pass the same path into the second pass of the build.

@ericstj

ericstj commented Jul 31, 2019

Copy link
Copy Markdown
Member Author

Took a look at some logs shared offline and found the problem. The path specified for DotNetSourceBuildIntermediatePath in the first pass was different than the one for the second pass. They need to be the same in order to share the generated source.

@dseefeld

dseefeld commented Aug 1, 2019

Copy link
Copy Markdown
Contributor

@ericstj In the initial build, we're passing a path to generate the code to via DotNetSourceBuildIntermediatePath. When we generate the tarball, we copy the src to a directory under ./src and then pass that path in DotNetSourceBuildIntermediatePath. I compared the two directories just to be sure and the contents are identical. The source that the error is complaining about Microsoft.Win32.Registry.AccessControl/netcoreapp-Linux/Microsoft.Win32.Registry.AccessControl.notsupported.cs does not exist.

@ericstj
Change sequencing of _RedefineNotSupportedSourceFile
420fc58 
When running as an initial target _RedefineNotSupportedSourceFile would
run too many times, as currently the config system will evaluate projects
under the parent project's Configuration.
@ericstj

ericstj commented Aug 1, 2019

Copy link
Copy Markdown
Member Author

Chatted with @dseefeld offline and the latest change addresses the failure he was seeing. Will merge this once green and port to release/3.0.

@ericstj ericstj added the auto-merge Automatically merge PR once CI passes. label Aug 2, 2019
@ghost

ghost commented Aug 2, 2019

Copy link
Copy Markdown

Hello @ericstj!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ericstj
ericstj merged commit 1ea5d58 into dotnet:master Aug 2, 2019
ericstj added a commit to ericstj/corefx that referenced this pull request Aug 2, 2019
@ericstj
Rename OfflineBuild property to DotNetBuildOffline (dotnet#39926)
767dae4 
* Rename OfflineBuild property to DotNetBuildOffline

* Change sequencing of _RedefineNotSupportedSourceFile

When running as an initial target _RedefineNotSupportedSourceFile would
run too many times, as currently the config system will evaluate projects
under the parent project's Configuration.
@ericstj ericstj mentioned this pull request Aug 2, 2019
Merged
@karelz karelz modified the milestones: 3.0, 5.0 Aug 3, 2019
@karelz

karelz commented Aug 3, 2019

Copy link
Copy Markdown
Member

This PR went into master - fixing milestone to 5.0.
It was later ported into release/3.0 branch in PR #39979.

wtgodbe pushed a commit that referenced this pull request Aug 7, 2019
@ericstj @wtgodbe
Rename OfflineBuild property to DotNetBuildOffline (#39926) (#39979)
0fd6438 
* Rename OfflineBuild property to DotNetBuildOffline

* Change sequencing of _RedefineNotSupportedSourceFile

When running as an initial target _RedefineNotSupportedSourceFile would
run too many times, as currently the config system will evaluate projects
under the parent project's Configuration.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@ericstj
Rename OfflineBuild property to DotNetBuildOffline (dotnet/corefx#39926)
5a6b96e 
* Rename OfflineBuild property to DotNetBuildOffline

* Change sequencing of _RedefineNotSupportedSourceFile

When running as an initial target _RedefineNotSupportedSourceFile would
run too many times, as currently the config system will evaluate projects
under the parent project's Configuration.


Commit migrated from dotnet/corefx@1ea5d58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@dseefeld dseefeld dseefeld approved these changes
@safern safern safern approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ericstj ericstj

Labels

area-Infrastructure-libraries auto-merge Automatically merge PR once CI passes.

Projects

None yet

Milestone

5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@ericstj @dseefeld @karelz @safern