Fixing lzma configuration

[axililsz]

Build will be failed for lzma at arm64 platform.

external/org_lzma_lzma/src/liblzma/common/memcmplen.h:19:11: fatal error: immintrin.h: No such file or directory

[sfc-gh-mheimel]

We are hitting the same problem, this is currently preventing us from updating to a newer boost version. Does anyone have a workaround for this?

[sfc-gh-sgiesecke]

This seems to have been broken by 02de303 which changed this line in config.lzma-linux.h

#undef HAVE_IMMINTRIN_H

to

#define HAVE_IMMINTRIN_H 1

I generally wonder how a platform-agnostic config.h can work here.

This was also noted in #319:

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.

[cpsauer]

Hey, guys. Agreed. I merged that PR since it seemed like an incremental improvement, but I totally agree about the (preexisting) underlying problems around configuration--which is why I wrote that part you quoted.

I don't currently have bandwidth to really roll up my sleeves and fix, so I'm going to have to ask you guys experiencing the problem to explore fixes. (Sorry--would truly like to help more but am just overloaded, am not currently using linux/these compression libraries, and am just here to help out occasionally where I have comparative advantage.)

Some thoughts about how we might eliminate the issue:

• What do you guys think about instructing people to instead install lzma via their distro's package manager, and linking against that?
  • That is, have linux[debian] users sudo apt-get install liblzma-dev and then adding a -llzma, taking a runtime dependency on liblzma5.
  • The reasoning being: LZMA is popular enough that I'd think it'd be widely and reliably available across the built-in package managers. (Please double check yours!)
  • Elsewhere, e.g., with zlib, we take the approach of dynamically linking against libraries the OS can reliably provide [and bundling (building + statically linking) otherwise]. Linux is a special case because there's (usually) a great dependency-management system built in, so it can reliably provide more libraries than are shipped with the OS.
  • [More on this over in bazel-make-cc-https-easy, where we take this approach for libcurl.]
• [Another option, if we really still want to independently build from source on linux would be to call into auto tools via rules_foreign_cc. I'd think it's work on linux for linux, but I think the above approach might be better.]

Quick fixes--if you want to plug the gap in the meantime--since we shouldn't let the perfect be the enemy of the good:

• If undef'ing HAVE_IMMINTRIN_H would indeed unblock you guys, want to toss up a quick PR doing that, and I'll merge? My one ask would be that you at least experiment with whether conditionally defining it with __has_include would solve things for your case while also keeping intel intrinsics for non-arm. Something like

#if defined __has_include
  #if __has_include (<immintrin.h>)
    #define HAVE_IMMINTRIN_H 1
  #endif
#endif

maybe also testing that the following (inner block) works, just to make sure you're hitting the __has_include block OK.

#if __has_include (<immintrin.h>)
  #define HAVE_IMMINTRIN_H 1
#endif

That said, the above would be a stopgap that would leave real problems. Please also respond to the above!

Cheers,
Chris

[sfc-gh-sgiesecke]

@cpsauer Thanks for looking into this.

We're not blocked on this, we're currently patching this locally as a workaround.

What do you guys think about instructing people to instead install lzma via their distro's package manager, and linking against that?

That's not an option, we're aiming at a fully hermetic build. (CC @sfc-gh-rwoodbury)

to call into auto tools via rules_foreign_cc

We're doing that for other dependencies as well. I am not entirely sure about the drawbacks of this (again CC @sfc-gh-rwoodbury). While we have statically generated config headers for some dependencies ourselves, I think it's hard to provide these for a wider scope, where the build environment is not sufficiently constrained.

We'll check out if

#if __has_include (<immintrin.h>)
  #define HAVE_IMMINTRIN_H 1
#endif

works.

It seems there's no non-Intel architecture Linux CI job for rules_boost. Is it feasible to add one?

[sfc-gh-sgiesecke]

This seems to work:

 #if __has_include (<immintrin.h>)
  #define HAVE_IMMINTRIN_H 1
#endif

[cpsauer]

Great; thanks for checking out the temporary workaround. I just PR'd and merged the full version (backlinked above) so you can stay on the mainline, unpatched. (Or if you have more patches, please PR them!)

I'll change the title of this issue to reflect the change in scope towards more generally fixing lzma configuration on linux.

We'd really appreciate your help and expertise switching liblzma[linux] to rules_foreign_cc--or otherwise calling their build scripts directly--to prevent things like this in the future, and make things more correct! (And sorry again that this issue pre-exists. Wish I had the bandwidth to just fix for you.)

For my continued learning: What's the underlying draw for you guys to hermeticity vs. just reproducibility?

[cpsauer]

It seems there's no non-Intel architecture Linux CI job for rules_boost. Is it feasible to add one?

Realized I forgot to reply to this part. Sure! I'll add to .bazelci.

[cpsauer]

^ The ARM Linux CI change is ready to go in #390, but it looks like there are actually more fixes needed to build lzma on ARM Linux, beyond the cpu.h __has_include I quickly added. See errors in the bazelci on #390 (ignoring circleci).

I don't have the power to merge things that don't pass CI, so if y'all move things over to rules_foreign_cc (and/or upstream the patches you use to build successfully), then ping me I'll merge that ARM Linux testing PR!