Lots of prompts when doing straight-pull-all is tiring

[vyp]

After reproducing an emacs config from the version lockfile on another machine, I did straight-pull-all on it to update all my packages. Then this prompt popped up for each package:

https://github.com/raxod502/straight.el/blob/87525b5b89ed8b6a6823422b0d9b0cb097eeac04/straight.el#L1038-L1040

So I had to press a for all packages to update them, which you can believe can be really repetitive and tiring especially if you have a lot of packages (like 100s). I think it was detached because git checkout <commit> was used for all the packages to get them on the correct revisions as specified by the lockfile? Would using git reset --hard <commit> instead work (to avoid the detached head state)?

[raxod502]

Yes, using git reset --hard would be preferable, and I was already planning to do that in #66. Another issue is that M-x straight-pull-all gives you a prompt for each package that must be upgraded to a newer version, which could be remedied by a better interface as discussed here.

[vyp]

Oh right so you're saying there would be a popup for each package even if the repos were not in detached head states?

Yeah so similar to #103 I just want to say update all my packages and then come back to them all updated (or not if there's failures) without having to stay at emacs answering all the prompts. Even if the operation is synchronous, it would still be better than how it is now in my opinion.

My searches didn't find #66 though, I feel like this issue is a duplicate of that? PS - Nice title for #66.

[xendk]

In the vein of "baby steps" how about making straight-vc-git--validate-head quietly fast forward or git reset --hard whenever the currently checked out commit is an ancestor of the remote branch head? That would eliminate all questions in the most common case.

[colonelpanic8]

@xendk Yeah that seems very reasonable to me.

[raxod502]

Sounds good. I can't deny it's an improvement over the current situation.

[xendk]

I did this, but then got cold feet.

Feels wrong to have straight-vc-git--validate-head quietly pull in changes. It would also affect straight-normalize-package, which might not be what you'd want. It's also a nice "ask the user what we should do" function.

I considered adding a ff-ok argument to make it optional, but that would cascade through a lot of functions.

So I thought, shouldn't straight-vc-git--merge-from-remote-raw ff if possible as that's really the place we're saying we want to merge? Problem is, that's calling straight-vc-git--validate-local to ensure everything is clean for merge, which also calls straight-vc-git--validate-head, which would popup before straight-vc-git--merge-from-remote-raw gets a chance to ff.

So I think we need a looser "check that we haven't diverted too much" function that won't consider HEAD being behind remote/HEAD a problem (it could still call straight-vc-git--validate-head for user input when they have diverted too much, might consider renaming validate-head though), and use that in straight-vc-git--validate-local, and let straight-vc-git--merge-from-remote-raw do the ff'ing itself.

But taking a step back, I seem to have muddied the waters (at least in my own head). The original issue as about detached heads, which is properly dealt with in #66. The

Yeah so similar to #103 I just want to say update all my packages and then come back to them all updated (or not if there's failures) without having to stay at emacs answering all the prompts.

part is somewhat addressed in #241 (at least one can get coffee while fetching the repos and answer all the questions in quick succession), which leaves getting straight-vc-git--merge-from-remote-raw to quietly ff for this issue. A later straight-pull-all might even queue up failures and keep processing, as to not hold up the process on one modified package.

[raxod502]

I considered adding a ff-ok argument to make it optional, but that would cascade through a lot of functions.

Isn't it sufficient to allow fast-forwarding when the ref argument is non-nil, and not otherwise? The function does two different things depending on that argument:

