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

feat: export an additional env var pointing to the location of jbang #1811

Merged
merged 3 commits into from
Jul 6, 2024

Conversation

dsyer
Copy link
Contributor

@dsyer dsyer commented Jun 20, 2024

It's really hard (impossible?) to find the location of the script that launched the JVM from within jbang. It would be really useful to be able to do that so you can run jbang again with some logic to precompute the command line. This works really simply:

$ jbang --interactive
WARNING: Using incubator modules: jdk.incubator.vector, jdk.incubator.foreign
|  Welcome to JShell -- Version 17.0.7
|  For an introduction type: /help intro

jshell> System.getenv("JBANG")
$1 ==> "/home/dsyer/.sdkman/candidates/jbang/current/bin/jbang"

Edit: I don't know how to do this for Windows users. Anyone got an idea?

It's *really* hard (impossible?) to find the location of the script that
launched the JVM from within jbang. It would be really useful to be able
to do that so you can run jbang again with some logic to precompute the
command line. This works really simply:

```
$ jbang --interactive
WARNING: Using incubator modules: jdk.incubator.vector, jdk.incubator.foreign
|  Welcome to JShell -- Version 17.0.7
|  For an introduction type: /help intro

jshell> System.getenv("JBANG")
$1 ==> "/home/dsyer/.sdkman/candidates/jbang/current/bin/jbang"
```
@dsyer dsyer changed the title Export an additional env var pointing to the location of jbang feat: export an additional env var pointing to the location of jbang Jun 20, 2024
@maxandersen
Copy link
Collaborator

I'm not totally against exporting this variable assuming we find way it works on windows but Thus far we haven't exposed this as its not guaranteed to work.

i.e. the same app could have been run with pure java or native exc launched.

I would prefer some more internal name like JBANG_LAUNCH_SCRIPT or similar.

@dsyer
Copy link
Contributor Author

dsyer commented Jun 20, 2024

I don't care about the name really. How about JBANG_LAUNCHER? I get that it's not guaranteed to work if the launcher isn't used, but at least it provides a way to launch jbang in the case that the launcher is used (which is the majority I suspect). I can't think of anything that would work completely generically.

@maxandersen
Copy link
Collaborator

okey, so I pushed code that should work on osx, linux and windows (need ci to verify the latter one).

I also checked what other env vars we actually expose and added tests for those ...BUT I can't actually spot where we use them.. @quintesse can we remove those?

@@ -0,0 +1,13 @@
Feature: env variables

Scenario: JBANG_RUNTIME_SHELL available
Copy link
Collaborator

Choose a reason for hiding this comment

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

@quintesse what is the use of this variable?

Copy link
Contributor

@quintesse quintesse Jun 27, 2024

Choose a reason for hiding this comment

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

The runtime scripts tells JBang (Java code) what shell (eg. *nix sh/bash/etc or CMD or Powershell) it was started from. There's just no good way to otherwise detect what kind of command line we should be generating.

When command('jbang env@jbangdev jbang')
Then match err contains "^JBANG_RUNTIME_SHELL"

Scenario: JBANG_STDIN_NOTTY available
Copy link
Collaborator

Choose a reason for hiding this comment

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

@quintesse what is the use of this variable?

Copy link
Contributor

@quintesse quintesse Jun 27, 2024

Choose a reason for hiding this comment

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

This tells JBang (Java code) that we don't have a console/terminal connected to stdin. Java can't detect this by itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this is a pretty good approximation, in case you ever need it:

boolean isAnsi = System.console()!=null && 
  System.getProperty("os.name").toLowerCase(Locale.ENGLISH).contains("win");

Copy link
Contributor

Choose a reason for hiding this comment

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

@dsyer that would only work on *nix systems and we need it to work for Windows too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But where in code are we using that env var?

Copy link
Contributor

Choose a reason for hiding this comment

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

See Util.haveConsole() which is used by ConsoleInput.get() which is used by Util.askInput() to detect if we can ask the user for input on the console or if we should use a GUI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

kk

When command('jbang env@jbangdev jbang')
Then match err contains "^JBANG_STDIN_NOTTY"

Scenario: JBANG_LAUNCH_CMD available
Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually prefer if we call this _JBANG_LAUNCH_CMD but kept with JBANG_ as we use that above already.

Reason beeing we actually have a few JBANG_* env var used for input - so wanted _JBANG to signal its not input just temporarily available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I said I didn't care about the name, but I don't like starting env var names with non-alpha chars, so I vote for JBANG_LAUNCH_CMD.

@maxandersen maxandersen merged commit a2ef5a7 into jbangdev:main Jul 6, 2024
11 checks passed
@maxandersen
Copy link
Collaborator

@all-contributors please add @dsyer for code

Copy link
Contributor

@maxandersen

I've put up a pull request to add @dsyer! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants