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

[cherry-picker] Cherry-picker looses state if the target branch doesn't have a config in it #277

Closed
webknjaz opened this issue Jul 28, 2018 · 19 comments

Comments

@webknjaz
Copy link
Contributor

Basically, if there's some conflict and cherry-picker pauses it gets lost, not knowing how to proceed with --continue.
I think, one workaround might be to backport the commit with config.
However, it looks cleaner to me if we could use smth like git config (abuse custom .git/config section?) to have some state information in there. What do you think?

/cc: @abadger

@Mariatta
Copy link
Member

Hi, I'm GitMate.io!

It seems you've just enabled the issue triaging. I'm just scraping all issues from your repository and will give you some more information about this in a few minutes or so.

Because of the rate limit we can't scrape all information (including all comments and authors) right now - our system is already set up to scrape this in the next days over which the predictions will become more precise every day.

If you want me to use a different account for triaging your issues, simply create one and log in with it.

Sit tight!

@Mariatta
Copy link
Member

GitMate.io thinks possibly related issues are #249 ([cherry-picker] Document config options), #250 ([cherry-picker] Misleading assumption about the main branch), #251 ([cherry-picker] Misleading assumption about backport branches naming), #44 (Add the branch number to cherry pick commits?), and #89 (Change cherry-picker's README to treat it as a package).

@webknjaz
Copy link
Contributor Author

@Mariatta Wow, this bot is pretty aggressive :)

@abadger
Copy link
Contributor

abadger commented Jul 28, 2018

I'm thinking this can be thought of in two ways:

  • Where is my canonical config?

    • Using .git/config might be good for this
  • What is the config for the action we're presently dealing with?

    • For this we'd want cherry_picker to spit out a config file when it hits a pause point and then it would know to look for, load, and erase this temporary config when cherry_picker --continue is invoked.

Do you think one of these strategies is more desirable?

@webknjaz
Copy link
Contributor Author

Are you referring to toml file or config for a current action as a canonical config?

I was thinking about storing smth temporary for paused mode under .git/ (git config command ideally fits here, since it safely edits .git/config). It also may be some place under /tmp/.

@abadger
Copy link
Contributor

abadger commented Jul 31, 2018

Canonical config would be the toml file. But if that's the route bring taken, then it would be storing one config value in .git/config that points to it.

For example,

[cherry_picker]
config_branch="master"

I don't think of config as a place for storing temporary information. But maybe in the git ecosystem, it's common? (I think git's own commands store temporary state in .git/ but not in .git/config?)

@webknjaz
Copy link
Contributor Author

Yes, git itself uses .git/ folder for storing state.
Actually, while attending EuroPython I've been reminded that there's git notes. Guys used it to store release notes information attached to tag/commit. And in general, you can have non-default namespaces in Git and store custom refs there.
I'd use git config because it's a git-related tool, so it's logical to do it there. Yet, we could invent something more sophisticated like attaching a note to the very first commit in the tree with everything needed for current command to keep working.

@Mariatta
Copy link
Member

Mariatta commented Aug 1, 2018

@Mariatta Wow, this bot is pretty aggressive :)

I didn't write this bot!

Is the loss of information because you don't have .cherry_picker.toml in the maintenance branches?
If so, would it be simpler to have the same .cherry_picker.toml in all branches?

@webknjaz
Copy link
Contributor Author

webknjaz commented Aug 1, 2018

Well, basically yes. It's because in this "paused" mode the config is not there.
If we backport the config to old branches it would "fix" the symptom, however, imagine you change the config in the main branch and because config in target branch is different it might start to behave differently in a very flaky manner.
So we need to somehow preserve the config on devel throughout the process and make sure it's immutable from the beginning of backporting process till the end of conflict resolution.
So I think it might make sense to copy it under .git/ and remove it upon process completion.

@Mariatta
Copy link
Member

Mariatta commented Aug 1, 2018

Ok. I'm fine with saving the currently used config somewhere, and clean it up afterwards.
Using .git/config sounds good.

@webknjaz
Copy link
Contributor Author

webknjaz commented Aug 1, 2018

It's fine for key-value things, but it will probably not preserve variable types:

$ git config --local cherry-picker.some value
$ git config --local -l | grep cherry-picker
cherry-picker.some=value
$ git config --local cherry-picker.some
value

So I'm not sure whether we should go for it. A more resilient way is to copy a temporary config under /tmp/ and save path to it in git config, so that it will still be toml.

@Mariatta
Copy link
Member

Mariatta commented Aug 1, 2018

Makes sense. +1 to saving the toml in /tmp/

@Mariatta
Copy link
Member

Mariatta commented Aug 1, 2018

and clean up the tmp directory too afterwards.

@webknjaz
Copy link
Contributor Author

webknjaz commented Aug 1, 2018

and clean up the tmp directory too afterwards.

of course :)

@webknjaz
Copy link
Contributor Author

webknjaz commented Sep 10, 2018

Subproblems:

  • #108 Remember PR remote name for cherry_picker --continue
  • #107 Loss of title and description when doing cherry_picker --continue

@webknjaz
Copy link
Contributor Author

JFYI I've started working on a state preservation solution, which will rely on storing state in local (repo) Git config and will also read config contents directly from the correct revision in Git tree w/o creating a tmp file, which is even more resilient. This is going to be fun :)

@webknjaz
Copy link
Contributor Author

This command git show HEAD:.cherry_picker.toml outputs the file contents to stdout from the given revision. So I only need to store revision + chosen config (last arg) in git config!

@webknjaz
Copy link
Contributor Author

@Mariatta @abadger this is not yet well-tested demo: #295. Feel free to share your thoughts :)

@webknjaz
Copy link
Contributor Author

Closing this since subproblems https://github.com/python/core-workflow/issues/107 and https://github.com/python/core-workflow/issues/108 can be addressed separately based on the work in #295.

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

No branches or pull requests

3 participants