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

bazel: Bugfix, Update to version 6.1.2; PG bazel #16149

Merged
merged 2 commits into from
May 3, 2023

Conversation

essandess
Copy link
Contributor

Description

Cherry picked from #15397.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 12.6 21G115 x86_64
Xcode 14.0 14A309

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@cjones051073 for port bazel.
@missa-prime for port bazel.

@essandess
Copy link
Contributor Author

essandess commented Sep 19, 2022

CI error:

2022-09-17 21:05:01: FATAL: bazel crashed due to an internal error

I do not observe this with a trace build. I recommend that a reviewer confirm this and merge whether or not the CI completed.

bazel is unreliable in the best of times, and at least this PR will give us a working up-to-date bazel binary. Currently, the bazel port is old and broken.

@mascguy
Copy link
Member

mascguy commented Sep 20, 2022

CI error:

2022-09-17 21:05:01: FATAL: bazel crashed due to an internal error

I do not observe this with a trace build. I recommend that a reviewer confirm this and merge whether or not the CI completed.

bazel is unreliable in the best of times, and at least this PR will give us a working up-to-date bazel binary. Currently, the bazel port is old and broken.

I'll add this to my to-do list, unless @reneeotten or someone else wants to take a look. ;-)

@essandess
Copy link
Contributor Author

I confirm that this latest version builds locally on:
macOS 12.6.1 21G217 x86_64
Xcode 14.1 14B47b

@essandess essandess marked this pull request as ready for review November 27, 2022 23:53
@essandess essandess changed the title bazel: Bugfix, Update to version 5.3.0; PG bazel bazel: Bugfix, Update to version 5.3.2; PG bazel Nov 27, 2022
devel/bazel/Portfile Outdated Show resolved Hide resolved
@essandess essandess changed the title bazel: Bugfix, Update to version 5.3.2; PG bazel bazel: Bugfix, Update to version 6.1.1; PG bazel Mar 30, 2023
Copy link
Contributor

@ryandesign ryandesign left a comment

Choose a reason for hiding this comment

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

CI failed:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.ExceptionInInitializerError
	at com.google.devtools.build.lib.actions.ParameterFile.writeContentLatin1(ParameterFile.java:141)
	at com.google.devtools.build.lib.actions.ParameterFile.writeContent(ParameterFile.java:120)
	at com.google.devtools.build.lib.actions.ParameterFile.writeParameterFile(ParameterFile.java:112)
	at com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction$ParamFileWriter.writeOutputFile(ParameterFileWriteAction.java:172)
	at com.google.devtools.build.lib.exec.FileWriteStrategy.beginWriteOutputToFile(FileWriteStrategy.java:58)
	at com.google.devtools.build.lib.analysis.actions.FileWriteActionContext.beginWriteOutputToFile(FileWriteActionContext.java:49)
	at com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction.beginExecution(AbstractFileWriteAction.java:66)
	at com.google.devtools.build.lib.actions.Action.execute(Action.java:133)
	at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$5.execute(SkyframeActionExecutor.java:957)
	at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.continueAction(SkyframeActionExecutor.java:1124)
	at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.run(SkyframeActionExecutor.java:1082)
	at com.google.devtools.build.lib.skyframe.ActionExecutionState.runStateMachine(ActionExecutionState.java:160)
	at com.google.devtools.build.lib.skyframe.ActionExecutionState.getResultOrDependOnFuture(ActionExecutionState.java:93)
	at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.executeAction(SkyframeActionExecutor.java:516)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.checkCacheAndExecuteIfNeeded(ActionExecutionFunction.java:827)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.computeInternal(ActionExecutionFunction.java:323)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.compute(ActionExecutionFunction.java:161)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:571)
	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 @297e2f87
	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:61)
	at com.google.devtools.build.lib.unsafe.StringUnsafe.<clinit>(StringUnsafe.java:36)
	... 22 more

ERROR: Could not build Bazel

devel/bazel/Portfile Show resolved Hide resolved
_resources/port1.0/group/bazel-1.0.tcl Outdated Show resolved Hide resolved
devel/bazel/Portfile Outdated Show resolved Hide resolved
@essandess
Copy link
Contributor Author

CI failed:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.ExceptionInInitializerError
	at com.google.devtools.build.lib.actions.ParameterFile.writeContentLatin1(ParameterFile.java:141)
 …
	at com.google.devtools.build.lib.unsafe.StringUnsafe.<clinit>(StringUnsafe.java:36)
	... 22 more

ERROR: Could not build Bazel

I confirm that this builds and runs on:

macOS 13.3.1 22E261 x86_64
Xcode 14.3 14E222b

Good luck tracking down whatever java thing breaks on the CI 🙃

@essandess essandess changed the title bazel: Bugfix, Update to version 6.1.1; PG bazel bazel: Bugfix, Update to version 6.1.2; PG bazel Apr 30, 2023
@catap
Copy link
Contributor

