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

Rebuild for abseil_cpp20220623 (redux) #138

Merged
merged 4 commits into from
Aug 23, 2022

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Aug 15, 2022

Bot had a resolution error; tackle main before taking on #134/#135

Closes #141
Closes #145

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

So both the previous(ly working) as well as the new abseil build fail with something java-related:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.ExceptionInInitializerError
	at com.google.devtools.build.lib.actions.ParameterFile.writeContent(ParameterFile.java:118)
        [...]
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:382)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make java.lang.String(byte[],byte) accessible: module java.base does not "opens java.lang" to unnamed module @18494e8a
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:188)
	at java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:181)
	at com.google.devtools.build.lib.unsafe.StringUnsafe.<init>(StringUnsafe.java:75)
	at com.google.devtools.build.lib.unsafe.StringUnsafe.initInstance(StringUnsafe.java:56)
	at com.google.devtools.build.lib.unsafe.StringUnsafe.<clinit>(StringUnsafe.java:37)
	... 28 more

ERROR: Could not build Bazel

@h-vetinari
Copy link
Member Author

This works now CC @conda-forge/bazel @hmaarrfk

I prepared PRs (#139, #140) for the backports as well.

@h-vetinari
Copy link
Member Author

In this PR I still left abseil-cpp to pick up both the current and the old abseil version (will switch in follow-up). In the backport PRs I went straight for the new builds.

@ngam
Copy link

ngam commented Aug 17, 2022

I think the likely cause for the java failure is actually the way we pass the commands to the custom toolchain...

Copy link

@ngam ngam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regardless of the java thing, this should be merged sooner rather than later. Thanks!!

I say this because somehow the pinning allows for different abseils to exist in the host_env and build_env, see logs in e.g. conda-forge/tensorflow-feedstock#264 and conda-forge/jaxlib-feedstock#130

@h-vetinari
Copy link
Member Author

I say this because somehow the pinning allows for different abseils to exist in the host_env and build_env,

I don't have a systematic answer to this (and it's definitely just my lack of insight and not necessarily a shortcoming), but I've seen such things before as well. Different deps in the different environments might lead to different resolutions vis-à-vis unfinished migrations. The solution is to specify the respective dep (in this case abseil) in the build env as well, so it gets populated by the same value from the pinning.

Introducing this might still run into resolution errors if not all deps of the build env are migrated yet

@h-vetinari
Copy link
Member Author

I do remember seeing arcane build options like merge_build_host: true, which presumably avoids this, but I've never had to use it, so I don't know if it's an appropriate tool.

@ngam
Copy link

ngam commented Aug 17, 2022

I am not sure it is problem per se...

In both cases above, things look fine with mixing the host and build. conda-forge/jaxlib-feedstock#130 looks completely fine. I will test the packages more thoroughly later locally before merging. Also the current error in conda-forge/tensorflow-feedstock#264 looks likely to be about missing flatbuffers in run (not really declared as a run dependency upstream, so idk what's going on)

@ngam
Copy link

ngam commented Aug 20, 2022

@conda-forge/bazel any outstanding issues here? Could we merge this soon?

I am blocking both conda-forge/jaxlib-feedstock#130 and conda-forge/tensorflow-feedstock#264 waiting for this. They both can pass with the bazel built with older abseil, but I think that's dodgy, and we should build with the newer bazel. I do not have appetite to rebuild them again later due to some bizarre missing symbols (like we always end up doing...)

I am blocking them by adding abseil-cpp to build, and so it must satisfy that abseil-cpp and bazel are built with the same pin. Interestingly, tensorflow no longer builds successfully with the older abseil, see detailed errors in conda-forge/tensorflow-feedstock#264 and conda-forge/tensorflow-feedstock#260 (wildly enough, it fails after it is done compiling). Again, could get around this by taking abseil-cpp out of the build section, but I am afraid this is a recipe for disaster... and I'd rather wait for this to go through.

@ngam
Copy link

ngam commented Aug 20, 2022

@conda-forge-admin, please rerender

@ngam
Copy link

ngam commented Aug 20, 2022

We can have a go at unpinning jdk later if you want. It will require substantial edits to to build.sh to restructure the bootstrapping process.

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Aug 23, 2022
@github-actions github-actions bot merged commit c7e1570 into conda-forge:main Aug 23, 2022
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@h-vetinari h-vetinari mentioned this pull request Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@conda-forge-admin, please add user @h-vetinari
3 participants