bazel: add patches to compile with gcc-11 by glennpj · Pull Request #28548 · spack/spack

glennpj · 2022-01-21T23:43:41Z

Bazel does not currently build with GCC-11. There are patches available on the bazel project site for Alpine Linux that should work. The problem is that java-tools also needs patching.

adamjstewart · 2022-01-23T21:00:42Z

Based on #27138 (comment) I'm guessing that this wasn't enough to solve the issue?

var/spack/repos/builtin/packages/bazel/package.py

adamjstewart · 2022-02-18T20:20:10Z

#29016 adds Bazel 5, might be worth looking at. If that fixes GCC 11 build issues I vote we close this.

glennpj · 2022-05-16T01:17:14Z

My apologies for this PR languishing so long. I committed another set of changes to address older versions.

Versions less than 4 can also be patched to get those versions to compile with newer GCC versions.
Version 4.0.0 is kind of a special case. As commented in the package.py file, it needs newer versions of grpc and abseil. I am not sure how to handle that. For now, that version is in a conflicts statement.

#29016 adds Bazel 5, might be worth looking at. If that fixes GCC 11 build issues I vote we close this.

Bazel 5 does build without the patches. I allowed for that in the constraints settings of this PR. I do not think we should close this though as this gets bazel 3 to build with newer GCC versions.

Note: I could not build versions @:0.16 so I marked those deprecated.

adamjstewart · 2022-05-16T17:19:56Z

Also see #30655

adamjstewart · 2022-05-18T04:50:12Z

So, I need Bazel 4.1+ for Apple M1 support, and we have a few open Bazel PRs now, including #29016 and #30655. I'm no longer maintaining the Bazel package, which means that no one is maintaining the package, which means that no one cares if the package breaks. So I'm inclined to just start merging things and ask questions later. This PR seems to encompass most (all?) of #30655, and #29016 hasn't gotten a response since I reviewed it. So I'm thinking about just merging this and closing #30655 and #29016. Any objections from @elfprince13 or @haralmha? Any volunteers to officially maintain the package?

haralmha · 2022-05-18T07:37:47Z

@adamjstewart sounds good to me. We are building the bazel based packages from wheels now anyway.

adamjstewart · 2022-05-18T20:14:11Z

Tried building this on macOS 12.3.1 and Apple M1 Pro with Apple Clang 13.1.6 and JDK 18.0.1.1 but it doesn't build for me. Here is the build log: spack-build-out.txt. Any idea why this doesn't work?

elfprince13 · 2022-05-18T20:15:52Z

I can check tomorrow if this solves my issue.

adamjstewart · 2022-05-19T00:37:39Z

Also tried Bazel 5.1.1 but I also saw build issues. Maybe my JDK version is too new?

elfprince13 · 2022-05-27T19:50:03Z

