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

Update xz util lib to 5.4.0 #319

Merged
merged 25 commits into from
Feb 6, 2023
Merged

Update xz util lib to 5.4.0 #319

merged 25 commits into from
Feb 6, 2023

Conversation

Vertexwahn
Copy link
Collaborator

@Vertexwahn Vertexwahn commented Dec 21, 2022

  • Update xz util lib to 5.4.0
  • Add bash script to update LZMA config files

@cpsauer
Copy link
Collaborator

cpsauer commented Dec 22, 2022

Hi, @Vertexwahn! Thanks for your contributions and for proposing the update.

Looks like the CI is just some transitory misconfiguration hiccup, so let's not worry about that.

But before I merge--just double checking: Does it indeed look like there are no important changed to the xz autotools configure scripts that'd require matching changes to the config.lzma-*.h files in this repo? Maybe the easiest would be to just look quickly at their sources, but you could also run ./configure and diff. (Example script in #245)

Cheers,
Chris

@Vertexwahn
Copy link
Collaborator Author

I started to add a bash script to update LZMA config files. The config files look slightly different on my system (Ubuntu 22.04). Do I need to run "./configure" on macOS? Will this give me different headers or is configure independently of my OS (I have no experience with autotools)? Currently, the script has only the 3 configurations - is is unclear for me which parameters to use for config.lzma-windows.h, etc.

update_lzma_config_header.sh Outdated Show resolved Hide resolved
update_lzma_config_header.sh Outdated Show resolved Hide resolved
@cpsauer
Copy link
Collaborator

cpsauer commented Dec 23, 2022

@Vertexwahn, thanks for giving the script a shot. That's a great idea for making this easier to update into the future.

To answer your question: Unfortunately the ./configure output is dependent on the target OS and architecture. It's basically running a series of tests against the system to autodetect which defines should be used for compilation. I don't know offhand how xz has things set up for Windows either. It's often a special case--they should have something about it in their build docs. (I should also note: I'm a little skeptical that the linux/windows headers are currently right, given they aren't conditioned on architecture. For example, the one for linux hardcodes sizeof(size_t) as 8, but 32 bit linux/windows is perfectly valid. Not that any of this is necessarily your problem, but if you're willing to solve this stuff, it'd be great.)

Gregor's comments look like good improvements to me! I'll tap back a couple other quick ideas in a moment.

One future proofing approach--once we have these scripts--would be to run them at build time as part of a genrule. That'd keep them updated by default for all future versions and make it so that people can update without needing to run the script against all targets, which requires using multiple OSs. (The rules_foreign_cc autotools wrappers haven't been good options in my past experience, since rules_foreign_cc tends to incorrectly assume the host machine is the target. But it might have improved. If @gjasny or anyone else knows anything more about the current state of those, please say something.) Anyway, again, this certainly isn't a problem you created or need to solve, but I think that could be another good solution in the long run. If you don't want to tackle it, that's fine, too. I'd be happy to run the Apple/Android scripts for ya; I've got all those SDKs set up. And for a repo I maintain, wrapping curl, which also uses autotools, I just quickly hand-update my bazel script based on the changes I see in their source. That's viable, too, especially when there's a lot of shared code.

P.S. Re circleci...I'm not quite sure what's up. Would you be down to rebase or merge in master to see if that fixes things? If not, let's ping nelhage in case he's seen this before.

@Vertexwahn
Copy link
Collaborator Author

Vertexwahn commented Dec 23, 2022

I added all requested changes to the script.

@cpsauer
Copy link
Collaborator

cpsauer commented Dec 23, 2022

