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

crossgen.sh improvements - #1504

Merged
ellismg merged 3 commits into
dotnet:masterfrom
ellismg:crossgen-improvements
Jun 1, 2017
Merged

crossgen.sh improvements#1504
ellismg merged 3 commits into
dotnet:masterfrom
ellismg:crossgen-improvements

Conversation

@ellismg

@ellismg ellismg commented May 17, 2017

Copy link
Copy Markdown

This PR has two changes:

  1. Don't crossgen a file if the ni is already present. This will address the segmentation faults we see when we try to crossgen System.Private.Corelib.dll (the .ni.dll already exists, it's in the package).
  2. If NUGET_PACKAGES is set in the environment, respect that variable as the location to restore packages to.

I'd like some sort of knob to allow me to restore packages to a shared location as part of the build from source work, and NUGET_PACKAGES seemed like the obvious choice, since the tooling understands that by default.

ellismg added 2 commits May 16, 2017 16:56
@ellismg
Do not try to crossgen a file if the ni already exists
c6dcaf9 
Aside from waisting time, we were hitting cases where we'd try to
crossgen the IL version of System.Private.CoreLib.dll when the ni
already exists (as we ship an already ngen'd image in the nupkg we
consume) and doing so was causing crossgen to fail.
@ellismg
Respect NUGET_PACKAGES if it is set
f9f9139 
If NUGET_PACKAGES is set, restore packages to that folder instead of
into the packages folder in the source tree.
@ellismg
ellismg requested review from mellinoe and weshaggard May 17, 2017 00:04
weshaggard
weshaggard reviewed May 17, 2017
View reviewed changes
Comment thread src/Microsoft.DotNet.Build.Tasks/PackageFiles/crossgen.sh
__outname="${__file/.dll/.ni.dll}"
__outname="${__outname/.exe/.ni.exe}"
echo "$__file -> $__outname"
if [[ ($__file == *.dll && -e ${__file/.dll/.ni.dll}) || ($__file == *.exe && -e ${__file/.exe/.ni.exe}) ]]; then

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

weshaggard
weshaggard reviewed May 17, 2017
View reviewed changes
Comment thread src/Microsoft.DotNet.Build.Tasks/PackageFiles/crossgen.sh
__toolsDir=$__scriptpath/../Tools
__dotnet=$__toolsDir/dotnetcli/dotnet
__packagesDir=$__scriptpath/../packages
__packagesDir="${NUGET_PACKAGES:-${__scriptpath}/../packages}"

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@danmoseley
danmoseley requested a review from wtgodbe May 23, 2017 00:24
@ellismg

ellismg commented May 24, 2017

Copy link
Copy Markdown
Author

@weshaggard I added an explicit check to not crossgen System.Private.Corelib.dll.

I also added a commit that makes it possible to specify CommitHash and VersionSeedDate properties explicitly instead of always shelling out to git (which is useful for the build for source stuff).

weshaggard
weshaggard reviewed May 24, 2017
View reviewed changes
Comment thread src/Microsoft.DotNet.Build.Tasks/PackageFiles/versioning.targets Outdated
<LatestDateCommandExitCode>0</LatestDateCommandExitCode>
</PropertyGroup>


This comment was marked as spam.

Sign in to view

@ellismg
ellismg force-pushed the crossgen-improvements branch from b05c67b to fbbd0f3 Compare May 24, 2017 20:59
@ellismg
Allow CommitHash and VersionSeedDate to be overriden
92e10cb 
This will allow us to set these values explicitly when we are not
building a git repository.
@ellismg
ellismg force-pushed the crossgen-improvements branch from fbbd0f3 to 92e10cb Compare May 24, 2017 21:00
@chcosta chcosta mentioned this pull request May 31, 2017
Merged
@ellismg
ellismg merged commit 36c522b into dotnet:master Jun 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@wtgodbe wtgodbe Awaiting requested review from wtgodbe
2 more reviewers
@weshaggard weshaggard weshaggard approved these changes
@mellinoe mellinoe mellinoe approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ellismg @mellinoe @weshaggard @dnfclas