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

Yf make shell replacement optional #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yfarjoun
Copy link

@yfarjoun yfarjoun commented Apr 5, 2021

This PR enables the use of images with complex default ENTRYPOINT arrays.

In essence, it leaves the original entry point as it by default and simply puts a "sh -c <script>" as the comment. if the user would like to remove the entry point they can put --entrypoint="" as an option.

If the user would like to not use a shell or a prefix (for example because their entry point already has that) they can redefine each of them separately.

@yfarjoun yfarjoun marked this pull request as draft April 5, 2021 14:07
@yfarjoun
Copy link
Author

yfarjoun commented Apr 5, 2021

I'm having trouble testing this because I can't echo ::set-output.... etc. as the CMD is only one string in this case.

I could repackage one of the docker images and provide it a more complex ENTRYPOINT, but I'm not sure how to do this.., any suggestions regarding the test @addnab ?

@yfarjoun yfarjoun marked this pull request as ready for review April 5, 2021 15:20
@yfarjoun
Copy link
Author

yfarjoun commented Apr 7, 2021

Any ideas on how to test, or just take it as is? @addnab

@addnab
Copy link
Owner

addnab commented Apr 7, 2021

Hi @yfarjoun I'll take a look on Friday if it's ok. I'll work on adding a test for this as well.

@yfarjoun
Copy link
Author

yfarjoun commented Apr 7, 2021

perfect! thanks. I just didn't want this to fall off your radar. I'm currently using a branch on my fork and I always prefer to move back to the "source" 😄

@yfarjoun
Copy link
Author

I think that I don't like the design of this change...I'mm working on a different one.

@yfarjoun yfarjoun closed this Apr 13, 2021
@yfarjoun yfarjoun reopened this Apr 13, 2021
@yfarjoun yfarjoun marked this pull request as draft April 13, 2021 17:17
@yfarjoun yfarjoun force-pushed the yf_make_shell_replacement_optional branch from 0d29800 to 4360d75 Compare April 13, 2021 17:51
@yfarjoun yfarjoun marked this pull request as ready for review April 13, 2021 18:12
@yfarjoun yfarjoun force-pushed the yf_make_shell_replacement_optional branch from 32fb57c to 7390192 Compare April 15, 2021 02:57
author Yossi Farjoun <[email protected]> 1617479686 -0400
committer Yossi Farjoun <[email protected]> 1618455120 -0400

Change the way the script is run. instead of modfying the ENTRYPOINT, we will simply add the "shell" (default sh) and the "script_prefix" (default -c)
and then the script.

That way the possible comple entry point is not disturbed. if a user would like to remove the ENTRYPOINT they can provide an "option" "--entrypoint ''" for example.
@yfarjoun yfarjoun force-pushed the yf_make_shell_replacement_optional branch from 77360d1 to 2f4bbe9 Compare April 15, 2021 03:01
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.

2 participants