catap commented May 2, 2023

@essandess which java version have you use?

@catap
Copy link
Contributor

catap commented May 2, 2023

@essandess the root cause of issue: java PG doesn't respect java.version 11 and tries to build it on JDK17 and JDK17 is supported since 7.0.0: bazelbuild/bazel#14040

DEBUG: java-portgroup: Trying to find JVM version: 11
DEBUG: java-portgroup: Detected JVMs: 8 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.372-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home 17 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home 11 /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
DEBUG: java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
DEBUG: Discovered JAVA_HOME via /usr/libexec/java_home: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home

@catap
Copy link
Contributor

catap commented May 2, 2023

@essandess I've opened a PR into your branch which fixes CI: essandess#2

The run: https://github.com/catap/macports-ports/actions/runs/4860773891

Let unblock it!

@cjones051073
Copy link
Member

cjones051073 commented May 2, 2023

@essandess the root cause of issue: java PG doesn't respect java.version 11 and tries to build it on JDK17 and JDK17 is supported since 7.0.0: bazelbuild/bazel#14040

DEBUG: java-portgroup: Trying to find JVM version: 11
DEBUG: java-portgroup: Detected JVMs: 8 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.372-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home 17 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home 11 /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
DEBUG: java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
DEBUG: Discovered JAVA_HOME via /usr/libexec/java_home: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home

No, The java PG does its best to satisfy whatever requirements it is asked to. In this case there is I think something fishy in the environment on the CI machine causing issues.

The message

java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

Comes from

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/java-1.0.tcl#L90

and shows that an attempt to run /usr/libexec/java_home -f -v 11 successfully returned that path.

However....

That path then appears to fail a test to confirm its a valid directory, at

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/java-1.0.tcl#L105

as that is the only way the message

DEBUG: Discovered JAVA_HOME via /usr/libexec/java_home: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home

can appear, and at this point the java PG gives up trying to satisfy the requirements, as its run out of options to do so, and just uses the default, which in this case appears to be for version 17.

So the primary question is why does the path

/Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

get listed as a valid JM alternative, but then fail the isdirectory test.

I don't think there is anything wrong with the java PG here, it is just doing its best to work with the environment it is presented with.

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 yes, the issue that Github runners has 3rd party JDK. As soon as I've removed it, the CI had passed.

I think that ignoring 3rd party JDKs isn't the right move. Cleaner to remove it from Github runners.

@cjones051073
Copy link
Member

cjones051073 commented May 2, 2023

@cjones051073 yes, the issue that Github runners has 3rd party JDK. As soon as I've removed it, the CI had passed.

I think that ignoring 3rd party JDKs isn't the right move. Cleaner to remove it from Github runners.

A priori there is nothing wrong with third party JDKs, assuming they function correctly... So for me the question I ask above is still relevant, why is the JDK that is made available failing the isdirectory test ?

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 yes, the issue that Github runners has 3rd party JDK. As soon as I've removed it, the CI had passed.
I think that ignoring 3rd party JDKs isn't the right move. Cleaner to remove it from Github runners.

A priori there is nothing wrong with third party JDKs, assuming they function correctly... So for me the question I ask above is still relevant, why is the JDK that is made available failing the isdirectory test ?

Because it isn't a directory :) More of that this path doesn't exists.

See, it returns a path:

java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

and here /Home/Users appears twice. At the begining of the path, and in the moddle as .../x64/Contents/Home/Users/runner/hostedtoolcache/...

=> seems something is quite wrong with this JDK and and easy way is remove it, and use MacPorts JDK.

@cjones051073
Copy link
Member

@cjones051073 yes, the issue that Github runners has 3rd party JDK. As soon as I've removed it, the CI had passed.
I think that ignoring 3rd party JDKs isn't the right move. Cleaner to remove it from Github runners.

A priori there is nothing wrong with third party JDKs, assuming they function correctly... So for me the question I ask above is still relevant, why is the JDK that is made available failing the isdirectory test ?

Because it isn't a directory :) More of that this path doesn't exists.

See, it returns a path:

java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

and here /Home/Users appears twice. At the begining of the path, and in the moddle as .../x64/Contents/Home/Users/runner/hostedtoolcache/...

=> seems something is quite wrong with this JDK and and easy way is remove it, and use MacPorts JDK.

I do not agree with the changes you make in essandess#2 .
They might well help, by removing the problematic path, but the proper fix is is for someone to look into why the above path is being created and fix that instead. As I said, there is in principle nothing wrong with using a third party JDK as long as that JDK works.

@cjones051073
Copy link
Member

@cjones051073 yes, the issue that Github runners has 3rd party JDK. As soon as I've removed it, the CI had passed.
I think that ignoring 3rd party JDKs isn't the right move. Cleaner to remove it from Github runners.