@nelhage, any idea what's up with CircleCI's config error here? It seems to be working in other PRs (like Renovate's) and @Vertexwahn has already rebased.

[@Vertexwahn, looks like it's now building correctly with the config you generated, yes?]

@Vertexwahn
Copy link
Collaborator Author

@cpsauer Building works now (I extended the BUILD.lzma file with two new source files)

update_lzma_config_header.sh Outdated Show resolved Hide resolved
update_lzma_config_header.sh Outdated Show resolved Hide resolved
update_lzma_config_header.sh Show resolved Hide resolved
update_lzma_config_header.sh Outdated Show resolved Hide resolved
update_lzma_config_header.sh Outdated Show resolved Hide resolved
@Vertexwahn Vertexwahn requested review from gjasny and cpsauer and removed request for gjasny January 9, 2023 07:35
@cpsauer
Copy link
Collaborator

cpsauer commented Feb 1, 2023

@Vertexwahn, merging w/ some quick async changes! (Would love it if you'd take a peek and lmk if you think I messed anything up.) Thanks so much for going the extra mile to add and refine the script to make future updates smoother. And more generally, I really appreciate you leaving things better than you found them. [And sorry I was so slow in getting back; got absolutely buried during the holidays.]

@Vertexwahn
Copy link
Collaborator Author

Vertexwahn commented Feb 4, 2023

@cpsauer cd test && bazel test --cxxopt=-std=c++17 //... fails on Ubuntu 22.04 with your commits. I get the following errors:

ERROR: /home/vertexwahn/.cache/bazel/_bazel_vertexwahn/cc4590d5a577b17209b16bde722a0d52/external/org_lzma_lzma/BUILD.bazel:52:11: Compiling src/liblzma/rangecoder/price_tablegen.c failed: (Exit 1): gcc failed: error executing command (from target @org_lzma_lzma//:lzma) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -MD -MF ... (remaining 43 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/org_lzma_lzma/src/liblzma/common/common.h:17,
                 from external/org_lzma_lzma/src/liblzma/rangecoder/range_common.h:17,
                 from external/org_lzma_lzma/src/liblzma/rangecoder/price_tablegen.c:18:
external/org_lzma_lzma/src/common/mythread.h:126:9: error: unknown type name 'clockid_t'
  126 |         clockid_t clk_id;
      |         ^~~~~~~~~
external/org_lzma_lzma/src/common/mythread.h:144:33: error: unknown type name 'sigset_t'
  144 | mythread_sigmask(int how, const sigset_t *restrict set,
      |                                 ^~~~~~~~
external/org_lzma_lzma/src/common/mythread.h:145:17: error: unknown type name 'sigset_t'; did you mean 'size_t'?
  145 |                 sigset_t *restrict oset)
      |                 ^~~~~~~~
      |                 size_t
external/org_lzma_lzma/src/common/mythread.h: In function 'mythread_create':
external/org_lzma_lzma/src/common/mythread.h:164:9: error: unknown type name 'sigset_t'; did you mean 'size_t'?
  164 |         sigset_t old;
      |         ^~~~~~~~
      |         size_t
external/org_lzma_lzma/src/common/mythread.h:165:9: error: unknown type name 'sigset_t'; did you mean 'size_t'?
  165 |         sigset_t all;
      |         ^~~~~~~~
      |         size_t
external/org_lzma_lzma/src/common/mythread.h:166:9: warning: implicit declaration of function 'sigfillset' [-Wimplicit-function-declaration]
  166 |         sigfillset(&all);
      |         ^~~~~~~~~~
external/org_lzma_lzma/src/common/mythread.h:168:9: warning: implicit declaration of function 'mythread_sigmask' [-Wimplicit-function-declaration]
  168 |         mythread_sigmask(SIG_SETMASK, &all, &old);
      |         ^~~~~~~~~~~~~~~~
external/org_lzma_lzma/src/common/mythread.h:168:26: error: 'SIG_SETMASK' undeclared (first use in this function)
  168 |         mythread_sigmask(SIG_SETMASK, &all, &old);
      |                          ^~~~~~~~~~~
external/org_lzma_lzma/src/common/mythread.h:168:26: note: each undeclared identifier is reported only once for each function it appears in
external/org_lzma_lzma/src/common/mythread.h: In function 'mythread_cond_init':
external/org_lzma_lzma/src/common/mythread.h:256:26: error: 'CLOCK_REALTIME' undeclared (first use in this function)
  256 |         mycond->clk_id = CLOCK_REALTIME;
      |                          ^~~~~~~~~~~~~~
external/org_lzma_lzma/src/common/mythread.h: In function 'mythread_condtime_set':
external/org_lzma_lzma/src/common/mythread.h:308:19: warning: implicit declaration of function 'clock_gettime' [-Wimplicit-function-declaration]
  308 |         int ret = clock_gettime(cond->clk_id, &now);
      |                   ^~~~~~~~~~~~~
INFO: Elapsed time: 1.714s, Critical Path: 0.23s
INFO: 87 processes: 67 remote cache hit, 12 internal, 8 linux-sandbox.

Your last commit Simplify lzma build with globs breaks the build - without this commit the compilation & testing on Ubuntu 22.04 works

@cpsauer
Copy link
Collaborator

cpsauer commented Feb 6, 2023

Hey, @Vertexwahn! Yep. Now fixed.

To explain what happened: I'd realized A separate breakage with zstd on master would have prevented my merging--and had to dash off to other things, since there'd been a few more fixups needed than anticipated. All fixed now. Merging in.

@cpsauer cpsauer merged commit 2e899d4 into nelhage:master Feb 6, 2023
rjhuijsman added a commit to 3rdparty/stout that referenced this pull request Jan 8, 2024
Since very recently, builds using this repo have started failing with
errors due to the Boost dependency:
```
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/com_github_3rdparty_stout/BUILD.bazel:62:11: @com_github_3rdparty_stout//:stout depends on @boost//:variant in repository @boost which failed to fetch. no such package '@boost//': java.io.IOException: Error downloading [https://boostorg.jfrog.io/artifactory/main/release/1.79.0/source/boost_1_79_0.tar.gz] to /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/boost/temp13387589294389186013/boost_1_79_0.tar.gz: Checksum was 5e89103d9b70bba5c91a794126b169cb67654be2051f90cf7c22ba6893ede0ff but wanted 273f1be93238a068aba4f9735a4a2b003019af067b9c183ed227780b8f36062c
```

We fix this error by upgrading to a newer version of the Boost repo. We
can't go beyond nelhage/rules_boost#319 since it
breaks our Windows support. See (TODO: link to issue).

While we're here, also add a `.bazeliskrc`, which is important to have
this repo work with our modern Codespaces.
rjhuijsman added a commit to 3rdparty/stout that referenced this pull request Jan 8, 2024
Since very recently, builds using this repo have started failing with
errors due to the Boost dependency:
```
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/com_github_3rdparty_stout/BUILD.bazel:62:11: @com_github_3rdparty_stout//:stout depends on @boost//:variant in repository @boost which failed to fetch. no such package '@boost//': java.io.IOException: Error downloading [https://boostorg.jfrog.io/artifactory/main/release/1.79.0/source/boost_1_79_0.tar.gz] to /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/boost/temp13387589294389186013/boost_1_79_0.tar.gz: Checksum was 5e89103d9b70bba5c91a794126b169cb67654be2051f90cf7c22ba6893ede0ff but wanted 273f1be93238a068aba4f9735a4a2b003019af067b9c183ed227780b8f36062c
```

We fix this error by upgrading to a newer version of the Boost repo. We
can't go beyond nelhage/rules_boost#319 since it
breaks our Windows support; see: nelhage/rules_boost#535

While we're here, also add a `.bazeliskrc`, which is important to have
this repo work with our modern Codespaces.
rjhuijsman added a commit to 3rdparty/stout that referenced this pull request Jan 8, 2024
* Fix boost import

Since very recently, builds using this repo have started failing with
errors due to the Boost dependency:
```
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/com_github_3rdparty_stout/BUILD.bazel:62:11: @com_github_3rdparty_stout//:stout depends on @boost//:variant in repository @boost which failed to fetch. no such package '@boost//': java.io.IOException: Error downloading [https://boostorg.jfrog.io/artifactory/main/release/1.79.0/source/boost_1_79_0.tar.gz] to /home/vscode/.cache/bazel/_bazel_vscode/e9b5149ef40c40531841044f5a70254d/external/boost/temp13387589294389186013/boost_1_79_0.tar.gz: Checksum was 5e89103d9b70bba5c91a794126b169cb67654be2051f90cf7c22ba6893ede0ff but wanted 273f1be93238a068aba4f9735a4a2b003019af067b9c183ed227780b8f36062c
```

We fix this error by upgrading to a newer version of the Boost repo. We
can't go beyond nelhage/rules_boost#319 since it
breaks our Windows support; see: nelhage/rules_boost#535

While we're here, also add a `.bazeliskrc`, which is important to have
this repo work with our modern Codespaces.

* Also fix MergeQueue config's windows check entry
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.

3 participants