Just tried building Bazel 4.2.2 and this also needs a patch for:

  >> 72881    external/com_google_absl/absl/synchronization/internal/graphcycles.cc:451:26: error: no member named 'numeric_limits' in namespace 'std'
     72882      if (x->version == std::numeric_limits<uint32_t>::max()) {
     72883                        ~~~~~^
  >> 72884    external/com_google_absl/absl/synchronization/internal/graphcycles.cc:451:41: error: unexpected type name 'uint32_t': expected expression
     72885      if (x->version == std::numeric_limits<uint32_t>::max()) {
     72886                                            ^
  >> 72887    external/com_google_absl/absl/synchronization/internal/graphcycles.cc:451:52: error: no member named 'max' in the global namespace
     72888      if (x->version == std::numeric_limits<uint32_t>::max()) {
     72889                                                     ~~^
     72890    3 errors generated.
     72891    [935 / 2,844] 7 actions running
     72892        JavacBootstrap .../devtools/build/buildjar/libstarlark-deps.jar; 48s local
     72893        Compiling src/google/protobuf/descriptor.cc; 12s local

glennpj · 2022-05-30T18:21:52Z

Just tried building Bazel 4.2.2 and this also needs a patch for:

@elfprince13 This PR patches that already. Version 4.2.2 builds fine for me.

adamjstewart · 2022-07-06T04:31:19Z

@glennpj when you get a chance can you rebase and make sure your patches still apply to all versions? @elfprince13 and I can test this and get it merged soon.

var/spack/repos/builtin/packages/bazel/package.py

adamjstewart · 2022-08-02T21:34:55Z

Need to rebase/merge master. Probably want to run @spackbot fix style before that though.

spackbot-app · 2022-08-02T21:34:57Z

Let me see if I can fix that for you!

adamjstewart · 2022-08-02T21:35:17Z

Oops, didn't intend to run that for you...

spackbot-app · 2022-08-02T21:36:32Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  var/spack/repos/builtin/packages/bazel/package.py
==> Running isort checks
  isort checks were clean
==> Running mypy checks
Success: no issues found in 555 source files
  mypy checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/bazel/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
var/spack/repos/builtin/packages/bazel/package.py:124: [E501] line too long (137 > 99 characters)
  flake8 found errors

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

glennpj · 2022-08-02T21:41:34Z

@spackbot fix style

spackbot-app · 2022-08-02T21:41:36Z

Let me see if I can fix that for you!

spackbot-app · 2022-08-02T21:43:08Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  var/spack/repos/builtin/packages/bazel/package.py
==> Running isort checks
  isort checks were clean
==> Running mypy checks
Success: no issues found in 555 source files
  mypy checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/bazel/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
var/spack/repos/builtin/packages/bazel/package.py:124: [E501] line too long (137 > 99 characters)
  flake8 found errors

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

- deprecate versions 0.16 and lower as they do not build - versions older than 0.22 depend on java@8 - adjust when patches are applied - adjust gcc conflicts directive

glennpj · 2022-08-03T16:04:07Z

So, spackbot adds a conflicts label but does not remove it when conflicts are resolved.

adamjstewart · 2022-08-03T16:09:03Z

The conflicts label is for usage of the conflicts directive in a package, not for merge conflicts. Maybe we should remove that label, I agree that it's kind of confusing.

glennpj · 2022-08-08T18:30:26Z

The conflicts label is for usage of the conflicts directive in a package, not for merge conflicts. Maybe we should remove that label, I agree that it's kind of confusing.

The label referring to package conflicts makes perfect sense, but my brain did associate it with the merge conflicts at the time. Perhaps package-conflicts or something similar.

adamjstewart · 2022-08-08T18:32:03Z

Is this PR good to go now? @elfprince13 can you test this and make sure it solves your problems?

adamjstewart

Added a commit to fix some issues I noticed during patching. With these changes, all patches now apply successfully to all versions. I haven't actually tested building older bazel with newer GCC, but I'll leave that part to someone else to test.

adamjstewart · 2022-08-09T00:52:53Z

var/spack/repos/builtin/packages/bazel/gcc11_2.patch

@@ -0,0 +1,14 @@
+diff --git a/third_party/ijar/zlib_client.h b/third_party/ijar/zlib_client.h


Any reason not to combine 1 and 2 into a single patch?

No, the reason they are separate was just to be consistent with the source the patches were derived from. That is also why they are named the way they are. I thought that would be the most clear way to track what is going on.

aweits · 2022-08-10T12:06:46Z

Added a commit to fix some issues I noticed during patching. With these changes, all patches now apply successfully to all versions. I haven't actually tested building older bazel with newer GCC, but I'll leave that part to someone else to test.

works over here....

[aweits@skl-a-00 spack]$ spack find [email protected] %[email protected]
==> 1 installed package
-- linux-rhel7-skylake_avx512 / [email protected] ----------------------
[email protected]

elfprince13 · 2023-09-19T17:30:08Z

Is this PR good to go now? @elfprince13 can you test this and make sure it solves your problems?

It didn't but #40084 should get us the rest of the way there.

spackbot-app bot added conflicts new-version patch update-package labels Jan 21, 2022

spackbot-app bot requested a review from adamjstewart January 21, 2022 23:43

glennpj marked this pull request as draft January 22, 2022 05:50

adamjstewart reviewed Jan 23, 2022

View reviewed changes

var/spack/repos/builtin/packages/bazel/package.py Show resolved Hide resolved

glennpj mentioned this pull request Feb 7, 2022

Installation issue: [email protected] via [email protected] #28799

Closed

4 tasks

spackbot-app bot added the dependencies label May 16, 2022

glennpj marked this pull request as ready for review May 16, 2022 01:28

glennpj requested a review from adamjstewart May 16, 2022 01:29

alalazo assigned adamjstewart May 16, 2022

adamjstewart mentioned this pull request May 16, 2022

Fix older bazels on new GCCs #30655

Closed

This was referenced Jul 1, 2022

Bazel: new versions, patches, conflicts, and java version contraints #31402

Merged

Installation issue: py-tensorflow on apollo80 with gcc #21190

Closed

adamjstewart mentioned this pull request Jul 13, 2022

Better OS/arch support for ML libraries #31549

Open

25 tasks

glennpj force-pushed the bazel branch from 0126674 to da69e11 Compare July 31, 2022 01:23

glennpj mentioned this pull request Jul 31, 2022

TensorFlow/Keras: add versions 2.8–2.9 #31615

Merged

adamjstewart reviewed Jul 31, 2022

View reviewed changes

var/spack/repos/builtin/packages/bazel/package.py Outdated Show resolved Hide resolved

var/spack/repos/builtin/packages/bazel/package.py Outdated Show resolved Hide resolved

spackbot-app bot added the new-variant label Aug 2, 2022

glennpj and others added 3 commits August 2, 2022 16:57

bazel: new versions and patches to compile with gcc-11

7941d06

Get older versions to build

4d47150

- deprecate versions 0.16 and lower as they do not build - versions older than 0.22 depend on java@8 - adjust when patches are applied - adjust gcc conflicts directive

[@spackbot] updating style on behalf of glennpj

c6b7c6d

glennpj force-pushed the bazel branch from 3abca5c to c6b7c6d Compare August 2, 2022 22:17

adamjstewart changed the title ~~bazel: new versions and patches to compile with gcc-11~~ bazel: add patches to compile with gcc-11 Aug 9, 2022

Ensure that patches apply to all versions

2e67174

adamjstewart approved these changes Aug 9, 2022

View reviewed changes

adamjstewart reviewed Aug 9, 2022

View reviewed changes

adamjstewart merged commit 4d06dcd into spack:develop Aug 11, 2022

glennpj deleted the bazel branch August 11, 2022 02:05

greenc-FNAL mentioned this pull request Sep 9, 2022

Changes required for Cetmodules dependencies. FNALssi/spack#31

Merged

		@@ -0,0 +1,14 @@
		diff --git a/third_party/ijar/zlib_client.h b/third_party/ijar/zlib_client.h

Conversation

glennpj commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Jan 23, 2022

Uh oh!

Uh oh!

adamjstewart commented Feb 18, 2022

Uh oh!

glennpj commented May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented May 16, 2022

Uh oh!

adamjstewart commented May 18, 2022

Uh oh!

haralmha commented May 18, 2022

Uh oh!

adamjstewart commented May 18, 2022

Uh oh!

elfprince13 commented May 18, 2022

Uh oh!

adamjstewart commented May 19, 2022

Uh oh!

elfprince13 commented May 27, 2022

Uh oh!

glennpj commented May 30, 2022

Uh oh!

adamjstewart commented Jul 6, 2022

Uh oh!

Uh oh!

Uh oh!

adamjstewart commented Aug 2, 2022

Uh oh!

spackbot-app bot commented Aug 2, 2022

Uh oh!

adamjstewart commented Aug 2, 2022

Uh oh!

spackbot-app bot commented Aug 2, 2022

Uh oh!

glennpj commented Aug 2, 2022

Uh oh!

spackbot-app bot commented Aug 2, 2022

Uh oh!

spackbot-app bot commented Aug 2, 2022

Uh oh!

glennpj commented Aug 3, 2022

Uh oh!

adamjstewart commented Aug 3, 2022

Uh oh!

glennpj commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Aug 8, 2022

Uh oh!

adamjstewart left a comment

Choose a reason for hiding this comment

Uh oh!

adamjstewart Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

glennpj Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

aweits commented Aug 10, 2022

Uh oh!

elfprince13 commented Sep 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

glennpj commented Jan 21, 2022 •

edited

Loading

glennpj commented May 16, 2022 •

edited

Loading

glennpj commented Aug 8, 2022 •

edited

Loading