-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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: pack checksums into fewer files, by target #38963
Conversation
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files
Disables progress bars, which are often illegible anyways with parallel make invocations.
a624753
to
75be274
Compare
# Add this guy to his project target (e.g. `make -f contrib/refresh_checksums.mk openblas`) | ||
$(1): checksum-$(1)-$(2)-$(3) | ||
# Add a dependency to the pack target | ||
# TODO: can we make this so it only adds an ordering but not a dependency? |
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 believe this TODO has been satisfied.
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.
any suggestion on how to do it?
CURL_OPTS="-fkL --connect-timeout $TIMEOUT -y $TIMEOUT" | ||
FETCH_OPTS="-T $TIMEOUT" | ||
WGET_OPTS="-nv --no-check-certificate --no-use-server-timestamps --tries=1 --timeout=$TIMEOUT" | ||
CURL_OPTS="-fkLSs --connect-timeout $TIMEOUT -y $TIMEOUT" |
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'm 70% against making these completely silent; on slow connections it may look like the build is needlessly frozen.
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 already silence much of what happens in deps. I contemplated making it conditional on the presence of --jobserver-fds=
(or -j
) in MAKEFLAGS
, but didn't seem worth it
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.
Most of what we silence doesn't depend on network speed though; A users build could potentially freeze for 2-3 minutes due to being stuck downloading an LLVM build; I think we should continue to print out download status reports to stdout.
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.
With the exception of the curl
printing, I'm happy to merge this. Nice improvement!
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files
Merged in 8af7a04 |
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files (cherry picked from commit 8af7a04)
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files (cherry picked from commit 8af7a04)
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files
Hopefully this should preserve the ability to generate these in parallel (and keep a clean tree) and prevent conflicts when updating different deps targets, while consolidating any related files (cherry picked from commit 8af7a04)
Hopefully this should preserve the ability to generate these in parallel
(and keep a clean tree) and prevent conflicts when updating different
deps targets, while consolidating any related files.
This reduces from ~1000 files to ~40, and makes it feel like this is one
of my more impressive diffs. 😂