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

Pass full command name to executables #136

Merged
merged 7 commits into from
Aug 27, 2024
Merged

Conversation

jdevera
Copy link
Contributor

@jdevera jdevera commented Jun 23, 2024

Use the ${$PREFIX}_COMMAND_NAME environment variable to pass the full command to the executable files when they are run.

The full command would be the app name, followed optionally by the group name, and finally the command name that was configured in the manifest to execute the command.

This can be used, for instance, by programs to output a meaningful usage message that refers to how the program was actually executed.

jdevera-hj and others added 2 commits June 23, 2024 20:47
Use the ${$PREFIX}_COMMAND_NAME environment variable to pass the full
command to the executable files when they are run.

The full command would be the app name, followed optionally by the group
name, and finally the command name that was configured in the manifest
to execute the command.

This can be used, for instance, by programs to output a meaningful usage
message that refers to how the program was actually executed.
Copy link
Contributor

@bhou bhou left a comment

Choose a reason for hiding this comment

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

Please rename the command to avoid ambiguity, and handle the situations that user renamed the binary itself.

gh-pages/content/en/docs/overview/resources.md Outdated Show resolved Hide resolved
internal/context/context.go Outdated Show resolved Hide resolved
internal/frontend/default-frontend.go Outdated Show resolved Hide resolved
internal/frontend/default-frontend.go Outdated Show resolved Hide resolved
test/README.md Show resolved Hide resolved
The program can be renamed after building, and the COMMAND_NAME variable
should contain the actual name, rather than the built time name.

Still resort to the build time name if the program name cannot be
obtained.
@jdevera
Copy link
Contributor Author

jdevera commented Aug 24, 2024

Thanks for the review!

In my second round of changes, I:

  • Renamed the variable to ${PREFIX}_FULL_COMMAND_NAME
  • Loaded it with the actual name of the executable for the launcher (still with a fallback to the configured name at build time in case of errors)
  • Added the variable to the .bat file to fix windows tests
  • Changed the test to run on a renamed binary, so it also checks that the actual name is used

@jdevera jdevera requested a review from bhou August 24, 2024 12:16
Copy link
Contributor

@bhou bhou left a comment

Choose a reason for hiding this comment

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

The environment variable name in .bat script is not correct, which fails the integration test

test/packages-src/bonjour/bonjour.bat Outdated Show resolved Hide resolved
@jdevera jdevera requested a review from bhou August 26, 2024 13:23
@bhou bhou merged commit e82d0ff into criteo:main Aug 27, 2024
8 checks passed
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