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

De-duplicate run args passed to bazel image targets. #2184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

psigen
Copy link
Contributor

@psigen psigen commented Nov 6, 2022

When 'bazel run' is called, it expects that the args specified in the BUILD file are not known to the run command and will prepend them to all other arguments.

However, we have already baked those commands into the image, because users expect that 'args' in the case of an image target will be included in the image. This templated script de-duplicates the args specified in the BUILD file by passing itself the number of args already included in the image.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #2124

In the case of an image target being passed arguments, the arguments will actually be passed to the image
target by bazel twice, once at build time and another at runtime. This interferes with the expected order of
arguments (because command arguments are inserted before docker run arguments)

What is the new behavior?

The new behavior should match the documentation.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

When 'bazel run' is called, it expects that the args specified in the BUILD
file are not known to the run command and will prepend them to all other
arguments.

However, we have already baked those commands into the image, because users
expect that 'args' in the case of an image target will be included in the
image.  This templated script de-duplicates the args specified in the BUILD
file by passing itself the number of args already included in the image.
@psigen
Copy link
Contributor Author

psigen commented Nov 6, 2022

Please, if anyone has a good way to add unit tests for this, please help me add them

@github-actions
Copy link

github-actions bot commented May 6, 2023

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_docker!

@github-actions github-actions bot added the Can Close? Will close in 30 days unless there is a comment indicating why not label May 6, 2023
@psigen
Copy link
Contributor Author

psigen commented May 6, 2023

Hmm, I will try to rebase this to bring it up to date

@github-actions github-actions bot removed the Can Close? Will close in 30 days unless there is a comment indicating why not label May 7, 2023
Copy link

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_docker!

@github-actions github-actions bot added the Can Close? Will close in 30 days unless there is a comment indicating why not label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? Will close in 30 days unless there is a comment indicating why not
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant