cli: add --cd flag to change directories before executing commands

[bobertlo]

Fixes #4025

I was initally going to use --cwd and remove --cwd from repro, but that left repro with the very strange args.c value. So I turned to the tar manpage for inspiratoin and found the long form of -C is --cd. Tar is good enough for me.

I actually ran into this problem yesterday trying to launch dvc from ssh for a dumb reason.

I also added a docs PR:
treeverse/dvc.org#1780

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

[bobertlo]

This is always initialized with os.curdir if not given a value. Alternative would be if self.args.cwd != os.curdir

[pared]

@bobertlo Looks good to me, lets please wait for some more reviews, as it impact all commands.

[pmrowla]

Functionality-wise this LGTM, but it looks a bit strange for us to have different naming conventions for dvc -C/--cd and dvc repro -c/--cwd. Seems to me that we should be using the same set of short/long flags for both of those options.

[bobertlo]

@pmrowla the idea is not breaking existing functionality. With arparse, having --cwd on the global level would break --cwd functionality on the repro command. Also -C is much more standard than -c in this context.

I've seen at least one other issue where replacing argparse would be beneficial but otherwise this is the best I could think of so far?

I think the audience using -c/--cwd is limited enough that backward compatibility would be sufficient?

I could implement other solutions but it would either be whole thing or break compatibility.

[pmrowla]

w/argparse you can get around the naming conflict by specifying the dest variable name for either one of the conflicting options (so internally we would just have something like args.cwd and args.repro_cwd), but still keep consistent usage in terms of what the user would see.

I'm not sure how many DVC users actually use (or are even aware of) repro -c/--cwd, but I still think it looks odd to have two different sets of option flags that have the same effect. (maybe deprecating the repro option is something we should consider)

[bobertlo]

@pmrowla thanks for the great info on argparse. When I started working on this I saw #4116 and assumed the repro arguments would be left for compatibility and deprecated. I was trying to be as unobtrusive as possible but do feel -C is a much more intuitive namespace for this than -c.

The PR as is should not break any current functionality but I would be happy to make changes if a consensus is reached.

[efiop]

Sorry for the delay.

This looks great! Just 2 minor comments:

1. How about we only keep --cd and don't use -C for now? It seems like this option is for a pretty specific advanced usage, so taking up a nice short -C option feels a bit wasteful. We could always consider bringing it back in the future.

2. Great point about repro -c! Let's mark it as obsolete in dvc and docs right now and we'll drop it in 2.0

[bobertlo]

Ok. I will get on this ASAP.

1. Sure. I only chose -C because (iirc) it was mentioned in the original issue? Also, -C has a strong precedent here from that flags use in both tar and make. I like the flag but don't feel strongly. I think anyone who needs this flag will be able to find it and being able to use -C won't actually have much of a difference at all.
2. I will make sure to update the docs for repro -c at the same time.

[bobertlo]

Okay, I have simply removed the -C flag here and we should be good to go on this PR. I will review the docs PR and update the syntax and look at showing repro -c as deprecated.

[bobertlo]

@efiop okay, I have also added a little note to the help message for dvc repro about the -c flag being deprecated.

This should be ready for review now.

[efiop]

Thanks!