-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
bump(main/abseil-cpp): 20250127.0 #23518
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
Conversation
f559bb4 to
82e3651
Compare
thunder-coding
left a comment
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 do mention in multiple places that we don't support partial upgrades. So adding guards against it is just making it smell more imo.
|
Should you also add
When I build and then install the
Similarly, what about Maybe you mean to keep the number of packages built in a single PR limited, and then create a second PR afterward to rebuild the rest of the packages, if so then never mind, that would make sense. |
TomJo2000
left a comment
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.
python-grpcio doesn't seem to want to build right now.
packages/abseil-cpp/build.sh
Outdated
| TERMUX_PKG_AUTO_UPDATE=false | ||
| TERMUX_PKG_DEPENDS="libc++" | ||
| TERMUX_PKG_CONFLICTS="libgrpc (<< 1.52.0-1)" | ||
| TERMUX_PKG_BREAKS="termux-gui-c (<= 0.1.3-1), python-pyarrow (<= 19.0.0), python-onnxruntime (<= 1.20.2), python-grpcio (<= 1.70.1), protobuf-static (<= 25.1-3), mosh (<= 1.4.0-11), mosh-perl (<= 1.4.0-11), libwebrtc-audio-processing (<= 1.3-1), libre2 (<= 2024.07.02-1), libprotobuf-c (<= 1.5.1), libprotobuf (<= 2:25.1-3), libutf8-range (<= 2:25.1-3), protobuf-dev (<= 2:25.1-3), protobuf (<= 2:25.1-3), libncnn (<= 20230627-6), libgrpc (<= 1.70.1), libarrow-cpp (<= 19.0.0), et (<= 6.2.9), android-tools (<= 35.0.2)" |
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.
Yeah I don't know how I feel about this one.
That's gonna be a pain in the ass to keep updated and as Thunder already mentioned,
we explicitly do not support partial upgrades already so this smells.
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.
Ok, I'll remove 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.
But what about version requirement in TERMUX_PKG_DEPENDS, should I keep 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.
I'd say no.
Something akin to an SOVERSION guard in Abseil's build script might be a more sustainable solution.
That raises the question though, what constitutes a "major" version for Abseil?
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 do not think there is one after they switched to date-based versioning.
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'd need to do some branch parsing by the looks of it.
https://abseil.io/about/releases
It is weird, it is built fine locally. I'll try to build in local docker environment. |
|
Seems like everything is fine if I rebuild only abseil-cpp and python-grpcio everything is fine, but if I rebuild everything the error from the CI logs appears. Most likely some other package pollutes environment. |
|
Throw |
Anyway I want to investigate this for a while. |
|
Yeah I'm already planning another round of build testing, and with input from @robertkirkman we're gonna be expanding the scope of that testing to cover more build edgecases. |
The error is internally: It also affected someone's MacOS here in a slightly different situation. The minimum reproducible number and order of packages to build to reproduce the problem is: scripts/run-docker.sh ./build-package.sh -I -f libprotobuf python-grpcioIt is some kind of a conflict between
https://github.com/grpc/grpc/tree/master/third_party/upb/upb It is currently copied from protobuf 29, but It seems like Coincidentally, it must have something to do with headers in causes the error to go away. |
|
Oh, I was close to this. I had this as a theory and wanted to test this practically. I'll rebase the PR. |
82e3651 to
ae2de52
Compare
I will try to make a script that I might be able to run locally to generate the entire spreadsheet automatically, I'm not sure how long that will take though. |
ae2de52 to
933159a
Compare
|
CI building fails because of lack of disk space but local building is fine. Probably I will split it in two parts. @TomJo2000 How do you think, can we implement cleaning up $TERMUX_TOPDIR/*/{src,cache,tmp} to obtain some disk space in the case if there is no space available to avoid fails like this in the future? |
That should be a safe bet after any build. |
I do not think so. Termux supports continuing building ( |
|
Yes, I think that it is ok to delete I think that it might be possible to get a lot of extra free space by doing that, because that is where the extracted source code of packages and the intermediate build artifacts are, and for some packages, that is what takes up the largest amount of space. I am not sure whether it is easy to insert a step like this in between the build of every package in the CI workflow, though, without making some modification to |
|
Probably it will look like |
In the case of deleting the whole |
|
The cached packages in particular are very important to keep since it makes it a lot quicker to rebuild a dependency and its dependents in a batch. |
|
Yeah, that is the point of my cleanup step. |
|
Ok yes it is good then that your method does not delete Personally, my opinion is that a fixed value, and not a percentage, for example "less than 4 Gigabytes", should be used for the condition, because, if this code happens to run on local builds as well as CI, then in the event that the If there is a way that this can be made to only run in CI, though, or if you believe that using the percentage is overall better than a fixed value for a different reason, then maybe the percentage value is best. |
Not true for big packages like clvk (2.2GB), gdal (227MB), libarrow-cpp (264MB), python-grpcio (2.2GB), python-onnxruntime (2.6GB), wasm-component-ld (225MB), etc. It is only a few examples of what I currently see on my disk, the value in parentheses is a size of directory in
True. I will replace disk usage command checker with something like |
I am sorry I think that you misunderstood me, I was not clear in explaining what I meant. Stated clearly, I was only trying to say:
|
|
Oh, now I get it. Sorry for misunderstanding. |
|
I will open new PR with implementing new cleanup step and updating abseil-cpp along with its reverse dependencies after taking a nap and some testing. |
In this PR I am also making sure apt will not allow installation of incompatible packages.
I will merge this if nobody minds in 24 hours or after it gets 2+ approves.
CC @licy183 @thunder-coding because you commited there too.