Re: [PATCH v2 7/7] bisect: allows any terms set by user
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD git bisect new does not exist. Did you mean git bisect start HEAD? (autostart ?) y' is strictly equivalent to 'git bisect reset git bisect start git bisect new HEAD' In both case terms_defined value would be 0 while we kinda know that a term has been used. I don't understand. The user didn't say either bad or good, so in both cases we haven't seen a term yet. Or I misunderstood what you meant by define a term. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()
Mike Hommey m...@glandium.org writes: I'm tempted to make init_notes itself do the check, based on the value it is given for a read_only argument. Yeah, that would be one sensible way to go after making sure that everything goes thru this interface. On the other hand, some commands do their ref resolving themselves already. Again, as long as they do not bypass the read-only safety you are suggesting to add to init_notes(), that is OK. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] pull: allow dirty tree when rebase.autostash enabled
Kevin Daudt m...@ikke.info writes: rebase learned to stash changes when it encounters a dirty work tree, but git pull --rebase does not. Only verify if the working tree is dirty when rebase.autostash is not enabled. Signed-off-by: Kevin Daudt m...@ikke.info Helped-by: Paul Tan pyoka...@gmail.com --- Changes to v2: - Dropped the change of the existing --rebase test - Improvements to the test. Verified that the test fails before the change, and succeeds after the change. git-pull.sh | 5 - t/t5520-pull.sh | 11 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index 0917d0d..f0a3b6e 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -239,7 +239,10 @@ test true = $rebase { die $(gettext updating an unborn branch with changes added to the index) fi else - require_clean_work_tree pull with rebase Please commit or stash them. + if [ $(git config --bool --get rebase.autostash || echo false) = false ] Style (use of []). Shouldn't you be doing if ... then on an unborn elif we are not doing autostash require clean work tree fi which does not need unnecessarily deep nesting? + then + require_clean_work_tree pull with rebase Please commit or stash them. + fi fi oldremoteref= test -n $curr_branch diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index af31f04..aa247ec 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -233,6 +233,17 @@ test_expect_success '--rebase fails with multiple branches' ' test modified = $(git show HEAD:file) ' +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' + test_config rebase.autostash true + git reset --hard before-rebase + echo dirty new_file + git add new_file + git pull --rebase . copy + test_cmp_rev HEAD^ copy + test $(cat new_file) = dirty + test $(cat file) = modified again +' + test_expect_success 'pull.rebase' ' git reset --hard before-rebase test_config pull.rebase true -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 10/10] send-email: suppress meaningless whitespaces in from field
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: if (defined $sender) { + $sender =~ s/^\s+|\s$//g; I would say \s+ also for the second \s. Not really different, but it feels wrong to iterate the substitution as many times as there are trailing spaces to remove. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but Switch branchs or discard local changes still does not describe git checkout HEAD^^^ -- file.txt (restore to an old state, but does not switch branch) or git checkout -- file.txt (get from the index). You are right, especially when file.txt does not have any change relative to HEAD, there is no discarding going on. You are actively introducing a change to an unchanged file by checking contents out of a different revision. To me, discard local changes imply that there will be no uncommited changes on the files implied in the command after the operation. Yup. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Should the --encoding argument to log/show commands make any guarantees about their output?
Jan-Philip Gehrcke jgehr...@googlemail.com writes: I was surprised to see that the output of git log --encoding=utf-8 --format=format:%b can contain byte sequences that are invalid in UTF-8. Note: I am using git 2.1.4 and the %b format specifier represents the commit message body. Yeah, if the original was bad and cannot be sanely expressed in UTF-8, you have two options. You can show the contents as raw bytes recorded in the object with a warning so that the user can use it as such (e.g. perhaps the original was indeed an iso8859-2 but was incorrectly marked as UTF-8, or something like that, and a human that is more intelligent than a tool _could_ guess and attempt to recover). Or you can error out and refuse to produce output. We deliberately made a design choice to take the former option. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD
Johannes Schindelin johannes.schinde...@gmx.de writes: The symptom is that .git/CHERRY_PICK_HEAD is left behind after skipping an already-merged patch with `git rebase --continue` instead of `git rebase --skip`. I always prefer the former invocation because the latter would also skip legitimate patches if there were merge conflicts, while the former would not allow that. Makes sense; will queue. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 2015-06-17 18.19, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but Switch branchs or discard local changes still does not describe git checkout HEAD^^^ -- file.txt (restore to an old state, but does not switch branch) or git checkout -- file.txt (get from the index). You are right, especially when file.txt does not have any change relative to HEAD, there is no discarding going on. You are actively introducing a change to an unchanged file by checking contents out of a different revision. To me, discard local changes imply that there will be no uncommited changes on the files implied in the command after the operation. Yup. What was discussed in this thread sounded suspiciously familiar ;-). Unfortunately overwrite changes in the working tree and discard local changes are equally bad. As it does not say overwrite with what, we invite the original confusion that triggered these threads if the reader thought an equally useful but different overwrites with result of merging your local changes to the pristine (similar to what checkout -m does) would happen. At least, restore working tree files without saying restoring them to what state? is much less likely to cause such a confusion. So perhaps git-checkout - Switch branches or restore working tree files in the headline, and then explain restore to what state in the description? I'm not sure if the restore is always the right thing to describe: 'git checkout commit -- path' will copy the version from another commit into the workspace. My v3 will probably use the original line: git-checkout - Checkout a branch or paths to the working tree (and improve the description) git-checkout - Checkout a branch or paths to the working tree -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clone: check if server supports shallow clones
On Wed, Jun 17, 2015 at 09:35:23AM -0700, Junio C Hamano wrote: Of course it's hard to add to the test suite, since we do not have a way of hitting a server that does not understand shallow (I simply fudged server_supports() to return false on the client). We've had the shallow capability advertised since ed09aef0 (support fetching into a shallow repository, 2006-10-30), and this patch itself may not be that super-important in practice. Let's not worry too much about a test for situations that may not likely matter to us [*1*]. I had actually started looking at doing a generic interop testing suite. It would be nice to be able to do something like: cd t/interop ./run v1.0.0 v2.0.0 ./t0001-clone.sh and then the test script looks something like: test_expect_success 'clone with A from B' ' git.a clone -u git.b upload-pack ' The run script is similar to the version in t/perf that builds arbitrary revisions for testing, but with the twist that it points the PATH to git.a and git.b, which symlink into the bin-wrappers/ of the built directories (and probably disallows bare git to prevent mistakes). But I agree that this particular bug is not all that exciting to test. *1* How behind are re-implementations of upload-pack by other people, I have to wonder, though? JGit advertises shallow. Libgit2 does not, but it also does not implement upload-pack. :) I do wonder which server Mike was hitting to come across this in the first place. Maybe the Google Code dulwich-based one? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fetch-pack: check for shallow if depth given
On Wed, Jun 17, 2015 at 07:48:14AM -0400, Mike Edgar wrote: When a repository is first fetched as a shallow clone, either by git-clone or by fetching into an empty repo, the server's capabilities are not currently consulted. The client will send shallow requests even if the server does not understand them, and the resulting error may be unhelpful to the user. This change pre-emptively checks so we can exit with a helpful error if necessary. Signed-off-by: Mike Edgar ad...@google.com Looks good to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Should the --encoding argument to log/show commands make any guarantees about their output?
On 17.06.2015 18:42, Junio C Hamano wrote: Jan-Philip Gehrcke jgehr...@googlemail.com writes: I was surprised to see that the output of git log --encoding=utf-8 --format=format:%b can contain byte sequences that are invalid in UTF-8. Note: I am using git 2.1.4 and the %b format specifier represents the commit message body. Yeah, if the original was bad and cannot be sanely expressed in UTF-8, you have two options. You can show the contents as raw bytes recorded in the object with a warning so that the user can use it as such (e.g. perhaps the original was indeed an iso8859-2 but was incorrectly marked as UTF-8, or something like that, and a human that is more intelligent than a tool _could_ guess and attempt to recover). Or you can error out and refuse to produce output. The two-option scenario is totally clear. Although one must stress that the error-out option can, as discussed, be kept minimally invasive: it is sufficient (and common) to just skip those byte sequences (and replace them with a replacement symbol) that would be invalid in the requested output encoding. This would retain as much information as possible while guaranteeing a subsequent decoder to retrieve valid input. We deliberately made a design choice to take the former option. I totally support this design choice in general, especially when invoking `git whatever` without options. This here is, I think, mainly about documentation and the semantics of --encoding. From my point of view, `--encoding=utf-8` semantically suggests that the output *is* valid UTF-8. But it is not, not always. May initial question was: what do you think about this? Should we * just make this more clear in the docs and/or * should we adjust the behavior of --encoding or * should we do something entirely different, like adding a new command line option or * should we just leave things as they are? Thanks and cheers, Jan-Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Documentation on git-checkout --ours/--theirs improved
Simon Eugster simon...@gmail.com writes: A better picture would be nice. And regarding the textual description, are you going to commit your version? Nah, I'd rather not take credit away from you ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 10/10] send-email: suppress meaningless whitespaces in from field
Matthieu Moy matthieu@grenoble-inp.fr writes Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: if (defined $sender) { + $sender =~ s/^\s+|\s$//g; I would say \s+ also for the second \s. Not really different, but it feels wrong to iterate the substitution as many times as there are trailing spaces to remove. Oops should have read it one more time... Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but Switch branchs or discard local changes still does not describe git checkout HEAD^^^ -- file.txt (restore to an old state, but does not switch branch) or git checkout -- file.txt (get from the index). You are right, especially when file.txt does not have any change relative to HEAD, there is no discarding going on. You are actively introducing a change to an unchanged file by checking contents out of a different revision. To me, discard local changes imply that there will be no uncommited changes on the files implied in the command after the operation. Yup. What was discussed in this thread sounded suspiciously familiar ;-). Unfortunately overwrite changes in the working tree and discard local changes are equally bad. As it does not say overwrite with what, we invite the original confusion that triggered these threads if the reader thought an equally useful but different overwrites with result of merging your local changes to the pristine (similar to what checkout -m does) would happen. At least, restore working tree files without saying restoring them to what state? is much less likely to cause such a confusion. So perhaps git-checkout - Switch branches or restore working tree files in the headline, and then explain restore to what state in the description? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()
On Wed, Jun 17, 2015 at 06:02:46PM +0900, Mike Hommey wrote: In a sense that is weirdly broken already: $ git log --notes=:/foo /dev/null warning: notes ref refs/notes/:/foo is invalid but I wonder if we should be making expand_notes_ref a little more careful as part of the same topic. Interestingly, now that I look, there's also this: https://github.com/git/git/blob/master/notes-cache.c#L40 that doesn't use expand_notes_ref, but it's apparently only used in userdiff_get_textconv, and I'm not sure why. notes-cache.c predates expand_notes_ref. I don't think it's a big deal. The implementation of notes-cache is an internal detail (the user interacts with it by setting diff.*.cacheTextConv, and then git handles the cache). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clone: check if server supports shallow clones
Jeff King p...@peff.net writes: On Thu, Jun 11, 2015 at 08:02:33PM +0700, Duy Nguyen wrote: I see that do_fetch_pack checks server_supports(shallow). Is that enough to cover all fetch cases? And if it is, why does it not cover the matching clone cases? I think this replacement check would do if ((args-depth 0 || is_repository_shallow()) !server_supports(shallow)) die(Server does not support shallow clients); Oh, indeed, there is the depth flag I was looking for. :) And from some rudimentary testing, I believe that: git init git fetch --depth=1 ... is currently broken in the same way as clone (we are not shallow yet, so it does not complain when the server does not support it). I think the patch above fixes both that and the clone case. Of course it's hard to add to the test suite, since we do not have a way of hitting a server that does not understand shallow (I simply fudged server_supports() to return false on the client). We've had the shallow capability advertised since ed09aef0 (support fetching into a shallow repository, 2006-10-30), and this patch itself may not be that super-important in practice. Let's not worry too much about a test for situations that may not likely matter to us [*1*]. Thanks, all. [Footnote] *1* How behind are re-implementations of upload-pack by other people, I have to wonder, though? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2015, #04; Tue, 16)
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: * gr/rebase-i-drop-warn (2015-06-01) 2 commits - git rebase -i: warn about removed commits - git-rebase -i: add command drop to remove a commit Add drop commit-object-name subject command as another way to skip replaying of a commit in rebase -i, and then punish those who do not use it (and instead just remove the lines) by throwing a warning. What's the status of this one? A third commit was added (static check). I have some corrections and refactoring to do after the reviewing (and less time than before), a reroll is to be expected before next week. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: --- git-send-email.perl | 54 +++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index a0cd7ff..a1f6c18 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -477,9 +477,59 @@ foreach my $entry (@bcclist) { sub parse_address_line { if ($have_mail_address) { return map { $_-format } Mail::Address-parse($_[0]); - } else { - return split_addrs($_[0]); } + + my $commentrgx=qr/\((?:[^)]*)\)/; + my $quotergx=qr/(?:[^\\\]|\\.)*/; + my $wordrgx=qr/(?:[^][\s():;@\\,.]|\\.)+/; Spaces around = please. The code below is a bit hard to read (I'm neither fluent in Perl nor in the RFC ...). A few more comments would help. A few examples below (it's up to you to integrate them or not). + my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/; + + my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_; + push @tokens, ,; # parse a full address like # Phrase (comment) addr...@example.com (to clarify the wording) + my (@addr_list, @phrase, @address, @comment, @buffer) = (); + foreach my $token (@tokens) { + if ($token =~ /^[,;]$/) { Here and below: you're indenting with a 4-column offset, it should be 8. + if (@address) { + push @address, @buffer; + } else { + push @phrase, @buffer; + } + + my $str_phrase = join ' ', @phrase; + my $str_address = join '', @address; + my $str_comment = join ' ', @comment; # Escape special-characters with backslash + if ($str_phrase =~ /[][():;@\\,.\000-\037\177]/) { + $str_phrase =~ s/(^|[^\\])/$1/g; + $str_phrase = qq[$str_phrase]; + } + + if ($str_address ne $str_phrase ne ) { + $str_address = qq[$str_address]; + } + + my $str_mailbox = $str_phrase $str_address $str_comment; + $str_mailbox =~ s/^\s*|\s*$//g; + push @addr_list, $str_mailbox if ($str_mailbox); + + @phrase = @address = @comment = @buffer = (); + } elsif ($token =~ /^\(/) { + push @comment, $token; + } elsif ($token eq ) { + push @phrase, (splice @address), (splice @buffer); + } elsif ($token eq ) { + push @address, (splice @buffer); + } elsif ($token eq @) { + push @address, (splice @buffer), @; + } elsif ($token eq .) { + push @address, (splice @buffer), .; + } else { # We don't know what the token belongs to yet. We'll # decide where to append @buffer later. + push @buffer, $token; + } + } + + return @addr_list; -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git completion not using ls-remote to auto-complete during push
I do the following: $ git push origin :topic If I stop halfway through typing 'topic' and hit TAB, auto-completion does not work if I do not have a local branch by that name (sometimes I delete my local branch first, then I push to delete it remotely). I thought that git completion code was supposed to use ls-remote to auto complete refs used in push operations. Is this supposed to work? I'm using Git 2.4.3. Tested on both MSYS2 in Windows and Linux Mint. Same behavior in both. I am using the latest git completion code from the master branch in the git repo. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 2015-06-17 17.29, Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but Switch branchs or discard local changes still does not describe git checkout HEAD^^^ -- file.txt (restore to an old state, but does not switch branch) or git checkout -- file.txt (get from the index). You are right, especially when file.txt does not have any change relative to HEAD, there is no discarding going on. You are actively introducing a change to an unchanged file by checking contents out of a different revision. To me, discard local changes imply that there will be no uncommited changes on the files implied in the command after the operation. Yup. Thanks for the comments. I agree that we should keep the headline as is. What is about the the rest of the patch? Does it makes sense ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 05/10] send-email: Allow use of aliases in the From field of --compose mode
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: Aliases were expanded before checking the From field of the checking is misleading here. I thought you meant check that the From field is well-formed, while you mean set $sender based on the From: field. --compose option. This is inconsistent with other fields (To, Cc, ...) which already support aliases. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html