-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Update CSL #40287
Update CSL #40287
Conversation
macOS says:
|
This adds the `.so` files, particularly for libatomic that we need on FreeBSD
…talled Most commonly csl, llvm-tools, and clang, but there were other ones in the wrong lists also.
@@ -472,7 +472,7 @@ source-dist: | |||
# Make tarball with Julia code plus all dependencies | |||
full-source-dist: light-source-dist.tmp | |||
# Get all the dependencies downloaded | |||
@$(MAKE) -C deps getall NO_GIT=1 | |||
@$(MAKE) -C deps getall DEPS_GIT=0 USE_BINARYBUILDER=0 |
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.
We specify USE_BINARYBUILDER=1
and USE_BINARYBUILDER=0
to generate two different kinds of full-source-dist
tarballs. You shouldn't set it here unconditionally.
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.
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 think you misunderstand the target definitions here. The make -C get
target would be what is used to grab all of the binary files needed to build on this machine. The getall
target grabs all of the source files needed to build on any machine. The getall
target therefore logically cannot be used with USE_BINARYBUILDER=1. I could add $(MAKE) -f contrib/refresh_checksums.mk
here though, conditional on USE_BINARYBUILDER=1
, if you like.
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.
We have customers that are using things like make -C deps getall USE_BINARYBUILDER=1 USE_BINARYBUILDER_XXX=0
to generate a tarball that they can deploy to CI and patch with their own patches to deps and such without having to rebuild LLVM, for instance. What would you suggest they use instead?
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.
They should use get
(or perhaps extract
), which is the first stage of the build pipeline (or the second, which also supports adding custom patches after). The getall target is not part of the build pipeline, but is instead part of the uninstall / cleanall set, which are a list of all possible targets, not just those that currently need to be built locally.
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.
okay; once this is merged let's double-check that the generated tarballs look right.
Update; this is not working as intended. The "with binarybuilder" tarball is bundling the source versions of dependencies. :/ |
So you want us to add a call to refresh_checksums? |
I don't think this has anything to do with refresh_checksums; when running |
Bump; this still needs to be fixed. |
It was unclear what you wanted. Do you want a lite-source-dist to include binaries? Do you want a full-binarysouces-dist that had some binaries and some sources? Do you just want install dist to provide binaries? |
The old behavior was for |
It sounds like you want either |
I think you're trying to use |
|
I just tested this, and it's exactly what it used to do. Perhaps it's not what was intended, but it's what happens. Just do a |
Ah, yes, checking though the version history, it seems that was a regression in v1.5 relative to when I added it in 01654af. Whereas, the description in the Makefile has rather cryptically said (since e85a1a5):
where, by observation of the list, "all targets" was not conditional |
* deps: upgrade CSL This adds the `.so` files, particularly for libatomic that we need on FreeBSD * build: define uninstall targets even for deps that are not always installed Most commonly csl, llvm-tools, and clang, but there were other ones in the wrong lists also.
* deps: upgrade CSL This adds the `.so` files, particularly for libatomic that we need on FreeBSD * build: define uninstall targets even for deps that are not always installed Most commonly csl, llvm-tools, and clang, but there were other ones in the wrong lists also.
* deps: upgrade CSL This adds the `.so` files, particularly for libatomic that we need on FreeBSD * build: define uninstall targets even for deps that are not always installed Most commonly csl, llvm-tools, and clang, but there were other ones in the wrong lists also.
No description provided.