Remove copy_version_files.sh

[omajid]

Arcade provides this via the GenerateNativeVersionFile target, which we already call in various places. Lets remove this file.

This file, if used, will also cause issues in a VMR build without a git repo. In that mode, git commands do not work, but Arcade and Source Link will still provide git commit and related information correctly.

[jkoritzinsky]

This script is meant for bootstrapping scenarios where we don't have a .NET runtime available for the platform and for innerloop development where build-runtime.sh is invoked directly (without going through the root build script).

Can we instead condition it to safely fail if it's in a VMR-like scenario?

[omajid]

Thanks for that context! It doesn't seem to get invoked at all (or maybe it does but it doesn't have any effect) during a normal (aka top-level ./build.sh) or VMR build. I thought it was some leftover code. I guess I can just close this PR. Or maybe add a comment with that context to the script?

[am11]

Can we instead condition it to safely fail if it's in a VMR-like scenario?

or just:

if git status > /dev/null 2>&1; then
  call_version_script
done

and for innerloop development where build-runtime.sh is invoked directly

I think we can dedup it now, since src/coreclr/build-runtime.sh sources eng/native/build-commons.sh.

[omajid]

if command -v git > /dev/null; then

I would prefer to avoid this. It's possible to attempt to build the git-archive of the VMR, while the git tool is installed. If we want to avoid invoking this script, that would do the wrong thing.

[am11]

if git status > /dev/null 2>&1; then updated

[jkoritzinsky]

if git status > /dev/null 2>&1; then updated

I think @am11's idea here is good as it will also work in the Microsoft VMR build environment if that is ever necessary.

[omajid]

I am not sure about that either.

Chances are, devs are working in a git repo - if it isn't the VMR, it contains the scripts to create the VMR archive and/or build the VMR. So git status might report version of an unrelated repository.

I think it would be better to make sure that this script is never called in a full/outloop/VMR scenario at all.

[am11]

I think it is fine not to support every perceivable scenario. We have only handful of runtime build scenarios which are supported and frequently tested.

For VMR, another approach is add a fallback, something like DOTNET_RUNTIME_COMMIT here:

runtime/eng/native/version/copy_version_files.sh

Line 12 in 8b04e76

commit="$(git rev-parse HEAD 2>/dev/null)"

set DOTNET_RUNTIME_COMMIT in VMR (there, we likely have that context in msbuild scripts which runtime commit is in use) and leave the call-sites intact (unconditioned).

[am11]

set: dotnet/dotnet@main...am11:dotnet:patch-1
get: main...am11:runtime:patch-29

Something like that.

[omajid]

For VMR, another approach is add a fallback, something like DOTNET_RUNTIME_COMMIT here:

Heh, we are actually trying to get rid of all those env vars (see dotnet/installer#18941), and use $(SourceRevisionId) in msbuild.

And actually, that's what brought me here with this PR 😄

set: dotnet/dotnet@main...am11:dotnet:patch-1
get: main...am11:runtime:patch-29

Something like that.

Let me think over this a bit.

[am11]

I see. BTW, that runtimeGitCommitHash is what sync bot updates, e.g. dotnet/dotnet@3efd8ca and since stuff like <EnvironmentVariables Include="MSBUILDDISABLENODEREUSE=1" /> is likely to stay in VMR, so can RUNTIME_GIT_COMMIT?

Note that this script also updates SCCSID in runtime and diagnostics repos and altering this will require retesting those scenarios end-to-end. In addition to bootstrapping, libSOS in diagnostics actually depends on the value of that sccsid[] in a way that it breaks the debugging functionality, so it is pretty vital. That's why we want to keep this non-msbuild version evaluation working.

[omajid]

Backing up a bit:

• If I close this PR without merging, the current behaviour stays.
• That is: we always run this copy_version_files script.
  • If this is an outerloop/VMR build, arcade has already generated the version file, like it always does. It already generates it correctly. This script can already generate or not generate the correct version file, call git or not, and nothing really happens, because the arcade-generated version.c file is used.
    • Arcade (with sourcelink) already handles VMR git vs archive versions/commit hashes correctly.
    • There's no need to change anything.
  • If this is an an innerloop/bootstrap build, then this script already does the right thing. There's no need to change anything

In other words, it seems like there's nothing to do here and everything is already okay?

[huoyaoyuan]

Note: this file is currently annoying for WSL and cross compiling Android on Windows. I'd like to see it become more universally available.

[am11]

@huoyaoyuan, this script is only meaningful in the context of official product build (the one with OfficialBuildId etc. set), so unless it is breaking the build, ignore it.

@omajid, that sounds even better. Lets keep it as-is if it's not causing any issues.