-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Move optdata and version file generation up to MSBuild from build-runtime #58674
Merged
hoyosjs
merged 47 commits into
dotnet:main
from
jkoritzinsky:no-msbuild-in-build-runtime
Oct 27, 2021
Merged
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
bff7536
Move optdata and version file generation from the native build script…
jkoritzinsky a8b692c
Update CI to use the new prereqs model.
jkoritzinsky 1813a73
Remove some old MSBuild arg pass-through that's no longer needed as b…
jkoritzinsky 81f808e
Apply suggestions from code review
jkoritzinsky db7eb91
Move some targets around based on feedback.
jkoritzinsky 5fbd69e
Output version files in the artifacts/obj dir so they can be easily s…
jkoritzinsky 790204b
Move native pgo into a targets file instead of being a separate proje…
jkoritzinsky 751f259
Fix subsets.
jkoritzinsky 1066209
Share version file path by default
jkoritzinsky 2bb8b1d
Fix VER_FILEDESCRIPTION_STR
jkoritzinsky 0d1e82b
Fix version fallback copies.
jkoritzinsky 99fb01f
Fix include path for the version headers.
jkoritzinsky 0411fab
Update src/coreclr/dlls/mscordac/CMakeLists.txt
jkoritzinsky 70afaad
Enable easily sharing the fallback version files between all componen…
jkoritzinsky d4e8d9c
Remove some now-unused command-line options from our build scripts.
jkoritzinsky dd83c26
Bump importance output to pass the pgo path between steps.
jkoritzinsky 19a327c
Add eval statement to run copy_version_files script
jkoritzinsky aa62517
Rename some files and and standardize on a _version.c file name for L…
jkoritzinsky 7ddf99c
Remove temp proj ref.
jkoritzinsky 9817bc5
Fix permissions
jkoritzinsky bfcff4d
Reference targets to produce version file in libraries native build.
jkoritzinsky 7d0a041
Merge branch 'no-msbuild-in-build-runtime' of /mnt/d/source/runtime i…
jkoritzinsky 032bf86
Fix CoreCLR build breaks
jkoritzinsky 00333fc
Generate the version files for Mono
jkoritzinsky 8d5745d
include configurepaths.cmake on the Windows CoreFX build.
jkoritzinsky cfe980a
Restore mono.proj when building it through monoaotcross.proj.
jkoritzinsky 5fbb5bd
Update copy script to work on macos.
jkoritzinsky 7340c8e
Make sure mono pulls in the shared version files.
jkoritzinsky 0a3c1c5
Add artifcats/obj dir as include dir for CoreCLR diagonstic component…
jkoritzinsky 7898e75
Add artifacts/obj include for the whole mono build.
jkoritzinsky c0dcc5c
Update CMakeLists.txt
jkoritzinsky 22e1a60
Fix NativeVersion.rc path on Windows.
jkoritzinsky 879be00
Fix linux version parsing to point at the right file.
jkoritzinsky d871c22
Fix mono version parsing (apparently the quotes make a difference here)
jkoritzinsky a9b03d6
Update src/coreclr/build-runtime.sh
jkoritzinsky f9aa7be
Update eng/native/version/copy_version_files.sh
jkoritzinsky 83b726a
Update copy_version_files.sh to insert the current commit hash into _…
jkoritzinsky 7423625
Apply suggestions from code review
jkoritzinsky 272fb26
Update the _version.c writing script to only update the placeholder v…
jkoritzinsky 21fe213
Generate the version files for wasm/browser as well.
jkoritzinsky fb4b07c
Merge branch 'main' into no-msbuild-in-build-runtime
jkoritzinsky 44cc7af
Merge branch 'main' of https://github.com/dotnet/runtime into no-msbu…
jkoritzinsky 52aaaf8
Fix Mono WASM cross build.
jkoritzinsky 28d72d6
Merge branch 'main' into no-msbuild-in-build-runtime
jkoritzinsky fdfc57c
Update eng/nativepgo.targets
jkoritzinsky 6cfcf9a
Merge branch 'main' into no-msbuild-in-build-runtime
jkoritzinsky 9115b47
Fix comparison for NativeOptimizationDataSupported.
jkoritzinsky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
#include "_version.h" | ||
|
||
#include <windows.h> | ||
|
||
VS_VERSION_INFO VERSIONINFO | ||
FILEVERSION VER_FILEVERSION | ||
PRODUCTVERSION VER_PRODUCTVERSION | ||
FILEFLAGSMASK VS_FFI_FILEFLAGSMASK | ||
FILEFLAGS VER_DEBUG | ||
FILEOS VOS__WINDOWS32 | ||
FILETYPE VFT_DLL | ||
FILESUBTYPE VFT2_UNKNOWN | ||
BEGIN | ||
BLOCK "StringFileInfo" | ||
BEGIN | ||
BLOCK "040904E4" | ||
BEGIN | ||
VALUE "CompanyName", VER_COMPANYNAME_STR | ||
VALUE "FileDescription", VER_FILEDESCRIPTION_STR | ||
VALUE "FileVersion", VER_FILEVERSION_STR | ||
VALUE "InternalName", VER_INTERNALNAME_STR | ||
VALUE "LegalCopyright", VER_LEGALCOPYRIGHT_STR | ||
VALUE "OriginalFilename", VER_ORIGINALFILENAME_STR | ||
VALUE "ProductName", VER_PRODUCTNAME_STR | ||
VALUE "ProductVersion", VER_PRODUCTVERSION_STR | ||
END | ||
END | ||
|
||
BLOCK "VarFileInfo" | ||
BEGIN | ||
/* The following line should only be modified for localized versions. */ | ||
/* It consists of any number of WORD,WORD pairs, with each pair */ | ||
/* describing a language,codepage combination supported by the file. */ | ||
/* */ | ||
/* For example, a file might have values "0x409,1252" indicating that it */ | ||
/* supports English language (0x409) in the Windows ANSI codepage (1252). */ | ||
|
||
VALUE "Translation", 0x409, 1252 | ||
|
||
END | ||
END |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
static char sccsid[] __attribute__((used)) = "@(#)No version information produced"; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
#ifndef VER_COMPANYNAME_STR | ||
#define VER_COMPANYNAME_STR ".NET Foundation" | ||
#endif | ||
#ifndef VER_FILEDESCRIPTION_STR | ||
#define VER_FILEDESCRIPTION_STR ".NET Runtime" | ||
#endif | ||
#ifndef VER_INTERNALNAME_STR | ||
#define VER_INTERNALNAME_STR VER_FILEDESCRIPTION_STR | ||
#endif | ||
#ifndef VER_ORIGINALFILENAME_STR | ||
#define VER_ORIGINALFILENAME_STR VER_FILEDESCRIPTION_STR | ||
#endif | ||
#ifndef VER_PRODUCTNAME_STR | ||
#define VER_PRODUCTNAME_STR ".NET" | ||
#endif | ||
#undef VER_PRODUCTVERSION | ||
#define VER_PRODUCTVERSION 00,00,00,00000 | ||
#undef VER_PRODUCTVERSION_STR | ||
#define VER_PRODUCTVERSION_STR "0.0.0" | ||
#undef VER_FILEVERSION | ||
#define VER_FILEVERSION 00,00,00,00000 | ||
#undef VER_FILEVERSION_STR | ||
#define VER_FILEVERSION_STR "00,00,00,00000" | ||
#ifndef VER_LEGALCOPYRIGHT_STR | ||
#define VER_LEGALCOPYRIGHT_STR ".NET Foundation" | ||
#endif | ||
#ifndef VER_DEBUG | ||
#define VER_DEBUG VS_FF_DEBUG | ||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
@if not defined _echo @echo off | ||
setlocal EnableDelayedExpansion EnableExtensions | ||
|
||
set __VersionFolder=%~dp0 | ||
set __RepoRoot=%~dp0..\..\.. | ||
set __artifactsObjDir=%__RepoRoot%\artifacts\obj | ||
|
||
for /r "%__VersionFolder%" %%a in (*.h *.rc) do ( | ||
if not exist "%__artifactsObjDir%\%%~nxa" ( | ||
copy "%%a" "%__artifactsObjDir%" | ||
) | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
#!/usr/bin/env bash | ||
|
||
__VersionFolder="$(cd "$(dirname "$0")"; pwd -P)" | ||
jkoritzinsky marked this conversation as resolved.
Show resolved
Hide resolved
|
||
__RepoRoot="$(cd "$(dirname "$__VersionFolder")/../../"; pwd -P)" | ||
|
||
for path in "${__VersionFolder}/"*{.h,.c}; do | ||
if [[ "$(basename $path)" == _version.c ]]; then | ||
jkoritzinsky marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# For _version.c, update the commit ID if it has changed from the last build. | ||
# Set IFS to nothing to prevent the shell from combining all of the piped output into a single line in the script below | ||
IFS= | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just out of curiosity, is it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's bash jumbling the lines that are outputted from cat and sed. |
||
# update commit | ||
commit="$(git rev-parse HEAD 2>/dev/null)" | ||
commit="${commit:-N/A}" | ||
substitute="$(printf 'static char sccsid[] __attribute__((used)) = "@(#)Version N/A @Commit: %s";\n' "$commit")" | ||
version_file_contents="$(cat "$path" | sed "s|^static.*|$substitute|")" | ||
version_file_destination="$__RepoRoot/artifacts/obj/_version.c" | ||
current_contents= | ||
is_placeholder_file= | ||
if [[ -e "$version_file_destination" ]]; then | ||
current_contents="$(<"$__RepoRoot/artifacts/obj/_version.c")" | ||
# If the current file has the version placeholder this script uses, we can update it | ||
# to have the current commit. Otherwise, use the current version file that has the actual product version. | ||
is_placeholder_file="$(echo $current_contents | grep "@(#)Version N/A @Commit:")" | ||
else | ||
# Treat a non-existent file like a file that doesn't exist. | ||
is_placeholder_file=1 | ||
fi | ||
if [[ "$is_placeholder_file" && "$version_file_contents" != "$current_contents" ]]; then | ||
echo "$version_file_contents" > "$version_file_destination" | ||
fi | ||
elif [[ ! -e "$__RepoRoot/artifacts/obj/$(basename "$path")" ]]; then | ||
cp "$path" "$__RepoRoot/artifacts/obj/" | ||
fi | ||
done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
#define RuntimeAssemblyMajorVersion 0 | ||
#define RuntimeAssemblyMinorVersion 0 | ||
#define RuntimeFileMajorVersion 0 | ||
#define RuntimeFileMinorVersion 0 | ||
#define RuntimeFileBuildVersion 0 | ||
#define RuntimeFileRevisionVersion 0 | ||
#define RuntimeProductMajorVersion 0 | ||
#define RuntimeProductMinorVersion 0 | ||
#define RuntimeProductPatchVersion 0 | ||
#define RuntimeProductVersion |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
<Project> | ||
<PropertyGroup> | ||
<NativeOptimizationDataSupported Condition="'$(TargetOS)' == 'windows' And ('$(TargetArchitecture)' == 'x64' Or '$(TargetArchitecture)' == 'x86')">true</NativeOptimizationDataSupported> | ||
<NativeOptimizationDataSupported Condition="'$(TargetOS)' == 'Linux' And '$(TargetArchitecture)' == 'x64'">true</NativeOptimizationDataSupported> | ||
<NativeOptimizationDataSupported Condition="'$(NoPgoOptimize)' == 'true'">false</NativeOptimizationDataSupported> | ||
<NativeOptimizationDataSupported Condition="'$(DotNetBuildFromSource)' == 'true'">false</NativeOptimizationDataSupported> | ||
jkoritzinsky marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<NativeOptimizationDataSupported Condition="'$(Configuration)' != 'Release'">false</NativeOptimizationDataSupported> | ||
|
||
<_NativeOptimizationDataPackageTarget>$(TargetOS.ToLower())-$(TargetArchitecture.ToLower())</_NativeOptimizationDataPackageTarget> | ||
<_NativeOptimizationDataPackageTarget Condition="'$(TargetOS)' == 'windows'">windows_nt-$(TargetArchitecture.ToLower())</_NativeOptimizationDataPackageTarget> | ||
|
||
</PropertyGroup> | ||
<ItemGroup> | ||
<PackageReference Include="optimization.$(_NativeOptimizationDataPackageTarget).PGO.CoreCLR" | ||
Version="$(optimizationPGOCoreCLRVersion)" | ||
Condition="'$(optimizationPGOCoreCLRVersion)'!='' And '$(NativeOptimizationDataSupported)'=='true'" | ||
GeneratePathProperty="true" /> | ||
</ItemGroup> | ||
|
||
|
||
<!-- --> | ||
<!-- Task: GetPgoDataPackagePath --> | ||
<!-- --> | ||
<!-- Notes: --> | ||
<!-- --> | ||
<!-- DumpPgoDataPackagePath is used to get the path of the native PGO data --> | ||
<!-- for other MSBuild projects, generally to pass to another project or --> | ||
<!-- native script like build-runtime.cmd/sh. --> | ||
<!-- --> | ||
|
||
<Target Name="GetPgoDataPackagePath" Returns="$(PgoPackagePath)"> | ||
<PropertyGroup> | ||
<PgoPackagePathProperty>Pkgoptimization_$(_NativeOptimizationDataPackageTarget)_PGO_CoreCLR</PgoPackagePathProperty> | ||
</PropertyGroup> | ||
|
||
<!-- | ||
Use an item group for expansion of $($(PgoPackagePathProperty)) (an illegal MSBuild expression) | ||
i.e. the prop value's value. | ||
--> | ||
<ItemGroup> | ||
<PgoPackagePathPropertyItemList Include="$(PgoPackagePathProperty)" /> | ||
<PgoPackagePathPropertyItemList> | ||
<PgoPackagePath>$(%(Identity))</PgoPackagePath> | ||
</PgoPackagePathPropertyItemList> | ||
</ItemGroup> | ||
|
||
<PropertyGroup> | ||
<PgoPackagePath>@(PgoPackagePathPropertyItemList->'%(PgoPackagePath)')</PgoPackagePath> | ||
</PropertyGroup> | ||
|
||
<Error Condition="!Exists('$(PgoPackagePath)') And '$(NativeOptimizationDataSupported)' == 'true'" Text="Unable to locate restored PGO package at $(PgoPackagePath). Maybe the platform-specific package naming changed?" /> | ||
</Target> | ||
|
||
<Target Name="OutputPgoPathForCI" Condition="'$(ContinuousIntegrationBuild)' == 'true' and '$(NativeOptimizationDataSupported)' == 'true'" DependsOnTargets="GetPgoDataPackagePath"> | ||
<Message Text="##vso[task.setvariable variable=CoreClrPgoDataArg]-pgodatapath "$(PgoPackagePath)"" Importance="High" /> | ||
</Target> | ||
</Project> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just inline the
cp
line (as done on line 52) rather than indirection and eval? Note that we don't always have 1:1 correspondence between .sh and .cmd, when it comes to internal scripts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to move towards a 1:1 correspondence to simplify maintenance costs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate file is alright, but concern was regarding usage of
eval
: in other places like this file's (build-commons.sh
) own usage, we have managed to avoid eval by using sourcing. eval is generally considered as a last resort in scripting languages. We can usesource copy_version_files.sh
(or. copy_version_files.sh
) syntax here as well.This is my ideal version of
copy_version_files.sh
, consistent with most of the existing code:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For testing purposes, it's very nice to be able to manually call this script without having to set up any environment variables beforehand. I'd like to keep that experience if possible. If there's a way to call into another script using the current scripting engine when the path is constructed from vars in bash, I'd happily switch to that from
eval
.