(cl-defun straight-vc-git--validate-head (local-repo branch &optional ref)
  "Validate that LOCAL-REPO has BRANCH checked out.
If REF is non-nil, instead validate that BRANCH is ahead of REF.
Any untracked files created by checkout will be deleted without
confirmation, so this function should only be run after
`straight-vc-git--validate-worktree' has passed."

queue up failures

Yes, that would be great.

[xendk]

@raxod502
What I like about the current function, is that it doesn't do anything without asking, which makes it useful as a generic "get the repo in line" function. Besides it's complicated enough as it is.

And it's really straight-vc-git--merge-from-remote-raw that "knows" that fast-forwarding is okay in this case, so it makes sense that it's the one that's does it (also for the sanity of the next developer).

Not entirely sure how I see this, but I haven't had time to properly work on it.

[raxod502]

it's really straight-vc-git--merge-from-remote-raw that "knows"

Just to be clear, straight-vc-git--merge-from-remote-raw is the only function that calls straight-vc-git--validate-head with a non-nil third argument, so changes to that branch of the logic would in practice only affect straight-vc-git--merge-from-remote-raw.

Besides it's complicated enough as it is. [...] (also for the sanity of the next developer)

But these are both good arguments. ¯\_(ツ)_/¯

I haven't had time to properly work on it.

Yeah, same and probably for quite a while. So I'm pretty open to whatever solutions may be proposed, if they make things better :)

[xendk]

Just to be clear

Yeah.. But I just realized, from the docstring:

If REF is non-nil, instead validate that BRANCH is ahead of REF.

That's the exact opposite of what thee situation is for a fast-forward to be possible. Donno if this insight helps me much, though.

Thinking of possible usecases, I'm tempted to create some simpler "check that worktree is clean and we're ahead/behind the remote head" and letting users deal with everything else in magit...

I'm pondering straight-vc-git--validate-head up into straight-vc-git--validate-head (which just checks that it's not decapitated) and some other validation functions that caters to different needs, but don't have it figured out yet.

Aah well, one of these days I'll have an evening where I can properly dig in.

[raxod502]

That's the exact opposite of what thee situation is for a fast-forward to be possible.

No, it's the only situation in which a fast-forward is possible—namely, if the branch is behind the ref and needs to be fast-forwarded so that it's ahead.

[xendk]

No, it's the only situation in which a fast-forward is possible—namely, if the branch is behind the ref and needs to be fast-forwarded so that it's ahead.

Eh, no, unless we're talking past each other. If branch is master and ref is origin/master, if branch is ahead of ref, it can't be fast-forwarded (would rather be a reset). Fast-forwarding is possible when branch is behind ref. Unless I'm really bad at reading the docstring/code. That would make it suitable for checking if the branch could be pushed.

Or am I entirely blind?

[raxod502]

we're talking past each other

Very likely :P (but that's my fault too)

So let me clarify the terminology used in the docstring:

• "X is behind Y" means either X is strictly behind Y or X is even with Y
• "X is ahead of Y" means either X is strictly ahead of Y or X is even with Y
• "validate X" means if X is not currently true, then perform some action which will result in X being true

So straight-vc--git-validate-head, with a non-nil REF argument, validates that BRANCH is ahead of REF. This means that if BRANCH is neither ahead of REF nor even with it, then some action will be taken to make that the case. The most often case will be that BRANCH is actually behind REF, in which case it needs to be fast-forwarded so that it is even (i.e. ahead).

As a consequence, "validate that BRANCH is ahead of REF" will often entail a fast-forward.

[xendk]

then perform some action which will result in X being true

Aha, that's reason for the misunderstanding. I'm used to thinking of "validate" as "check that X is the case", not as "ensure that X is the case". I'd have named the function with "ensure".

Then automatic fast-forwarding makes sense again. I'll give it another pass with my newfound insight.

[colonelpanic8]

I'm used to thinking of "validate" as "check that X is the case", not as "ensure that X is the case". I'd have named the function with "ensure".

+1 https://www.google.com/search?q=validate

[raxod502]

I'd have named the function with "ensure".

That seems more reasonable. I have no objection to a rename.

I think the reason for the nomenclature is that I started with straight-vc-git--validate-remote, and then once I got to later functions I just picked the same word.

[raxod502]

@vyp Do you think this issue is solved by #245?

[raxod502]

I think it is.