homebrew-core
homebrew-core copied to clipboard
xdk 0.4.2 (new formula)
Note: was previously Homebrew#104232
- [x] Have you followed the guidelines for contributing?
- [x] Have you ensured that your commits follow the commit style guide?
- [x] Have you checked that there aren't other open pull requests for the same formula update/change?
- [x] Have you built your formula locally with
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting? - [x] Is your test running fine
brew test <formula>
, where<formula>
is the name of the formula you're submitting? - [x] Does your build pass
brew audit --strict <formula>
(after doingbrew install --build-from-source <formula>
)? If this is a new formula, does it passbrew audit --new <formula>
?
I have not been able to get this to pass "brew test" for environment reasons mentioned on a previous thread. Everything else seems to be working well, so my hope is that the environment issue is specific to my workstation.
Goodness gracious:
Error: Final newline missing.
The first time I submitted a formula, I got this error. So I added a final newline and re-submitted the formula, and I got a different error stating that the newline should not be there. I can't tell if this is a joke or something; it should either be an error to have the final newline, or it should be an error to not have the final newline, but both of those should not be an error.
Goodness gracious:
Error: Final newline missing.
The first time I submitted a formula, I got this error. So I added a final newline and re-submitted the formula, and I got a different error stating that the newline should not be there. I can't tell if this is a joke or something; it should either be an error to have the final newline, or it should be an error to not have the final newline, but both of those should not be an error.
it is not a joke, it is a code style alert. (adding a newline would be a simple fix)
also this formula does not build on m1 machines.
Hi Rui, I am running on an M1 (2021 MBP M1 Max). What error are you seeing on an M1? Or is this something you saw on the testing infrastructure? Sorry if these questions seem silly; I am still learning this process.
This is the same error that I got locally:
/opt/homebrew/Cellar/xdk/0.4.2/bin/xec: line 2: /bin/java: No such file or directory
Could someone help me to figure out how ${JAVA_HOME}
is blank? It only happens in brew test
mode, and it happens in the shell script created by Homebrew that explicitly avoids the possibility of ${JAVA_HOME}
being blank:
#!/bin/bash
JAVA_HOME="${JAVA_HOME:-/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home}" exec "${JAVA_HOME}/bin/java" ...
The script is created with this line of Ruby code in the Formula:
java = "${JAVA_HOME}/bin/java"
...
(bin/cmd).write_env_script java, args, Language::Java.overridable_java_home_env
Since I don't full understand what brew test
is doing under the covers, is it possible that it's creating a different shell script somewhere in a temporary test location that does NOT match what I see in /opt/homebrew
? And that when it does so, it somehow loses the ${JAVA_HOME}
and creates a bogus script? Is there some other way that the script should be invoking Java, instead of ${JAVA_HOME}/bin/java
?
--
Regarding the other issue flagged: Why would shipping universal binaries be a problem?
Unexpected universal binaries were found.
Some time back, we switched to universal binaries for macOS to try to make people's lives easier. Why would we want to switch back to the 1990s model? We have one distribution image, and that has to cover Windows, macOS, and Linux.
Could someone help me to figure out how ${JAVA_HOME} is blank?
Because it's never set by CI.
Regarding the other issue flagged: Why would shipping universal binaries be a problem?
Because it's not needed. Homebrew knows how to ship the correct binaries for the correct architecture. Why work around that optimization?
Because it's never set by CI.
But that is simply not true. (Hence the large amount of time that I have spent struggling with this.)
On my local macOS machine (both M1 and x86) and Linux machines (both M1 and x86), $JAVA_HOME
has a value.
Furthermore, the script produced by Homebrew using write_env_script
explicitly encodes a literal value for $JAVA_HOME
, just in case it is blank. It uses the ":-" shell feature. That seems like it is supposed to be a fail-safe!
Yet, on my machine, where I can see both $JAVA_HOME
(with a value) and the text of the script produced by Homebrew with a default literal value for $JAVA_HOME
, the brew test
has no $JAVA_HOME
.
The script that Homebrew emits has this line:
JAVA_HOME="${JAVA_HOME:-/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home}" exec "${JAVA_HOME}/bin/java" ...
If I change it manually to:
JAVA_HOME="${JAVA_HOME:-/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home}" exec "${JAVA_HOME:-/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home}/bin/java" ...
Then it works. But that seems super wrong (and non-obvious how) to do that from my Formula, though. So I did that from vi
after the brew test
failed. I am just trying to be super clear here: The syntax used by Homebrew in the Homebrew-generated portion of script emitted by write_env_script
does not work. It does not work on my machine. It does not work on the CI machine. That seems like an obvious bug.
After my fifth double espresso so far today, this is the solution that I am planning to push:
JAVA_HOME="${JAVA_HOME:-/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home}" exec "${JAVA_HOME:+$JAVA_HOME/bin/}java" ...
Just to be clear: I have changed the formula (in a completely non-obvious way) in order to compensate for the way that the brew test
carefully erases the $JAVA_HOME
variable, and the Homebrew-emitted script incorrectly fills in as "" -- even though the literal value is pasted by Homebrew into the script as a default, but due to the bug in the automatically emitted code, it does not "stick"!
I believe that this is the same problem that I saw a few years back when Bash changed how sourcing (and export
) works. My hypothesis is that the setting of the JAVA_HOME
variable (by the Homebrew emitted code) in the script is lost by the time that the script advances to the "exec" command! If I can find a general solution to this bug, I'll propose it.
I have pushed up to my fork, but I do not know what I need to do to update the pull request. Is it automatic? Here is the latest push to my fork:
https://github.com/xtclang/homebrew-core/commit/0febb797c3f3474c70d322db3ccaffe67dbc5dcd
The brew test
is now working locally.
--
homebrew$ brew test xdk
==> Testing xdk
==> /opt/homebrew/Cellar/xdk/0.4.2/bin/xec -help
homebrew$ brew audit --strict --online xdk
homebrew$
homebrew PR flow does not allow merge from master
, and you should use some branch other master
for creating the PR. Can you rebase to the upstream master and squash the commits to do a forced update? Then I can pull the commit for my local testing.
I figured out that it had to be in the master branch of the fork, because apparently that is how I set up the original pull request. Glad to see that all the tests are passing now.
@SMillerDev I have submitted some clarifying changes. Hopefully this clears things up, in terms of why I was unable to use the write_jar_script
. My use case may be fairly uncommon, but if it is not, I could imagine an optional parameter being added to write_jar_script
allowing program arguments to be appended after the jar name.
One other question, which I could not figure out despite reading through much code. Why did I have to do the chmod
myself, when the various helper methods (like write_jar_script
) do not? The shell scripts that I emit in the bin directory are not marked as executable, unless I explicitly chmod
them.
Hopefully this clears things up, in terms of why I was unable to use the write_jar_script.
The current version seems to do the same as my suggestion did, just with an explicit max memory size and much harder to maintain. What errors are you getting when you use my last suggestion?
Why did I have to do the chmod myself, when the various helper methods (like write_jar_script) do not?
Because they exist to help write scripts, it's already part of the code.
The current version seems to do the same as my suggestion did, just with an explicit max memory size and much harder to maintain. What errors are you getting when you use my last suggestion?
I appreciate that you are giving of your time to help me, so I want to be very respectful of that. I definitely do not want you to be using your scarce time to test and/or debug my code, or to have to learn and understand each of the technical nuances of arcane b.s. that I must know to do my job. I am more than happy to answer your questions, but I do not want to waste your time.
Here are the errors from your script suggestion:
Error: xdk-test: /Users/cameron/Development/homebrew-xvm/xdk-test.rb:16: syntax error, unexpected tIDENTIFIER, expecting end
.../"bin".write_jar_script libexec/"javatools/javatools.jar", c...
... ^~~~~~~
/Users/cameron/Development/homebrew-xvm/xdk-test.rb:16: syntax error, unexpected ',', expecting end
...exec/"javatools/javatools.jar", cmd, "#{cmd} -L #{libexec}/l...
... ^
/Users/cameron/Development/homebrew-xvm/xdk-test.rb:16: syntax error, unexpected '\n', expecting '.' or &. or :: or '['
/Users/cameron/Development/homebrew-xvm/xdk-test.rb:37: syntax error, unexpected end, expecting end-of-input
But there was an easy fix:
(libexec/"bin").write_jar_script ...
And that produces the following exec
line, if I cat
out the shell script:
exec "${JAVA_HOME}/bin/java" xec -L /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/lib -L /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/javatools -jar "/opt/homebrew/Cellar/xdk-test/0.4.2/libexec/javatools/javatools.jar" "$@"
Then, when I try to run the xec
command, for example, it produces:
zsh: permission denied: xec
OK, this is really weird: The shell script is not marked as executable. (I asked about this ... I couldn't figure out who was supposed to be marking the script as executable. See above.) Anyway, it was easy enough to see that problem and fix it from the shell:
homebrew$ ls -al /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/bin/xec
-rw-r--r-- 1 cameron admin 326 Jun 27 15:14 /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/bin/xec
homebrew$ chmod +x /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/bin/xec
homebrew$ ls -al /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/bin/xec
-rwxr-xr-x 1 cameron admin 326 Jun 27 15:14 /opt/homebrew/Cellar/xdk-test/0.4.2/libexec/bin/xec
homebrew$
And now we can run the commands (e.g. "xec") and see that they are completely broken:
homebrew$ xec
Error: Could not find or load main class xec
Caused by: java.lang.ClassNotFoundException: xec
With the java
command, and specifically when using the -jar
option, the order that the various things appear on the command line is critically important; you can see this when you type java --help
:
java [options] -jar <jarfile> [args...]
Any bits to the left of the -jar
option get gobbled by the JVM as JVM options, while everything to the right of the -jar
option is passed to the public static void main(String[])
method as arguments. I very carefully broke these out in my latest formula so you would see these details related to the order:
exec "#{java_bin}" #{java_opts} -jar #{java_jar} #{cmd} -L #{libexec}/lib -L #{libexec}/javatools "$@"
^^^^
Specifically, the portion #{cmd} -L #{libexec}/lib -L #{libexec}/javatools "$@"
needs to go to the main method as arguments.
Please trust me that I do not want to place my own cut&paste garbage into my formula; I would much rather use helper functions provided by the Homebrew project if they meet the requirements for what my formula must do. I have been reading and learning many thousands of lines of Homebrew code just to understand what is going on behind the scenes, when things do not work the way that I need them to. (And I am contributing back documentation edits to try to help others who may fall into the same holes that I did, because having to read all of the code to understand what is going on is not efficient.)
Let me know if you have more questions or suggestions.
Is there some "next step" for getting this pull request approved? This is my first time doing this, so I am not sure what to expect w.r.t. to the timeline, etc.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
I'm still curious if there is something that I'm supposed to do to get this in.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
I just realized that even though this was "approved" on this thread, that it never got merged. Was there something that I had to do to make that happen?
I just realized that even though this was "approved" on this thread, that it never got merged. Was there something that I had to do to make that happen?
mostly there are review comments need to be addressed.
@cpurdy do you mind creating a new one?
Yes, I can do that. I will try to "update" a few things accordingly this time, though, to avoid some of the issues we had last time around. We're in the middle of a few big time-suck projects ATM, so this will probably take a few days/weeks to get rolling.