A priori there is nothing wrong with third party JDKs, assuming they function correctly... So for me the question I ask above is still relevant, why is the JDK that is made available failing the isdirectory test ?

Because it isn't a directory :) More of that this path doesn't exists.
See, it returns a path:

java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

and here /Home/Users appears twice. At the begining of the path, and in the moddle as .../x64/Contents/Home/Users/runner/hostedtoolcache/...
=> seems something is quite wrong with this JDK and and easy way is remove it, and use MacPorts JDK.

I do not agree with the changes you make in essandess#2 . They might well help, by removing the problematic path, but the proper fix is is for someone to look into why the above path is being created and fix that instead. As I said, there is in principle nothing wrong with using a third party JDK as long as that JDK works.

So I guess my question now is who should the above buggy path be reported to ?

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 that's wired. The diff below fixes an issue, see: https://github.com/catap/macports-ports/actions/runs/4861461977

--- a/_resources/port1.0/group/java-1.0.tcl
+++ b/_resources/port1.0/group/java-1.0.tcl
@@ -165,7 +165,9 @@ namespace eval java {
         ui_debug "java-portgroup: Detected JVMs: $versions_found"
         # Match the systems JVMs with the one requested by MacPorts
         # Higher JVM Versions win
-        return [match_jvm_version $version_requested $versions_found]
+        set filtered [match_jvm_version $version_requested $versions_found]
+        ui_debug "java-portgroup: filtered JVM: $filtered"
+        return $filtered
     }

@cjones051073
Copy link
Member

Nothing IMHO needs to be changed in the java PG here. It simply relies on the system returning a valid list of paths when it executes /usr/libexec/java_home -V. The issue here is that is including a buggy path. Thats not an issue with the java PG itself, but the underlying CI environment.

... or...

its possible what is being returned is being incorrectly munged by the proc in the java PG

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/java-1.0.tcl#L171

However to judge if thats the case or not we need to see the raw output from

/usr/libexec/java_home -V

when run on the CI machine.

@catap
Copy link
Contributor

catap commented May 2, 2023

However to judge if thats the case or not we need to see the raw output from

/usr/libexec/java_home -V

when run on the CI machine.

You're reading my mind and I'm running it right now. So, here the output:

  Matching Java Virtual Machines (5):
  /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      17.0.7 (x86_64) "Eclipse Adoptium" - "OpenJDK 17.0.7" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.372-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home

=> environment is innocent and issue from java PG, or at least seems so.

@cjones051073
Copy link
Member

However to judge if thats the case or not we need to see the raw output from

/usr/libexec/java_home -V

when run on the CI machine.

You're reading my mind and I'm running it right now. So, here the output:

  Matching Java Virtual Machines (5):
  /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      17.0.7 (x86_64) "Eclipse Adoptium" - "OpenJDK 17.0.7" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.372-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home

=> environment is innocent and issue from java PG, or at least seems so.

Yeah, it looks like the logic falls over when there are mulitple paths for exactly the same version. I guess this was a scenario never expected....

So the correct fix here is to attempt to deal with this in some way...

@cjones051073
Copy link
Member

However to judge if thats the case or not we need to see the raw output from

/usr/libexec/java_home -V

when run on the CI machine.

You're reading my mind and I'm running it right now. So, here the output:

  Matching Java Virtual Machines (5):
  /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      17.0.7 (x86_64) "Eclipse Adoptium" - "OpenJDK 17.0.7" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Adopt_jdk/11.0.19-7/x64/Contents/Home
      11.0.19 (x86_64) "Eclipse Adoptium" - "OpenJDK 11.0.19" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.372-7/x64/Contents/Home
      1.8.0_372 (x86_64) "Eclipse Temurin" - "Eclipse Temurin 8" /Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home

=> environment is innocent and issue from java PG, or at least seems so.

Yeah, it looks like the logic falls over when there are mulitple paths for exactly the same version. I guess this was a scenario never expected....

So the correct fix here is to attempt to deal with this in some way...

The core issue here is the use of dict append when added new keys to the version dict

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/java-1.0.tcl#L189

