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 https://github.com/nelhage/rules_boost/commit/02de303226645c8209eddef11ed0af4137540a46 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!

[sfc-gh-sgiesecke]

It turns out the approach with __has_include doesn't work everywhere. With clang on aarch64, the header actually does exist, but it just includes an #error directive.

We indeed switched to build lzma using rules_foreign_cc now.

[cpsauer]

Thanks for following up. Would you be willing to give back and contribute your fix? (Could also run the configure script to generate the header and then build with Bazel? Up to you.)

[wijagels]

Could this be resolved the same way osx arm64 is handled? Just need a good config.h generated on linux arm64, condition the linux config on os + arch instead of just os.

[cpsauer]

I think so (esp for Android). Though distos might differ? The benefit being that Bazel still does the build.

The downside is that people would need to manually regenerate all config.h to update dependencies, which is a problem bc not a lot of people have all platforms handy. Could either generate the config.h using ./configure on build (if needed for distro differences) or add a CI step that checks for the right config across platforms and makes differences copy/pastable?

[For non-Android Linux w/ good package managers built in, part of me still thinks we should just use their binary distributions and save build time and hassle.]

[sfc-gh-sgiesecke]

Would you be willing to give back and contribute your fix?

I don't have a patch at hand (we just have a BUILD.bazel file for lzma in our own repo now), but it's essentially this:

filegroup(
    name = "all_sources",
    srcs = glob(["**"]),
)

configure_make(
    name = "lzma",
    configure_options = [
        "--disable-lzmadec",
        "--disable-lzmainfo",
        "--disable-lzma-links",
        "--disable-xz",
        "--disable-xzdec",
        "--enable-shared=no",
    ],
    lib_source = ":all_sources",
    out_static_libs = [
        "liblzma.a",
    ],
    targets = [
        "install",
    ],
    visibility = ["//visibility:public"],
)

Could this be resolved the same way osx arm64 is handled? Just need a good config.h generated on linux arm64, condition the linux config on os + arch instead of just os.

While this might work, I think that's far from ideal, because you're pinning what the configure call produces in some specific environment (toolchain, ...). It's worse on Linux than on OS X probably, because there's a lot more variety between distros.

[sfc-gh-sgiesecke]

The "right thing" to do would be to re-implement what configure does in BUILD.bazel. But that's probably complex and will need to follow changes in lzma. I think using foreign_rules_cc is much more maintainable. Not sure if @sfc-gh-rwoodbury or @sfc-gh-jmerino have other thoughts on this.

[cpsauer]

kk. to echo back, sounds like the expectations is that Linux distros differ enough that we'd want to run configure per machine. (In contrast with, say, Android or Apple platforms where the goal is to produce portable binaries that run over all machines with the same OS.)

Thoughts on just generating config.h with configure and then building with Bazel?

[cpsauer]

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

^ Would still love an answer on this one. It'll help me guide things towards being good for you and other Linux users

[sfc-gh-sgiesecke]

sounds like the expectations is that Linux distros differ enough that we'd want to run configure per machine.

Yes, I don't think that can generally work without using something like https://snapcraft.io/ (which isn't without drawbacks either).

Thoughts on just generating config.h with configure and then building with Bazel?

I am not sufficiently proficient in Bazel to really judge that option, but doesn't this still incur the drawbacks that using rules_foreign_cc would have AND that you can't be certain that the Bazel build definition is in sync with the original one (for aspects beyond what's in config.h).

What's the underlying draw for you guys to hermeticity vs. just reproducibility?

Sorry I skipped over that question, but again, I can't really answer this in depth. We don't have reproducibility (which would, aside from a lot of other work, require to eliminate any time differences from sneaking into the build result). I am not entirely sure how hermeticity and reproducibility are related. Probably hermeticity is a precondition for "portable" reproducibility.