Skip to content

Conversation

@tmds
Copy link
Member

@tmds tmds commented Jun 20, 2023

@tmds
Copy link
Member Author

tmds commented Jun 20, 2023

The changes to llvm.proj are for supporting non-crossbuilds.

There are a few issues when the vmr builds this through arcade:

1._InitializeAssemblyVersion which is used in packages.builds is not found. This target should come from Microsoft.DotNet.Arcade.Sdk:

    /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/packages.builds(38,11): error MSB4057: The target "_InitializeAssemblyVersion" does not exist in the project.
  ##vso[task.logissue type=error;sourcepath=/home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/packages.builds;linenumber=38;columnnumber=11;code=MSB4057;](NETCORE_ENGINEERING_TELEMETRY=Build) The target "_InitializeAssemblyVersion" does not exist in the project.
  1. I copied the LICENSE file to the project root to silence Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/AfterSourceBuild.proj which expects to find the license there:

edit: no longer an issue since the branch was changed, and the new target branch already includes a LICENSE file in the root.

  1. The first time this builds, the microsoft.dotnet.build.tasks.packaging fails to find the Icon.png. Starting the build a second time, the issue disappears. This is the error message (it appears for several pkgproj, the output only shows one):
  /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/package-cache/microsoft.dotnet.build.tasks.packaging/8.0.0-beta.23316.6/build/Packaging.targets(1264,5): error : Error when creating nuget lib package from /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/artifacts/packages/Release/specs/runtime.linux-x64.Microsoft.NETCore.Runtime.JIT.Tools.nuspec. NuGet.Packaging.Core.PackagingException: The icon file 'Icon.png' does not exist in the package. [/home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/Microsoft.NETCore.Runtime.JIT.Tools/Microsoft.NETCore.Runtime.JIT.Tools.pkgproj]
  /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/package-cache/microsoft.dotnet.build.tasks.packaging/8.0.0-beta.23316.6/build/Packaging.targets(1264,5): error :    at NuGet.Packaging.PackageBuilder.ValidateIconFile(IEnumerable`1 files, String iconPath) [/home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/Microsoft.NETCore.Runtime.JIT.Tools/Microsoft.NETCore.Runtime.JIT.Tools.pkgproj]
  /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/package-cache/microsoft.dotnet.build.tasks.packaging/8.0.0-beta.23316.6/build/Packaging.targets(1264,5): error :    at NuGet.Packaging.PackageBuilder.Save(Stream stream) [/home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/Microsoft.NETCore.Runtime.JIT.Tools/Microsoft.NETCore.Runtime.JIT.Tools.pkgproj]
  /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/package-cache/microsoft.dotnet.build.tasks.packaging/8.0.0-beta.23316.6/build/Packaging.targets(1264,5): error :    at Microsoft.DotNet.Build.Tasks.Packaging.NuGetPack.Pack(String nuspecPath, String nupkgPath, Manifest manifest, Boolean packSymbols) in /_/src/arcade/artifacts/source-build/self/src/src/Microsoft.DotNet.Build.Tasks.Packaging/src/NuGetPack.cs:line 304 [/home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/src/nuget/Microsoft.NETCore.Runtime.JIT.Tools/Microsoft.NETCore.Runtime.JIT.Tools.pkgproj]
  /home/tmds/repos/dotnet/src/llvm-project/artifacts/source-build/self/package-

@MichaelSimons @premun can you provide some assistance with these issues?

@MichaelSimons
Copy link
Member

It looks like this repo also needs a source-build CI enabled.

@MichaelSimons
Copy link
Member

@MichaelSimons @premun can you provide some assistance with these issues?

I can take a look but it won't be until later in the week. I am fully booked today.

@tmds
Copy link
Member Author

tmds commented Jun 21, 2023

I can take a look but it won't be until later in the week. I am fully booked today.

@MichaelSimons that would be great. I'm sure you'll make faster progress debugging this than I would.

@directhex can you please review the changes I've made to llvm.proj?

@tmds
Copy link
Member Author

tmds commented Jun 21, 2023

Did I break the Windows arm64 builds?

llvm.proj(139,5): error MSB3073: (NETCORE_ENGINEERING_TELEMETRY=Build) The command "
    :: VisualStudio includes vswhere.exe that can be used to locate current VisualStudio installation.
    set VSWHERE_TOOLS_BIN=%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe
    set VS_VCINSTALL_DIR=

    :: Try to locate installed VisualStudio VC environment.
    if "%VCINSTALLDIR%" == "" if exist "%VSWHERE_TOOLS_BIN%" (
        for /f "tokens=*" %%a in ('"%VSWHERE_TOOLS_BIN%" -latest -prerelease -property installationPath') do (
            set VS_VCINSTALL_DIR=%%a\VC\
        )
    )

    if NOT "%VCINSTALLDIR%" == "" set VS_VCINSTALL_DIR=%VCINSTALLDIR%

    :: Run VS build environment script.
    call "%VS_VCINSTALL_DIR%\Auxiliary\Build\vcvarsamd64_arm64.bat"

     ninja " exited with code 1.

@directhex
Copy link

Did I break the Windows arm64 builds?

Yes.

The Windows build is split into two parts:

  • Compile various build-time tools needed by LLVM for $HOST
  • Execute those tools whilst building for $TARGET

The error you're getting is:

This version of D:\a\_work\1\s\artifacts\obj\BuildRoot-arm64\bin\clang-ast-dump.exe is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher. which implies it's trying to build/use ARM64 clang-ast-dump.exe on x64 build VMs.

@directhex
Copy link

The core issue is for Windows, the host/target steps are separated out in azure-pipelines.yml:

        - powershell: eng\build.ps1 -ci -restore -build -arch x64 -configuration $(_BuildConfig) $(_InternalBuildArgs) /p:BuildLLVMTableGenOnly=true
          displayName: 'Build host llvm-tblgen for cross-compiling'

        - powershell: eng\build.ps1 -ci -restore -build -pack $(archflag) -configuration $(_BuildConfig) $(_InternalBuildArgs) $(LLVMTableGenArg) $(ClangTableGenArg)
          displayName: 'Build and package'

We'll need to translate this logic to MSBuild for source-build to go through in a single pass.

@tmds
Copy link
Member Author

tmds commented Jun 23, 2023

@directhex I've fixed the Windows arm64 CI build issue by making it cross-build aware. CI is passing now. Can you review the changes to llvm.proj?

@MichaelSimons
Copy link
Member

A source-build CI leg should be enabled as well. Instructions can be found at https://github.com/dotnet/source-build/blob/main/Documentation/planning/arcade-powered-source-build/onboarding/ci-onboarding.md.

@directhex
Copy link

This will need re-targeting against dotnet/main-16.x

@tmds tmds changed the base branch from dotnet/main to dotnet/main-16.x July 3, 2023 10:12
@tmds tmds changed the base branch from dotnet/main-16.x to dotnet/main July 3, 2023 10:17
@tmds tmds changed the base branch from dotnet/main to dotnet/main-16.x July 3, 2023 11:04
@tmds
Copy link
Member Author

tmds commented Jul 3, 2023

A source-build CI leg should be enabled as well. Instructions can be found at https://github.com/dotnet/source-build/blob/main/Documentation/planning/arcade-powered-source-build/onboarding/ci-onboarding.md.

@MichaelSimons it seems like we're in the "The simplest way to onboard". -case with the eng/azure-pipelines.yml file:

stages:
- stage: build
displayName: Build
jobs:
- template: /eng/common/templates/jobs/jobs.yml

What platforms should I add under sourceBuildParameters?

@tmds
Copy link
Member Author

tmds commented Jul 3, 2023

This will need re-targeting against dotnet/main-16.x

I've updated the target branch.

@jkotas
Copy link
Member

jkotas commented Jul 4, 2023

What platforms should I add under sourceBuildParameters?

The list of the platforms in the doc is way out of date. It lists platforms that are no longer supported, it uses stale docker image links, etc.

I think it is sufficient to add a single non-portable build, say Debian 11. It is unlikely that this repo is going to have distro-specific source build issues.

@tmds
Copy link
Member Author

tmds commented Jul 4, 2023

The list of the platforms in the doc is way out of date. It lists platforms that are no longer supported, it uses stale docker image links, etc.

I also don't find any repo that uses sourceBuildParameters which I can use as a base.

@MichaelSimons
Copy link
Member

@NikolaMilosavljevic - Can you help @tmds get the source-build CI leg enabled? Please make sure there are tracking issues/PRs to ensure the guidelines are up-to-date. TIA.

@NikolaMilosavljevic
Copy link
Member

@tmds it is not necessary to specify any platform. Default platform would be used and that should be sufficient for this repo - https://github.com/dotnet/arcade/blob/e4bd767159fde419b50dd54c6d83e1ef970a68d0/eng/common/templates/jobs/source-build.yml#L42-L46

@NikolaMilosavljevic
Copy link
Member

@tmds the only other repo building non-managed for source-build, is runtime. It appears that they use almalinux-8: https://github.com/dotnet/runtime/blob/18321c608e32bc5d13ad0299822cf02ab7ebfd6c/eng/pipelines/common/templates/pipeline-with-resources.yml#L62-L64

Here are the job parameters: https://github.com/dotnet/runtime/blob/18321c608e32bc5d13ad0299822cf02ab7ebfd6c/eng/pipelines/runtime-official.yml#L423-L435

Note that runtime uses a different job template.

@ViktorHofer does almalinux-8 make the most sense for llvm repo's source-build leg? This would produce packages that will be consumed by other repos.

@jkotas
Copy link
Member

jkotas commented Jul 5, 2023

does almalinux-8 make the most sense for llvm repo's source-build leg?

almalinux-8 makes sense. (dotnet/runtime uses almalinux-8 as a proxy for Red Hat 8.)

@tmds
Copy link
Member Author

tmds commented Jul 8, 2023

@NikolaMilosavljevic it looks like the source build leg gets cancelled after an hour. How can I increase the time-out?

@NikolaMilosavljevic
Copy link
Member

@NikolaMilosavljevic it looks like the source build leg gets cancelled after an hour. How can I increase the time-out?

You can set timeoutInMinutes similarly to how it's done in runtime: https://github.com/dotnet/runtime/blob/a2a1885f8d0f66546788c758c62013c3da10de86/eng/pipelines/runtime-official.yml#L435 This is further consumed here: https://github.com/dotnet/runtime/blob/a2a1885f8d0f66546788c758c62013c3da10de86/eng/pipelines/common/global-build-job.yml#L47

@mmitche
Copy link
Member

mmitche commented Aug 3, 2023

Adding LLVM into source-build is going to have a significant impact on dependency flows specifically at sdk->installer. Adding 2+ hours to source-build is going to cause PR validation to rarely finish before another update flows in therefore likely causing large delays. Are the ways that we can mitigate this?

Unless the PR fails at any point (a PR check is failing), Maestro will not add new commits on top of a PR that is currently in validation. So it shouldn't cycle endlessly in the best case. But, failures are common so you can get into a bad spot.

I think overall that only enabling the objwriter is the right thing to do for .NET 8.

@NikolaMilosavljevic
Copy link
Member

The source-build leg is green 🎉

@NikolaMilosavljevic please check whether the changes I pushed look good, then this should be ready to go.

They look good. I'll sign-off after source-build leg is scoped down to build just the ObjWriter.

@Thefrank
Copy link

Thefrank commented Aug 3, 2023

From a personal experience of working with this repo on FreeBSD, the ObjWriter takes minutes (15 at most) to build whereas the entirety of ObjWriter+JITTools+LLVM takes over 2 hours. At 2+ hours this repo alone would add almost 40% to the build time of a source-build (again, for FreeBSD)

@omajid
Copy link
Member

omajid commented Aug 3, 2023

As a distro maintainer, it also bothers me that we are rebuilding llvm, which many distros already maintain and package separately. Is there no way we can just build the custom subcomponents like ObjWriter but re-use the llvm packages that distros already maintain? Is it possible to upstream the changes in our llvm fork?

@directhex
Copy link

directhex commented Aug 3, 2023 via email

@akoeplinger
Copy link
Member

@NikolaMilosavljevic hmm I found no way to pass custom properties into the source build leg via the .yml template so that it ends up being passed to build.sh, is that something we'd need to add in arcade?

@NikolaMilosavljevic
Copy link
Member

@NikolaMilosavljevic hmm I found no way to pass custom properties into the source build leg via the .yml template so that it ends up being passed to build.sh, is that something we'd need to add in arcade?

cc @mmitche

@jkotas
Copy link
Member

jkotas commented Aug 3, 2023

In dotnet/runtime, we customize what does and does not get build using the DotNetBuildFromSource property: https://github.com/search?q=repo%3Adotnet%2Fruntime%20DotNetBuildFromSource

Can we do the same thing here?

@mmitche
Copy link
Member

mmitche commented Aug 3, 2023

Agreed with @jkotas. There are some other options with eng/SourceBuild.props e.g. https://github.com/dotnet/aspnetcore/blob/main/eng/SourceBuild.props if that doesn't work.

@NikolaMilosavljevic
Copy link
Member

In dotnet/runtime, we customize what does and does not get build using the DotNetBuildFromSource property: https://github.com/search?q=repo%3Adotnet%2Fruntime%20DotNetBuildFromSource

Can we do the same thing here?

Maybe we could define DotNetBuildFromSource in common import, and set it false, and only set this property to true in projects that we do want to build. This will need to be updated post-8.0, to build everything, but it should unblock the 8.0 work.

@mmitche
Copy link
Member

mmitche commented Aug 3, 2023

In dotnet/runtime, we customize what does and does not get build using the DotNetBuildFromSource property: https://github.com/search?q=repo%3Adotnet%2Fruntime%20DotNetBuildFromSource
Can we do the same thing here?

Maybe we could define DotNetBuildFromSource in common import, and set it false, and only set this property to true in projects that we do want to build. This will need to be updated post-8.0, to build everything, but it should unblock the 8.0 work.

Do you mean ExcludeFromSourceBuild?

@NikolaMilosavljevic
Copy link
Member

NikolaMilosavljevic commented Aug 3, 2023

ExcludeFromSourceBuild

Yeah, that is the one that's meant for project-level conditioning. The other property would work as well, but would require more changes.

@akoeplinger
Copy link
Member

akoeplinger commented Aug 7, 2023

I pushed some changes so we're only building objwriter in the source build job, it completes in ~30 mins now on AzDO.
@NikolaMilosavljevic please take one more look.

@akoeplinger akoeplinger merged commit f947ac2 into dotnet:dotnet/main-16.x Aug 7, 2023
@akoeplinger
Copy link
Member

Thanks everyone for the patience! :shipit:

@MichaelSimons
Copy link
Member

Thanks @akoeplinger for getting this in!

radekdoulik pushed a commit to radekdoulik/llvm-project that referenced this pull request May 9, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
akoeplinger added a commit that referenced this pull request May 17, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
radekdoulik pushed a commit that referenced this pull request Aug 16, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
radekdoulik pushed a commit that referenced this pull request Sep 2, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
radekdoulik pushed a commit that referenced this pull request Sep 12, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
radekdoulik pushed a commit that referenced this pull request Sep 19, 2024
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants