tsrc icon indicating copy to clipboard operation
tsrc copied to clipboard

Make `tsrc sync` optionally automatically git stash & pop to update the default branch even if current is dirty

Open gdubicki opened this issue 3 years ago • 17 comments

I understand and am all for the current default behavior of tsrc sync, which when it encounters a repo that is not on the branch configured in the manifest, just prints this info out and does not do anything with it.

However, in my use case, I would always prefer tsrc to check if that repo is not dirty and if not, then switch to the branch defined in the manifest and do git pull.

Would a PR adding such optional and disabled by default, behavior be accepted?

gdubicki avatar Mar 11 '21 09:03 gdubicki

Would a PR adding such optional and disabled by default, behavior be accepted?

You're welcome to try ;)

My instincts tell me it's going to be difficult to implement and use because just naming the behavior is hard. You have to find something shorter than switch to the branch defined in the manifest and run git pul if not dirty. Finding a good word for sync was already pretty hard.

Actually, I think we scould address this kind of feature request without having to change the behavior of tsrc sync or adding new command-line flags by having tsrc foreach be a tiny bit clever.

I've created an issue about that here: #287.

Feel free to add your feedback there.

dmerejkowsky avatar Mar 11 '21 14:03 dmerejkowsky

So I went ahead and tried it.

I started with a workspace like this:

$ tsrc status
:: Using workspace in /home/dmerej/tmp/tsrc
:: Collecting statuses of 3 repo(s)
=> Workspace status:                                                                       
* bar      other (expected: master)
* foo      master 
* manifest master 

Then I wrote a sync.sh script looking like this:

# in workspace/sync.sh
if [[ "${TSRC_PROJECT_STATUS_DIRTY}" = "true" ]]; then
  echo project is dirty
  exit 1
fi

git switch $TSRC_PROJECT_MANIFEST_BRANCH
git pull

And finally, I ran:

$ tsrc foreach -- bash $(pwd)/sync.sh
:: Using workspace in /home/dmerej/tmp/tsrc
:: Running `bash /home/dmerej/tmp/tsrc/sync.sh` on 3 repos
* (1/3) bar
$ bash /home/dmerej/tmp/tsrc/sync.sh
Switched to branch 'master'
Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)
Updating 9e7a8e4..5f9bbd4
Fast-forward
* (2/3) foo
$ bash /home/dmerej/tmp/tsrc/sync.sh
project is dirty
* (3/3) manifest
$ bash /home/dmerej/tmp/tsrc/sync.sh
Already on 'master'
Your branch is up to date with 'origin/master'.
Already up to date.
Error: Command failed for 1 repo(s)
* foo

Easy Peasy :)

dmerejkowsky avatar Jul 29 '21 15:07 dmerejkowsky

By the way you can write the sync.sh script in any programming language - which is nice I think

dmerejkowsky avatar Jul 29 '21 15:07 dmerejkowsky

@gdubicki should we close this ?

dmerejkowsky avatar Aug 03 '21 13:08 dmerejkowsky

Thank you for providing a workaround with the script, @dmerejkowsky , but frankly I would still really love to have this feature included in tsrc...

One reason is that I am lazy and forgetful ;P and I don't want to have this extra step to make my tsrc fully working on a new machine. The other is that I want this workflow to be used throughout my global company and rolling out a solution that will require an extra script and non-standard command is much more difficult than telling about a new cli parameter.

But in the mean time I realized that what I actually do and what I would like to automate, is a bit different behavior. After running tsrc sync I go through the list of all the repos that were on unexpected branches and for each:

  1. switch to its dir
  2. git stash
  3. git checkout
  4. git pull
  5. git checkout
  6. git stash pop

This way I restore it to the dirty state it was in, but the default branch is up to date.

I think that this behavior should be safe to be enabled as the new default. Or at least be configurable in the workspace config, so that we could roll out the new config along with the manifest update.

What do you think, @dmerejkowsky ?

gdubicki avatar Aug 14 '21 12:08 gdubicki

I think that this behavior should be safe to be enabled as the new default

Well, I tried that already in a previous project a long time ago. It's doable of course, but things start to get really tricky if anything goes wrong. For instance, what do you do if step 3 fails? If step 6 fails?

I really don't want tsrc to be 'suprising' in any way - better do less that do unexpected or confusing things.

