Re: [PATCH] rebase -i: introduce the 'test' command
On Mon, Dec 03, 2018 at 08:01:44PM +0100, Johannes Schindelin wrote: > Hi Luc, > > On Mon, 3 Dec 2018, Luc Van Oostenryck wrote: > > > On Sat, Dec 01, 2018 at 03:02:09PM -0500, Jeff King wrote: > > > I sometimes add "x false" to the top of the todo list to stop and create > > > new commits before the first one. That would be awkward if I could never > > > get past that line. However, I think elsewhere a "pause" line has been > > > discussed, which would serve the same purpose. > > > > > > I wonder how often this kind of "yes, I know it fails, but keep going > > > anyway" situation would come up. And what the interface is like for > > > getting past it. E.g., what if you fixed a bunch of stuff but your tests > > > still fail? You may not want to abandon the changes you've made, but you > > > need to "rebase --continue" to move forward. I encounter this often when > > > the correct fix is actually in an earlier commit than the one that > > > yields the test failure. You can't rewind an interactive rebase, so I > > > complete and restart it, adding an "e"dit at the earlier commit. > > > > In this sort of situation, I often whish to be able to do nested rebases. > > Even more because it happen relatively often that I forget that I'm > > working in a rebase and not on the head, and then it's quite natural > > to me to type things like 'git rebase -i @^^^' while already rebasing. > > But I suppose this has already been discussed. > > Varieties of this have been discussed, but no, not nested rebases. Interesting :) > The closest we thought about was re-scheduling the latest commits, > which is now harder because of the `--rebase-merges` mode. > > But I think it would be doable. Your idea of a "nested" rebase actually > opens that door quite nicely. It would not *really* be a nested rebase, > and it would still only be possible in interactive mode, but I could > totally see > > git rebase --nested -i HEAD~3 I don't mind much if it would be "really nested" or "as-if nested" but with this flag --nested I wonder what would happen if I would use it in a 'top-level' rebase (or, said in another way, would I be able to alias 'rebase' to 'rebase --nested')? > to generate and prepend the following lines to the `git-rebase-todo` file: > > reset abcdef01 # This is HEAD~3 > pick abcdef02 # This is HEAD~2 > pick abcdef03 # This is HEAD~ > pick abcdef04 # This is HEAD > OK, I see. This would not be nestable/stackable but would solve the problem nicely. Best regards, -- Luc
Re: [PATCH] rebase -i: introduce the 'test' command
On Sat, Dec 01, 2018 at 03:02:09PM -0500, Jeff King wrote: > On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote: > > > > > Would it not make more sense to add a command-line option (and a config > > > > setting) to re-schedule failed `exec` commands? Like so: > > > > > > Your proposition would do in most cases, however it is not possible to > > > make a distinction between reschedulable and non-reschedulable commands. > > > > True. But I don't think that's so terrible. > > > > What I think is something to avoid is two commands that do something very, > > very similar, but with two very, very different names. > > > > In reality, I think that it would even make sense to change the default to > > reschedule failed `exec` commands. Which is why I suggested to also add a > > config option. > > I sometimes add "x false" to the top of the todo list to stop and create > new commits before the first one. That would be awkward if I could never > get past that line. However, I think elsewhere a "pause" line has been > discussed, which would serve the same purpose. > > I wonder how often this kind of "yes, I know it fails, but keep going > anyway" situation would come up. And what the interface is like for > getting past it. E.g., what if you fixed a bunch of stuff but your tests > still fail? You may not want to abandon the changes you've made, but you > need to "rebase --continue" to move forward. I encounter this often when > the correct fix is actually in an earlier commit than the one that > yields the test failure. You can't rewind an interactive rebase, so I > complete and restart it, adding an "e"dit at the earlier commit. In this sort of situation, I often whish to be able to do nested rebases. Even more because it happen relatively often that I forget that I'm working in a rebase and not on the head, and then it's quite natural to me to type things like 'git rebase -i @^^^' while already rebasing. But I suppose this has already been discussed. -- Luc
Re: [PATCH] reopen_tempfile(): truncate opened file
On Tue, Sep 04, 2018 at 07:36:43PM -0400, Jeff King wrote: > On Tue, Sep 04, 2018 at 12:38:07PM -0400, Jeff King wrote: > > > > And just to be clear I'm looking forward to a patch from Jeff to fix > > > this since he clearly put more thoughts on this than me. With commit.c > > > being the only user of reopen_lock_file() I guess it's even ok to just > > > stick O_TRUNC in there and worry about O_APPEND when a new caller > > > needs that. > > > > That's the way I'm leaning to. The fix is obviously a one-liner, but I > > was hoping to construct a minimal test case. I just haven't gotten > > around to it yet. > > It turned out not to be too bad to write a test. It feels a little like > black magic, since I empirically determined a way in which the > cache-tree happens to shrink with the current code. But that assumption > is tested with a sanity check, so we'll at least know if it becomes a > noop. > > > The bug is ancient, so I don't think it's important for v2.19. > > The patch below should work on master or maint. We could do a fix > directly on top of the bug, but merging-up is weird (because the buggy > code became part of a reusable module). It's great that you were able to create a reproducer relatively easily. Thank you, guys. -- Luc
Re: [BUG] index corruption with git commit -p
On Sun, Sep 02, 2018 at 03:24:09AM -0400, Jeff King wrote:
> On Sun, Sep 02, 2018 at 09:12:04AM +0200, Duy Nguyen wrote:
> > diff --git a/builtin/commit.c b/builtin/commit.c
> > index 2be7bdb331..60f30b3780 100644
> > --- a/builtin/commit.c
> > +++ b/builtin/commit.c
> > @@ -432,6 +432,7 @@ static const char *prepare_index(int argc, const char
> > **argv, const char *prefix
> > if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
> > if (reopen_lock_file(_lock) < 0)
> > die(_("unable to write index file"));
> > + ftruncate(index_lock.tempfile->fd, 0);
> > if (write_locked_index(_index, _lock, 0))
> > die(_("unable to update temporary index"));
> > } else
>
> Doh, of course. I even thought about this issue and dug all the way into
> reopen_lock_file(), but for some reason temporarily forgot that O_WRONLY
> does not imply O_TRUNC.
>
> Arguably this should be the default for reopen_lockfile(), as getting a
> write pointer into an existing file is not ever going to be useful for
> the way Git uses lockfiles. Opening with O_APPEND could conceivably be
> useful, but it's pretty unlikely (and certainly not helpful here, and
> this is the only caller). Alternatively, the function should just take
> open(2) flags.
>
> At any rate, I think this perfectly explains the behavior we're seeing.
Yes, this certainly make sense.
For fun (and testing) I tried to take the problem in the other direction:
* why hasn't this be detected earlier (I'm reasonably be sure I did the
same operation a few times already without seeing the corruption)?
* how easy it is to reproduce the problem?
* Is it enough to do an interactive commit with nothing needing interactive?
So I tried the following and some variants:
> for i in $(seq -w 0 100); do echo $i > f$i; done
> git add f* && git commit -m add f* && git rm -q f* && git commit -m rm -p
but I didn't succeed to recreate the problem. So I'm still wondering
what is special in my repository & tree that trigger the corruption.
Anyway, thanks to al of you for looking at this so quickly.
-- Luc
[BUG] index corruption with git commit -p
Hi, I've just had a scary error: error: index uses $?+? extension, which we do not understand fatal: index file corrupt Things were quickly recovered by deleting the index but it clearly looks to a but to me. Here are the steps to reproduce it: $ git clone git://github.com/lucvoo/sparse-dev.git $ cd $ git co index-corruption $ git rm -r validation/ Documentation/ $ git commit -m -p $ git status error: index uses $?+? extension, which we do not understand fatal: index file corrupt The 'extension' pattern '$?+?', can vary a bit, sometimes it's just '', but always seems 4 chars. If the commit command doesn't use the '-p' flag, there is no problem. The repository itself is not corrupted, it's only the index. It happends with git 2.18.0 and 2.17.0 -- Luc Van Oostenryck
Re: partial_clone_get_default_filter_spec has no callers
On Thu, Dec 07, 2017 at 01:59:26AM +, Ramsay Jones wrote: > > BTW, if you are using a version of sparse post v0.5.1, you can > get rid of the only sparse warning on Linux (assuming you don't > build with NO_REGEX set), by using the -Wno-memcpy-max-count option; > I have the following set in my config.mak: > > $ tail -2 config.mak > pack-revindex.sp: SPARSE_FLAGS += -Wno-memcpy-max-count > > $ > > [I haven't sent a proper patch, since the required version of > sparse is not widely available yet.] Note that sparse simply ignore unknown options/flags, so adding it now won't create a problem and can already help those using a recent vesion of sparse. Regards, -- Luc
Re: bug during checkout of remote branch and uncommited changes ?
On Fri, Jul 7, 2017 at 11:22 PM, Junio C Hamano <gits...@pobox.com> wrote: > Luc Van Oostenryck <luc.vanoostenr...@gmail.com> writes: > >> At least here, the scenario I gave allow to fully reproduce the problem. >> >> Would you like any other information? > > Not really. Here is what I locally ran and its output > > -- >8 -- cut here -- >8 -- > > #!/bin/sh > > mkdir -p /var/tmp/x/lvo && cd /var/tmp/x/lvo || exit > > rm -fr src dst > > ( > mkdir src && > cd src && > git init && > >file && > git add file && > git commit -m 'initial' && > git checkout -b abranch && > echo abranch >file && > git commit -a -m 'abranch' > ) || exit > > mkdir dst && > cd dst && > git init && > git pull ../src master && > echo mine >file && > git status -suno && > git remote add aremote ../src && > git fetch aremote abranch || exit > > git checkout abranch > > git reset --hard > > git checkout abranch > > -- 8< -- cut here -- 8< -- > > $ sh script > Initialized empty Git repository in /var/tmp/x/lvo/src/.git/ > [master (root-commit) 8535bd5] initial > 1 file changed, 0 insertions(+), 0 deletions(-) > create mode 100644 file > Switched to a new branch 'abranch' > [abranch 32814d9] abranch > 1 file changed, 1 insertion(+) > Initialized empty Git repository in /var/tmp/x/lvo/dst/.git/ > From ../src > * branchmaster -> FETCH_HEAD > M file > From ../src > * branchabranch-> FETCH_HEAD > * [new branch] abranch-> aremote/abranch > error: Your local changes to the following files would be overwritten by > checkout: > file > Please commit your changes or stash them before you switch branches. > Aborting > HEAD is now at 8535bd5 initial > Switched to a new branch 'abranch' > Branch abranch set up to track remote branch abranch from aremote. > > > > As far as I can see, everything is working as intended. The first > "git checkout abranch" stops due to the dirty local state before it > even tries to DWIM to create abranch, and then after resetting to > get rid of all the local modifications, the second "git checkout > abranch" manages to create the branch just fine. > > Even with different variations (like making dst a clone of src to > force the "checkout" to see an ambiguity), I do not seem to get your > "the first one fails with an error message about Ambiguity, the > second does not", which indeed is a curious symptom. > > Without knowing if this is an ancient Git, or what remote other than > aremote this repository has, what remote-tracking branches from > these remotes this repository has, if they share the same name > abranch that points at the same or different objects, etc., I simply > do not know what is causing you the symptom, and time I allocated to > look into this for now just ran out. Strange. I forgot to say that I was using git 2.13.0 I have a few more remote but 'abranch' was just a new branch (with name unique to the remote) with fresh objects. I'll investigate things more here and report if anything new. Thanks, for your time. -- Luc
Re: bug during checkout of remote branch and uncommited changes ?
On Fri, Jul 07, 2017 at 08:53:39AM -0700, Junio C Hamano wrote: > Luc Van Oostenryck <luc.vanoostenr...@gmail.com> writes: > > > $ git reset --hard > > patching file afile.c > > Is that a message from something? It does not sound like something > "git reset --hard" would say. Indeed, sorry. This is the result of a 'git diff | patch -p1 -R' to which I'm used since a long time ago. I have no more reason to use it but ... habits ... :) But doing 'git reset --hard' gives exactly the same result. > > $ git co > > fatal: Not tracking: ambiguous information for ref > > refs/remotes// > > > > What can be ambiguous here? > > I think the message "Not tracking" is given when there is a remote > other than that also has . Mmmm, no I don't have that. At this point there is (in .git): refs/remotes// refs/heads/ The second one didn't existed before the checkout attempt, of course. > Between the time you > got the message and the time you tried to checkout , did > anything happen to cause the second attempt succeed? No. At least here, the scenario I gave allow to fully reproduce the problem. Would you like any other information? -- Luc
bug during checkout of remote branch and uncommited changes ?
Hi, Lately I've been caught several time with a problem which, to me, is a bug. It can easily be reproduced, so I think It's very possible it has already been reported. The problem arise when trying to checkout a branch while having some uncommited changes. The scenario is the following: $ git status M afile.c $ git remote add $ git fetch remote: Counting objects: 7, done. remote: Compressing objects: 100% (6/6), done. remote: Total 7 (delta 5), reused 3 (delta 1) Unpacking objects: 100% (7/7), done. From * branch -> FETCH_HEAD * [new branch] -> / $ git co error: Your local changes to the following files would be overwritten by checkout: afile.c Please commit your changes or stash them before you switch branches. Aborting $ git reset --hard patching file afile.c $ git co fatal: Not tracking: ambiguous information for ref refs/remotes// What can be ambiguous here? Strangely, trying a second time, succeed: $ git co Previous HEAD position was ... Switched to branch '' -- Luc Van Oostenryck
Re: [PATCH] usage: fix a sparse 'redeclared with different type' error
On Tue, May 16, 2017 at 5:02 AM, Jeff King <p...@peff.net> wrote: > On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote: > >> >> If you need to re-roll your 'jk/bug-to-abort' branch, could you please >> squash this into the relevant patch (commit d8193743e0 "usage.c: add >> BUG() function", 12-05-2017). >> >> [Just FYI, sparse complains thus: >> usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type >> (originally declared at git-compat-util.h:1076) - different modifiers >> ] > > Hmm. Our model here is the die() function, which gets noreturn and > format attributes in the declaration, but only noreturn at the > definition. It's because a deficiency of sparse: the format attribute is simply ignored while the NORETURN is treated as a 'modifiers' not much different than 'const' or 'extern'. But the real problem here with sparse is that while it indeed compare the declaration with the definition, this is done not by checking compatibility but by strict equality. In other words, the NORETURN, but also a 'static', in the declaration should be somehow inherited in a subsequent definition but sparse doesn't do that yet. -- Luc Van Oostenryck
Re: [BUG] branch renamed to 'HEAD'
On Mon, Feb 27, 2017 at 04:02:33AM -0500, Jeff King wrote: > Ugh. Actually, there are a few complications I found: > > 1. Checking "HEAD" afterwards means you can't actually have a branch > named "HEAD". Doing so is probably insane, but we probably really > _do_ want to just disallow the @-conversion here. > > 2. This isn't limited to just HEAD and @-conversion. For instance: It's a bit what I was afraid of. Luc
Re: [BUG] branch renamed to 'HEAD'
On Mon, Feb 27, 2017 at 11:43:46AM +0530, Karthik Nayak wrote: > Hello, > > Thanks for reporting, but I don't think it is a bug. > > On Mon, Feb 27, 2017 at 10:22 AM, Luc Van Oostenryck > <luc.vanoostenr...@gmail.com> wrote: > > Hi, > > > > I just discover something which very much seems a bug to me > > while making an error in renaming a branch. > > The scenario is the following: > > - I have a branch named 'orig' > > - I want to make some experimental changes on it: > > $ git checkout -b temp orig > > $ ... edit some files ... > > $ ... make some tests & commits ... > > - I'm happy with my changes, so I want to have my original > > branch to now points to the head of this temp branch > > but did it wrongly: > > $ git branch -m -f orig @ > > Here you are using the '-m' flag, which is to rename a branch. So what > you're essentially > doing is: > $ git branch -m -f orig HEAD > Do note that this won't reset 'orig' to point to 'HEAD', rather this > renames 'orig' to 'HEAD'. > > What you actually want to do (to reset 'orig' to 'HEAD') is: > $ git branch -f orig @ > This would make orig point to the current HEAD. Sure. I said it the description that I made an error in the renaming. What I consider as a bug is that '@', which stands for refs/HEAD, have been interpreted as a branch named 'HEAD' and thus created a reference refs/heads/HEAD. Luc
[BUG] branch renamed to 'HEAD'
Hi, I just discover something which very much seems a bug to me while making an error in renaming a branch. The scenario is the following: - I have a branch named 'orig' - I want to make some experimental changes on it: $ git checkout -b temp orig $ ... edit some files ... $ ... make some tests & commits ... - I'm happy with my changes, so I want to have my original branch to now points to the head of this temp branch but did it wrongly: $ git branch -m -f orig @ - Now I discover that I don't have anymore a branch named 'orig' That's fine, I made an error. - I'm searching what had happened and discover the name my branch have been renamed to: 'HEAD' In others words I have now an entry .git/refs/heads/HEAD which points to where my original branch pointed. In my opinion, it's a bug that '@' have been expanded/resolved into a branch named 'HEAD'. Luc Van Oostenryck
Re: dangling commits in worktree
On Wed, Nov 23, 2016 at 04:45:56PM +0700, Duy Nguyen wrote: > > It's a known issue that gc (and maybe some others that do rev-list > --all, like fsck) "forgets" about some worktree's refs and you will > see what you see. Good. I just wanted to be sure it was a known problem. Thanks for the info. Luc
