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

[DO NOT MERGE] rust: revert to bootstrapping=yes for clang and depend on llvm-libs #17540

Closed
wants to merge 1 commit into from

Conversation

filnet
Copy link
Contributor

@filnet filnet commented Jun 14, 2023

No description provided.

@filnet
Copy link
Contributor Author

filnet commented Jun 14, 2023

Follow up on #17406.

@filnet
Copy link
Contributor Author

filnet commented Jun 14, 2023

package_llvm-libs() {
  pkgdesc="Low Level Virtual Machine Runtime Libraries (mingw-w64)"
  depends=("${MINGW_PACKAGE_PREFIX}-libffi"
           "${MINGW_PACKAGE_PREFIX}-gcc-libs"
           "${MINGW_PACKAGE_PREFIX}-zlib"
           "${MINGW_PACKAGE_PREFIX}-libxml2"
           "${MINGW_PACKAGE_PREFIX}-zstd")
[...]

So we should remove libffi and zlib from makedepends ?

makedepends=("${MINGW_PACKAGE_PREFIX}-cmake"
             "${MINGW_PACKAGE_PREFIX}-curl"
             "${MINGW_PACKAGE_PREFIX}-libffi"
             "${MINGW_PACKAGE_PREFIX}-libssh2"
             "${MINGW_PACKAGE_PREFIX}-lldb"
             "${MINGW_PACKAGE_PREFIX}-llvm-libs"
             "${MINGW_PACKAGE_PREFIX}-ninja"
             "${MINGW_PACKAGE_PREFIX}-openssl"
             "${MINGW_PACKAGE_PREFIX}-python"
             "${MINGW_PACKAGE_PREFIX}-autotools"
             $([[ "$_bootstrapping" == "no" ]] && echo "${MINGW_PACKAGE_PREFIX}-rust")
             "${MINGW_PACKAGE_PREFIX}-zlib")

@ognevny
Copy link
Collaborator

ognevny commented Jun 15, 2023

So we should remove libffi and zlib from makedepends ?

yes, it should be ok, but it's not necessary

@ognevny
Copy link
Collaborator

ognevny commented Jun 15, 2023

tested this package from CLANG64 artifact. compiled rustpython and my own project successfully

@lazka
Copy link
Member

lazka commented Jun 17, 2023

So we should remove libffi and zlib from makedepends ?

split package deps are ignored when building.

@jeremyd2019
Copy link
Member

So we should remove libffi and zlib from makedepends ?

split package deps are ignored when building.

I think this was comparing the depends of llvm-libs against the makedepends of rust, so they would be considered (as the depends of a makedepends), so I think it comes down to whether rust directly makes use of the dependencies or if they are only needed because llvm-libs needs them.

@jeremyd2019
Copy link
Member

jeremyd2019 commented Jun 26, 2023

I'm kind of surprised that llvm-libs is sufficient as a makedepends, I kind of thought the split between llvm and llvm-libs was runtime vs buildtime. At least llvm-config, the static libraries, and the llvm includes are part of the llvm package not llvm-libs. Maybe it's falling back to using the bundled llvm instead of the system one? In fact, rust usually statically links llvm, so llvm-libs shouldn't really be relevant for it.

@filnet
Copy link
Contributor Author

filnet commented Jun 26, 2023

So we should remove libffi and zlib from makedepends ?

split package deps are ignored when building.

I think this was comparing the depends of llvm-libs against the makedepends of rust, so they would be considered (as the depends of a makedepends), so I think it comes down to whether rust directly makes use of the dependencies or if they are only needed because llvm-libs needs them.

Yes, I was comparing the dependencies of llvm-libs to the makedepends of Rust.
The idea was to not have in the makedepends things that are provided by llvm-libs.

@@ -23,7 +23,7 @@ makedepends=("${MINGW_PACKAGE_PREFIX}-cmake"
"${MINGW_PACKAGE_PREFIX}-libffi"
"${MINGW_PACKAGE_PREFIX}-libssh2"
"${MINGW_PACKAGE_PREFIX}-lldb"
"${MINGW_PACKAGE_PREFIX}-llvm"
"${MINGW_PACKAGE_PREFIX}-llvm-libs"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe @mati865 or somebody who knows rust better could weigh in here, but I don't think this change is a good idea. Reasoning in prior comment ☝️

Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't right, build will fail without llvm-config.
So why did it succeed? I suppose lldb pulled in clang which pulled in llvm.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I guess then a case could be made that clang should depend on llvm-libs instead of llvm now (assuming it doesn't need any of that other stuff at runtime), but definitely not changing rust's makedepends.

Copy link
Member

Choose a reason for hiding this comment

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

We kinda depend on "cc" pulling in a complete toolchain in various places, including cli tools, so I'm not sure that's a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the consensus is to not change the makedepends at all.

@filnet filnet closed this Jul 11, 2023
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.

5 participants