Skip to content
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

build_llvm_release.bat: add tarball export to x64 release #79840

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

bamiaux
Copy link
Contributor

@bamiaux bamiaux commented Jan 29, 2024

Like linux releases, export a tar.xz files containing most llvm tools, including non toolchain utilities, llvm-config, llvm-link and others.

We do this by reconfiguring cmake one last time at the last step, running the install target so we do not need to recompile anything.

Fix #51192
Fix #53052

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@bamiaux
Copy link
Contributor Author

bamiaux commented Jan 29, 2024

A few thoughts:

  • linux archives add flang & mir to built projects. I didn't change this.
  • lld.lib is not distributed in this tarball, but I need plugin support, as done in this project https://github.com/jamesmth/llvm-project. Maybe I should open a new issue ?
  • I didn't add any x86 or arm64 tarball. It's easy to do but I thought I should get some feedback & maybe finalize x64 first ?

Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay, this got buried in my inbox.

Thanks for sending the PR! People keep asking for this, so I think it's worth a try if people think it's useful.

I tried your patch and ended up with a 936 MB .tar.xz file :) Uncompressed it's 4.4 GB.

.tar.xz is perhaps an unusual choice on Windows. On the other hand, a zip file would be much larger. I tried with 7zip's "maximum" setting and got a 1.34 GB file. (With "ultra" it's 1.33 GB so that doesn't seem worth it.)
Do you think users would prefer a zip file even though it's a bit bigger?


:: generate tarball with install toolchain only off
set filename=clang+llvm-%version%-x86_64-pc-windows-msvc
cmake -GNinja %cmake_flags% -DLLVM_INSTALL_TOOLCHAIN_ONLY=OFF ^
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also keep %cmake_profile_flags% from above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

-DCMAKE_INSTALL_PREFIX=%build_dir%/%filename% ..\llvm-project\llvm || exit /b 1
ninja install || exit /b 1
:: check llvm_config is present & returns something
%build_dir%/%filename%/bin/llvm-config.exe --bindir
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to || exit /b 1 to make it fail if it doesn't exist/works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

@bamiaux
Copy link
Contributor Author

bamiaux commented Feb 21, 2024

Apologies for the delay, this got buried in my inbox.

No worries

Thanks for sending the PR! People keep asking for this, so I think it's worth a try if people think it's useful.

I tried your patch and ended up with a 936 MB .tar.xz file :) Uncompressed it's 4.4 GB.

For reference clang+llvm-17.0.6-x86_64-linux-gnu-ubuntu-22.04.tar.xz is 951 MB which is in the same ball park

.tar.xz is perhaps an unusual choice on Windows. On the other hand, a zip file would be much larger. I tried with 7zip's "maximum" setting and got a 1.34 GB file. (With "ultra" it's 1.33 GB so that doesn't seem worth it.) Do you think users would prefer a zip file even though it's a bit bigger?

I don't think it matters much, the file format is supported by 7z, and this tarball is primarily intended for developers.
If we change it, I'd prefer to have a 7z archive on windows & keep the file size as small as possible.

@bamiaux
Copy link
Contributor Author

bamiaux commented Feb 21, 2024

Let me know if you prefer to have a squashed PR

Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I don't think it matters much, the file format is supported by 7z, and this tarball is primarily intended for developers.

All right, let's stick with .tar.xz.

Let me know if you prefer to have a squashed PR

No, that's fine. We'll squash when merging.

Do you have commit access or would you like me to merge this for you?

Like linux releases, export a tar.xz files containing most llvm tools,
including non toolchain utilities, llvm-config, llvm-link and others.

We do this by reconfiguring cmake one last time at the last step,
running the install target so we do not need to recompile anything.

Fix llvm#51192
Fix llvm#53052
@bamiaux
Copy link
Contributor Author

bamiaux commented Feb 22, 2024

Thanks for the review !

Just in case, I've squashed & rebased it on latest upstream/main
I don't have commit access, so, yes, please merge it for me when you find the time.

Do you know whether this new archive can be added to the latest stable (17.0.6) or the next release (18.1.0-rc3 o rc4) ?

@zmodem zmodem merged commit 52ada07 into llvm:main Feb 23, 2024
3 of 4 checks passed
@zmodem
Copy link
Collaborator

zmodem commented Feb 23, 2024

Do you know whether this new archive can be added to the latest stable (17.0.6) or the next release (18.1.0-rc3 o rc4) ?

I've added it to the 18.1.0-rc3 release (see also https://discourse.llvm.org/t/llvm-18-1-0-rc3-tagged/77116/4) and will upload the new tarball for future release builds also.

qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Mar 21, 2024
…28432c2ad

Local branch amd-gfx cb52843 Merged main:e8740d4eb1c88e968b155f73ac745f80b4681589 into amd-gfx:35293f0c8e63
Remote branch main 52ada07 build_llvm_release.bat: add tarball export to x64 release (llvm#79840)
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.

llvm installer on windows missing LLVMConfig.cmake Include llvm-link into Windows distribution
2 participants