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

Have option for vendir sync to run in different directory. #75

Closed
GrahamDumpleton opened this issue Jun 22, 2021 · 8 comments
Closed

Have option for vendir sync to run in different directory. #75

GrahamDumpleton opened this issue Jun 22, 2021 · 8 comments
Labels
carvel-accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request

Comments

@GrahamDumpleton
Copy link

Describe the problem/challenge you have

When working with packages and where the vendir.yml file is located in a sub directory, it is extra work to ensure that you first cd into the directory before running vendir sync. If running command manually or scripting actions working across multiple packages, means have to use something like the following to ensure left in original directory when command done.

(cd package/foo/bundle; vendir sync)

Describe the solution you'd like

Would like to see a command line option where can tell vendir sync the directory to run in. Eg.,

vendir sync -c package/foo/bundle

The c for option indicative of "change directory".

Thus vendir sync would then itself change the working directory of the process to the specified directory before taking any action.

Anything else you would like to add:

N/A

cc @jorgemoralespou


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@GrahamDumpleton GrahamDumpleton added carvel-triage This issue has not yet been reviewed for validity enhancement This issue is a feature request labels Jun 22, 2021
@ewrenn8
Copy link
Contributor

ewrenn8 commented Jul 6, 2021

To clarify a few scenarios:

-c config in -c dir -f result
yes no no error
yes yes no use in dir
yes no yes use -f
yes yes yes use -f
no - - do as before

Does that line up with what you would expect in this cases?

@GrahamDumpleton
Copy link
Author

Hmmm, that seems to complicate things unnecessarily, and doesn't capture the important thing about how relative paths are calculated with -f.

My simple rule would have been that if -c is specified, change to that working directory first before evaluating any other options for paths.

Thus if you use -f as well, then any relative path supplied to it would then be evaluated relative to the directory you changed to.

So:

vendir -c somedir -f relative/path

would be the same as having said:

(cd somedir; vendir -f relative/path)

So yes the table is sort of correct, but misses detail about what happens with relative paths supplied to -f.

@cppforlife
Copy link
Contributor

do we have some precedent in other tools to do something like this? tar does have -C that does something related.

@jorgemoralespou
Copy link

There's many other tools that support this model as well as many that don't. Is it a requirement that other tools do the same or that is proven to be convenient?

@cppforlife
Copy link
Contributor

There's many other tools that support this model

im just not familiar with any except tar. do you have some concrete examples (especially for commonly used tools)?

Is it a requirement that other tools do the same

it's not a requirement, but it's definitely an important part of research. id like to see if other tools do it because -C affects all other flags that take in files. it also might be good to understand what makes vendir special as compared to other tools (i hope this wont be a pattern we add to all others). additionally (cd package/foo/bundle; vendir sync) seems like a pretty standard pattern in bash scripts, so adding this feature to eliminate need for subshell seems a bit too special.

@jorgemoralespou
Copy link

jorgemoralespou commented Jul 29, 2021

im just not familiar with any except tar. do you have some concrete examples (especially for commonly used tools)?

It's not so common because typically tools don't need to work relative to the current directory. Most tools do support specify the root of the "context" of where actions will take place. As an example, kubectl does this when you do kubectl -f dir/sub-dir and many other tools do follow this practice, but since vendir (like any of the other compressing tools, zip, ...) will take current dir as root to use, then they do provide an alternate flag.

Another tool I can think of, as developer that I use is [odo](https://github.com/openshift/odo) that provides 2 flags:

odo create -h 
      --context string                                   Use given context directory as a source for component settings
      --devfile string                                    Path to the user specified devfile

It's not specific that the tool changes directory but that can specify the root of where actions should take place.

(cd package/foo/bundle; vendir sync) is a common pattern that is nice for all those ops people that can do that. tar, zip and all other tools could have done the same pattern, but decided that their users probably didn't need to be advanced enough to know about subshells, etc... Also, providing the functionality from within the tool, prevents different behaviour in different OSes, and simplify scripts.

@cppforlife cppforlife added carvel-accepted This issue should be considered for future work and that the triage process has been completed and removed carvel-triage This issue has not yet been reviewed for validity labels Aug 13, 2021
@cppforlife
Copy link
Contributor

added --chdir flag: 85f693d. will be included in upcoming release.

@cppforlife
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
carvel-accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request
Projects
None yet
Development

No branches or pull requests

4 participants