git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Git List <git@vger.kernel.org>,
	Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH v3 1/4] automatically ban strcpy()
Date: Sat, 28 Jul 2018 05:24:43 -0400	[thread overview]
Message-ID: <20180728092443.GB16454@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqq8t5woa5f.fsf@gitster-ct.c.googlers.com>

On Fri, Jul 27, 2018 at 10:34:20AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >>  $ git rebase --onto HEAD @{-1}~3 @{-1}^0
> >
> > Interesting. I'd have probably done it with an interactive rebase:
> >
> >   $ git rebase -i HEAD~4
> >   [change first "pick" to "edit"; after stopping...]
> >   $ git reset --hard HEAD^ ;# throw away patch 1
> >   $ git am -s mbox         ;# apply single patch
> >   $ git rebase --continue
> >
> > Which is really the same thing,...
> 
> I have unfounded fear for doing anything other than "edit", "commit
> --amend", "rebase --continue", or "rebase --abort" during a "rebase
> -i" session.  
> 
> Especiallly "reset --hard" with anything but HEAD.  I guess that is
> because I do not fully trust/understand how the sequencer machinery
> replays the remainder of todo tasks on top of the HEAD that is
> different from what it left me to futz with when it relinquished the
> control.

I jump around via "reset --hard" all the time during interactive
rebases. I don't recall having any issues, though that does not mean
there isn't a corner case lurking. :)

> Also "am" during "rebase -i" is scary to me, as "am" also tries to
> keep its own sequencing state.  Do you know if "rebase --continue"
> would work correctly in the above sequence if "am" conflicted, I
> gave up, and said "am --abort", for example?  I don't offhand know.

This one is trickier. I _assumed_ it would be fine to "am" during a
rebase, but I didn't actually try it (in fact, I rarely do anything
exotic with "am", since my main use is just test-applying patches on a
detached head).

It _seems_ to work with this example:

-- >8 --
git init repo
cd repo

for i in 1 2 3 4; do
  echo $i >file
  git add file
  git commit -m $i
done
git format-patch --stdout -1 >patch

git reset --hard HEAD^
GIT_EDITOR='perl -i -pe "/2$/ and s/pick/edit/"' git rebase -i HEAD~2
git am patch
git am --abort
-- 8< --

I think because "am" lives in $GIT_DIR/rebase-apply, and "rebase -i"
lives in ".git/rebase-merge". Of course "rebase" can use the
rebase-apply directory, but I think interactive-rebase never will.

So it works, but mostly by luck. :)

In my ideal world, operations like this that can be continued would be
stored in a stack, and each stack element would know its operation type.
So you could do:

  # push a rebase onto the stack
  git rebase foo

  # while stopped, you might do another operation which pushes onto the
  # stack
  git am ~/patch

  # aborting an operation (or finishing it naturally) pops it off the
  # stack; now we just have the rebase on the stack
  git am --abort

  # aborting an operation that's not at the top of the stack would
  # either be an error, or could auto-abort everybody on top
  git am ~/patch
  git rebase --abort ;# aborts the am, too!

  # you could even nest similar operations; by default we find the
  # top-most one in the stack, but you could refer to them by stack
  # position.
  #
  # put us in a rebase that stops at a conflict
  git rebase foo
  # oops, rewrite the last few commits as part of fixing the conflict
  git rebase -i HEAD~3
  # nope, let's abort the whole thing (stack level 0)
  git rebase --abort=0

  # it would also be nice to have generic commands to manipulate the
  # stack
  git op list ;# show the stack
  git op abort ;# abort the top operation, whatever it is
  git op continue ;# continue the top operation, whatever it is

I've hacked something similar to "git op continue" myself and find it
very useful, but:

  - it's intimately aware of all the possible operations, including some
    custom ones that I have. I wouldn't need to if each operation
    touched a well-known directory to push itself on the stack, and
    provided a few commands in the stack directory for things like "run
    this to abort me".

  - it sometimes behaves weirdly, because I "canceled" an operation with
    "reset" or "checkout", and later I expect to continue operation X,
    but find that some other operation Y was waiting. Having a stack
    means it's much easier to see which operations are still hanging
    around (we have this already with the prompt logic that shows things
    like rebases, but again, it has to be intimate with which operations
    are storing data in $GIT_DIR)

Anyway. That's something I've dreamed about for a while, but the thought
of retro-fitting the existing multi-command operations turned me off.
The current systems _usually_ works.

-Peff

  reply	other threads:[~2018-07-28  9:24 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19 20:33 [PATCH 0/2] fail compilation with strcpy Jeff King
2018-07-19 20:39 ` [PATCH 1/2] introduce "banned function" list Jeff King
2018-07-19 21:11   ` Eric Sunshine
2018-07-19 21:27     ` Jeff King
2018-07-19 21:59       ` Eric Sunshine
2018-07-20  0:55         ` Jeff King
2018-07-19 21:15   ` Stefan Beller
2018-07-19 21:32     ` Jeff King
2018-07-19 21:47       ` Stefan Beller
2018-07-20  0:54         ` Jeff King
2018-07-19 22:46   ` Junio C Hamano
2018-07-19 23:55     ` Randall S. Becker
2018-07-20  1:08     ` Jeff King
2018-07-20  1:12       ` Jeff King
2018-07-20  9:32       ` Junio C Hamano
2018-07-20 17:45         ` Jeff King
2018-07-20 13:22       ` Theodore Y. Ts'o
2018-07-20 17:56         ` Jeff King
2018-07-20 19:03           ` Junio C Hamano
2018-07-20 12:42   ` Derrick Stolee
2018-07-20 14:41   ` Duy Nguyen
2018-07-20 17:48     ` Elijah Newren
2018-07-20 18:04       ` Jeff King
2018-07-20 18:00     ` Jeff King
2018-07-19 20:39 ` [PATCH 2/2] banned.h: mark strncpy as banned Jeff King
2018-07-19 21:12   ` Ævar Arnfjörð Bjarmason
2018-07-19 21:33     ` Jeff King
2018-07-20 18:58 ` [PATCH 0/2] fail compilation with strcpy Junio C Hamano
2018-07-20 19:18   ` Jeff King
2018-07-20 21:50     ` Junio C Hamano
2018-07-24  9:23 ` [PATCH v2 0/4] " Jeff King
2018-07-24  9:26   ` [PATCH v2 1/4] automatically ban strcpy() Jeff King
2018-07-24 17:20     ` Eric Sunshine
2018-07-26  6:58       ` Jeff King
2018-07-26  7:21         ` [PATCH v3 " Jeff King
2018-07-26 17:33           ` Junio C Hamano
2018-07-27  8:08             ` Jeff King
2018-07-27 17:34               ` Junio C Hamano
2018-07-28  9:24                 ` Jeff King [this message]
2018-07-24  9:26   ` [PATCH v2 2/4] banned.h: mark strcat() as banned Jeff King
2018-07-24  9:27   ` [PATCH v2 3/4] banned.h: mark sprintf() " Jeff King
2018-07-24  9:28   ` [PATCH v2 4/4] banned.h: mark strncpy() " Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180728092443.GB16454@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).