If you call that twice for exactly the same key (`$vers) then you get what we seem, the path string for that version is all paths munged together.

A simple fix is to use dict set instead. Then if called multiple times for the same key you get last one wins.

@cjones051073
Copy link
Member

@catap Please can you try again with #18496

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 thanks, you was faster than me :)

It is testing right now here: https://github.com/catap/macports-ports/actions/runs/4862643389

I really think that we had found the cause and it I feel that I've encountered it once but never had motivated enough to dig.

@cjones051073
Copy link
Member

cjones051073 commented May 2, 2023

@cjones051073 thanks, you was faster than me :)

It is testing right now here: https://github.com/catap/macports-ports/actions/runs/4862643389

I really think that we had found the cause and it I feel that I've encountered it once but never had motivated enough to dig.

@catap Those builds above are still using essandess#2 as I see in the macOS13 logs messages you added there

2023-05-02T14:55:31.9724720Z ##[group]Uninstalling preinstalled JDKs
2023-05-02T14:55:31.9725400Z Moving directories...
2023-05-02T14:55:31.9907280Z /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk

Please cancel these and start them again without this MR...

... and of course instead with #18496

@catap
Copy link
Contributor

catap commented May 2, 2023

Sorry, tooo many branches!

The right one: https://github.com/catap/macports-ports/actions/runs/4862901100

@cjones051073
Copy link
Member

Sorry, tooo many branches!

The right one: https://github.com/catap/macports-ports/actions/runs/4862901100

OK, those seem to have worked. I see the warnings I would expect and a valid JVM path is now picked.

2023-05-02T15:24:10.4838430Z Warning: java-portgroup: Found multiple installations for JVM 11
2023-05-02T15:24:10.4839550Z Warning: java-portgroup: Found multiple installations for JVM 8
2023-05-02T15:24:10.4841620Z DEBUG: java-portgroup: Detected JVMs: 8 /Users/runner/hostedtoolcache/Java_Adopt_jdk/8.0.372-7/x64/Contents/Home 17 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64/Contents/Home 11 /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home
2023-05-02T15:24:10.4843350Z DEBUG: java-portgroup: Discovered matching JAVA_HOME: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.19-7/x64/Contents/Home

I am reasonably happy #18496 is an improvement over what is currently there, so I think I will merge it. One thing I do not do, as I wouldn't really know how, is make any decision on which path to use when there are multiple for the same (major) version. Currently its just whichever is the last listed by java_home -V.

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 seems that your PR works as expected

@cjones051073
Copy link
Member

cjones051073 commented May 2, 2023

@cjones051073 seems that your PR works as expected

yup. I've merged the Java PG fix and restarted the tests here.

@catap
Copy link
Contributor

catap commented May 2, 2023

@cjones051073 I've supprised that it doesn't require to be rebased

@cjones051073
Copy link
Member

cjones051073 commented May 2, 2023

Ah, i assumed the tests would do that themselves...

@cjones051073
Copy link
Member

@essandess Please rebase your branch here so we can retest.

@catap
Copy link
Contributor

catap commented May 2, 2023

Yahoo!

@mascguy
Copy link
Member

mascguy commented May 3, 2023

So are we ready to merge this then...?

@mascguy mascguy merged commit 94a4065 into macports:master May 3, 2023
3 checks passed
@mascguy
Copy link
Member

mascguy commented May 3, 2023

Just tested on 10.15/Xcode 12, and I did encounter two issues:

  • Building with ccache caused failures, so I added configure.ccache no locally. It might be something specific to my installation though, have you folks tested with ccache enabled?
  • The "Xcode Locator" component - built via file tools/osx/BUILD - specifies two arches: both arm64, and x86_64. (Plus it tries to pass linker option -no_adhoc_codesign, which doesn't appear (?) to be valid for 10.15.) It was necessary to patch that file, via the following:
--- a/tools/osx/BUILD.orig	2023-05-03 17:34:35.000000000 -0400
+++ b/tools/osx/BUILD	2023-05-03 17:43:45.000000000 -0400
@@ -29,7 +29,7 @@
 
 DARWIN_XCODE_LOCATOR_COMPILE_COMMAND = """
   /usr/bin/xcrun --sdk macosx clang -mmacosx-version-min=10.13 -fobjc-arc -framework CoreServices \
-      -framework Foundation -arch arm64 -arch x86_64 -Wl,-no_adhoc_codesign -Wl,-no_uuid -o $@ $< && \
+      -framework Foundation -arch x86_64 -Wl,-no_uuid -o $@ $< && \
   env -i codesign --identifier $@ --force --sign - $@
 """

With those two changes in play, though, bazel built successfully on 10.15. Great work you guys!

@essandess essandess deleted the bazel branch May 4, 2023 23:41
@essandess
Copy link
Contributor Author

Thanks everyone for getting this working!

mascguy added a commit to herbygillot/macports-ports that referenced this pull request May 19, 2023
mascguy added a commit that referenced this pull request May 19, 2023
@mascguy
Copy link
Member

mascguy commented May 19, 2023

Quick update: I'm now able to build Bazel for 10.14, by relaxing the Xcode/Clang minimum: Xcode 10.3/Clang 10 are sufficient, and the build looks good.

That change was included in Herby's update to Bazel 6.2, via PR: #18601

As for py-tensorflow, builds fail for 10.15 and earlier, with error "argument list too long." Currently reviewing potential upstream fixes for that, which might involve patching Bazel. But making progress!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
6 participants