I go through the list of all the repos that were on unexpected branches and for each

Maybe we need a command for that, then tsrc update-default-branches ?

In all seriousness, I'm also wondering if the steps 1 to 6 should be a feature of git itself ?

Rolling out a solution that will require an extra script and non-standard command is much more difficult than telling about a new cli parameter.

I know that, but various users have various needs and at one point we have to say no or the tool becomes to complex to use and to hard to maintain.

dmerejkowsky avatar Aug 14 '21 14:08 dmerejkowsky

In all seriousness, I'm also wondering if the steps 1 to 6 should be a feature of git itself ?

Maybe something with git update-ref?

dmerejkowsky avatar Aug 14 '21 19:08 dmerejkowsky

You can change your git config in the repo or globally to achieve same goal

[pull]
	#always rebase and stash before pulling
	rebase = true

[rebase]
	autoStash = true

mkjmdski avatar Sep 11 '21 07:09 mkjmdski

Well, what you're proposing does not do step 3 and 5 (where we temporarily switch branches and back).

But it works well with tsrc foreach -- git pull

dmerejkowsky avatar Sep 11 '21 12:09 dmerejkowsky

It also works well with tsrc sync (at least if it is global settings for git)

mkjmdski avatar Sep 11 '21 19:09 mkjmdski

I'm confused. tsrc sync implementation only calls git fetch and git merge --ff-only @{upstream} - why would settings in [rebase]and [pull] be used ?

dmerejkowsky avatar Sep 11 '21 19:09 dmerejkowsky

However, in my use case, I would always prefer tsrc to check if that repo is not dirty and if not, then switch to the branch defined in the manifest and do git pull.

This is exactly the case I've already faced. We have hundreds of repos and usually I forget to switch branches after the specific task in the specific repo is finished. Then I want to sync everything with tsrc sync. It ends with fails in a few repositories. I need to open all of them, switch to the correct branch and sync manually or call tsrc sync again for them. They are not dirty, everything is synced with the remote repository so it was expected behavior for me to switch to the main branch.

I understand you don't want to make it a default behavior. What do you think to add the possibility to configure e.g. within the manifest file to enable such an option?

ElEid avatar Apr 26 '22 06:04 ElEid

I prefer talking about the sync.sh in the doc rather than change tsrc's code.

It's a really flexible method and you can put it under version control for your organization if you like :)

I don't think a manifest option in the file would work because I'm pretty sure not everyone will use the same workflow.

If enough people start complaining, maybe we'll add more options to tsrc sync, like tsrc syc --checkout-first, but I really want to give the sync.sh method a try first.

Or maybe we need aliases, ala git ...

dmerejkowsky avatar Apr 26 '22 11:04 dmerejkowsky

I really want to give the sync.sh method a try first.

I did try it and it works but it's not doing the same things as sync does. F.e. it does not clone new repos added to the manifest since the last sync. It's very clearly a workaround, not a proper fix.

I don't think a manifest option in the file would work because I'm pretty sure not everyone will use the same workflow.

How about putting it into workspace configuration then? If I get it right this is a per-user config, not shared.

maybe we'll add more options to tsrc sync, like tsrc sync --checkout-first

So would you accept a PR with adding such optional, non-default mode for sync + allowing to make it default for a given workspace with a setting with it?

gdubicki avatar Apr 29 '22 08:04 gdubicki

It does not clone new repos added to the manifest since the last sync. It's very clearly a workaround, not a proper fix.

Hum. I think you're right. Did not think about that.

So would you accept a PR with adding such optional [...]

Maybe... This sounds a bit complicated, though. Perhaps we could have a separate command at first ?

In a previous version of this tool we ended up with checkout, rebase and sync but I recall having problems explaining what each command did :(

So I fear that naming new option (or command) would be pretty hard, but feel free to try ;)

dmerejkowsky avatar May 04 '22 11:05 dmerejkowsky

Of course it would need more polish, but how about we first do what I implemented in #370? The default behavior doesn't change, it requires only a one-time manifest update and it works for the simple and safe case when the repo is clean.

gdubicki avatar Oct 01 '22 11:10 gdubicki

See also https://github.com/your-tools/tsrc/pull/375.

gdubicki avatar Nov 20 '22 17:11 gdubicki