[PATCH v2] l10n: update German translation
Signed-off-by: Ralf Thielow --- v2 updates the translation up to the latest update of git.pot. range-diff: 1: f0a6c76bf ! 1: f8313495e l10n: update German translation @@ -205,13 +205,13 @@ -msgstr "" +msgstr "Falsche Reihenfolge bei multi-pack-index Pack-Namen: '%s' vor '%s'" #: midx.c:205 #, c-format - msgid "bad pack-int-id: %u (%u total packs" + msgid "bad pack-int-id: %u (%u total packs)" -msgstr "" -+msgstr "Fehlerhafte pack-int-id: %u (%u Pakete insgesamt)" ++msgstr "Ungültige pack-int-id: %u (%u Pakete insgesamt)" #: midx.c:246 msgid "multi-pack-index stores a 64-bit offset, but off_t is too small" -msgstr "" +msgstr "multi-pack-index speichert einen 64-Bit Offset, aber off_t ist zu klein." @@ -364,31 +364,31 @@ +#, c-format msgid "unable to join load_cache_entries thread: %s" -msgstr "kann Thread nicht erzeugen: %s" +msgstr "Kann Thread für load_cache_entries nicht erzeugen: %s" - #: read-cache.c:2200 + #: read-cache.c:2201 -#, fuzzy, c-format +#, c-format msgid "unable to create load_index_extensions thread: %s" -msgstr "kann Thread nicht erzeugen: %s" +msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s" - #: read-cache.c:2227 + #: read-cache.c:2228 -#, fuzzy, c-format +#, c-format msgid "unable to join load_index_extensions thread: %s" -msgstr "kann Thread nicht erzeugen: %s" -+msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s" ++msgstr "Kann Thread für load_index_extensions nicht beitreten: %s" - #: read-cache.c:2953 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086 + #: read-cache.c:2982 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086 #, c-format msgid "could not close '%s'" -msgstr "Konnte '%s' nicht schließen" +msgstr "Konnte '%s' nicht schließen." - #: read-cache.c:3026 sequencer.c:2203 sequencer.c:3592 + #: read-cache.c:3055 sequencer.c:2203 sequencer.c:3592 #, c-format @@ msgstr "Konnte '%s' nicht entfernen." #: rebase-interactive.c:10 @@ -802,14 +802,19 @@ #: builtin/grep.c:1051 -#, fuzzy msgid "invalid option combination, ignoring --threads" -msgstr "keine Unterstützung von Threads, --threads wird ignoriert" -+msgstr "ungültige Kombination von Optionen, --threads wird ignoriert" ++msgstr "Ungültige Kombination von Optionen, --threads wird ignoriert." - #: builtin/grep.c:1054 builtin/pack-objects.c:3395 + #: builtin/grep.c:1054 builtin/pack-objects.c:3397 msgid "no threads support, ignoring --threads" +-msgstr "keine Unterstützung von Threads, --threads wird ignoriert" ++msgstr "Keine Unterstützung für Threads, --threads wird ignoriert." + + #: builtin/grep.c:1057 builtin/index-pack.c:1503 builtin/pack-objects.c:2716 + #, c-format @@ msgstr "Für '%s' wurde der Alias '%s' angelegt." #: builtin/help.c:444 -#, fuzzy, c-format @@ -944,17 +949,17 @@ #: builtin/pack-objects.c:2123 msgid "suboptimal pack - out of memory" @@ "packen" - #: builtin/pack-objects.c:3316 + #: builtin/pack-objects.c:3318 -#, fuzzy msgid "respect islands during delta compression" -msgstr "Größe des Fensters für die Delta-Kompression" +msgstr "Delta-Islands bei Delta-Kompression beachten" - #: builtin/pack-objects.c:3340 + #: builtin/pack-objects.c:3342 #, c-format @@ "wurde nicht angefordert." #: builtin/pull.c:565 @@ -964,10 +969,28 @@ -msgstr "Konnte Commit '%s' nicht parsen." +msgstr "Konnte nicht auf Commit '%s' zugreifen." #: builtin/pull.c:843 msgid "ignoring --verify-signatures for rebase" +@@ + "config'." + + #: builtin/push.c:168 +-#, fuzzy, c-format ++#, c-format + msgid "" + "The upstream branch of your current branch does not match\n" + "the name of your current branch. To push to the upstream branch\n" +@@ + "Um auf den Branch mit demselben Namen im Remote-Repository zu versenden,\n" + "benutzen Sie:\n" + "\n" +-"git push %s %s\n" ++"git push %s HEAD\n" + "%s" + + #: builtin/push.c:183 @@ msgstr "unpack-trees protokollieren" #: builtin/rebase.c:29 -#, fuzzy @@ -1041,11 +1064,11 @@ -#, fuzzy msgid "could not determine HEAD revision" -msgstr "Konnte HEAD nicht loslösen" +msgstr "Konnte HEAD-Commit nicht bestimmen." - #: builtin/rebase.c:752 + #: builtin/rebase.c:753 -#, fuzzy, c-format +#, c-format msgid "" "%s\n" "Please specify which branch you want to rebase against.\n" @@ -1060,11 +1083,11 @@ +"Siehe git-rebase(1) für Details.\n" +"\n" +"git
Re: [PATCH] t/lib-git-daemon: fix signal checking
On Mon, Nov 26, 2018 at 09:03:37PM +0100, SZEDER Gábor wrote: > Test scripts checking 'git daemon' stop the daemon with a TERM signal, > and the 'stop_git_daemon' helper checks the daemon's exit status to > make sure that it indeed died because of that signal. > > This check is bogus since 03c39b3458 (t/lib-git-daemon: use > test_match_signal, 2016-06-24), for two reasons: > > - Right after killing 'git daemon', 'stop_git_daemon' saves its exit > status in a variable, but since 03c39b3458 the condition checking > the exit status looks at '$?', which at this point is not the exit > status of 'git daemon', but that of the variable assignment, i.e. > it's always 0. > > - The unexpected exit status should abort the whole test script with > 'error', but it doesn't, because 03c39b3458 forgot to negate > 'test_match_signal's exit status in the condition. > > This patch fixes both issues. Oof. Who says two wrongs don't make a right? :) Thanks for catching this, and the patch looks obviously correct. I peeked at the other test_match_signal conversions from that era, and they all look sane. -Peff
[PATCH v2 2/3] RelNotes 2.20: clarify sentence
I had to read this sentence a few times to understand it. Let's try to clarify it. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index f4e79c4cfb..1a5bbd2e91 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -305,7 +305,7 @@ Performance, Internal Implementation, Development Support etc. * The overly large Documentation/config.txt file have been split into million little pieces. This potentially allows each individual piece - included into the manual page of the command it affects more easily. + to be included into the manual page of the command it affects more easily. * Replace three string-list instances used as look-up tables in "git fetch" with hashmaps. -- 2.20.0.rc2.1.gc81af441bb
[PATCH v2 3/3] RelNotes 2.20: drop spurious double quote
We have three double-quote characters, which is one too many or too few. Dropping the last one seems to match the original intention best. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index 1a5bbd2e91..659474f7c3 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -578,7 +578,7 @@ Fixes since v2.19 * "git rev-parse --exclude=* --branches --branches" (i.e. first saying "add only things that do not match '*' out of all branches" - and then adding all branches, without any exclusion this time") + and then adding all branches, without any exclusion this time) worked as expected, but "--exclude=* --all --all" did not work the same way, which has been fixed. (merge 5221048092 ag/rev-parse-all-exclude-fix later to maint). -- 2.20.0.rc2.1.gc81af441bb
[PATCH v2 1/3] RelNotes 2.20: move some items between sections
Some items that should be in "Performance, Internal Implementation, Development Support etc." have ended up in "UI, Workflows & Features". Move them, and do s/uses/use/ while at it. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index b1deaf37da..f4e79c4cfb 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -137,11 +137,6 @@ UI, Workflows & Features command line, or setting sendemail.suppresscc configuration variable to "misc-by", can be used to disable this behaviour. - * Developer builds now uses -Wunused-function compilation option. - - * One of our CI tests to run with "unusual/experimental/random" - settings now also uses commit-graph and midx. - * "git mergetool" learned to take the "--[no-]gui" option, just like "git difftool" does. @@ -185,6 +180,11 @@ UI, Workflows & Features Performance, Internal Implementation, Development Support etc. + * Developer builds now use -Wunused-function compilation option. + + * One of our CI tests to run with "unusual/experimental/random" + settings now also uses commit-graph and midx. + * When there are too many packfiles in a repository (which is not recommended), looking up an object in these would require consulting many pack .idx files; a new mechanism to have a single -- 2.20.0.rc2.1.gc81af441bb
Re: [PATCH 1/3] RelNotes 2.20: move some items between sections
On Tue, 4 Dec 2018 at 03:23, Junio C Hamano wrote: > > Martin Ågren writes: > > > Some items that should be in "Performance, Internal Implementation, > > Development Support etc." have ended up in "UI, Workflows & Features" > > and "Fixes since v2.19". Move them, and do s/uses/use/ while at it. > > I agree with the early half of this change; I think it is OK to > consider lack of preparation for Travis transition and lack of > better testing in the maintenance track as bugs, though. Sure. Here's a resend where patch 1/3 has been simplified accordingly. Martin Ågren (3): RelNotes 2.20: move some items between sections RelNotes 2.20: clarify sentence RelNotes 2.20: drop spurious double quote Documentation/RelNotes/2.20.0.txt | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) Range-diff against v1: 1: d69f63b5f6 ! 1: 961bfc2ad6 RelNotes 2.20: move some items between sections @@ -3,8 +3,8 @@ RelNotes 2.20: move some items between sections Some items that should be in "Performance, Internal Implementation, -Development Support etc." have ended up in "UI, Workflows & Features" -and "Fixes since v2.19". Move them, and do s/uses/use/ while at it. +Development Support etc." have ended up in "UI, Workflows & Features". +Move them, and do s/uses/use/ while at it. Signed-off-by: Martin Ågren @@ -35,33 +35,3 @@ * When there are too many packfiles in a repository (which is not recommended), looking up an object in these would require consulting many pack .idx files; a new mechanism to have a single -@@ -two classes to ease code migration process has been proposed and -its support has been added to the Makefile. - -+ * The "container" mode of TravisCI is going away. Our .travis.yml -+ file is getting prepared for the transition. -+ (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). -+ -+ * Our test scripts can now take the '-V' option as a synonym for the -+ '--verbose-log' option. -+ (merge a5f52c6dab sg/test-verbose-log later to maint). -+ - - Fixes since v2.19 - - -@@ -didn't make much sense. This has been corrected. -(merge 669b1d2aae md/exclude-promisor-objects-fix later to maint). - -- * The "container" mode of TravisCI is going away. Our .travis.yml -- file is getting prepared for the transition. -- (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). -- -- * Our test scripts can now take the '-V' option as a synonym for the -- '--verbose-log' option. -- (merge a5f52c6dab sg/test-verbose-log later to maint). -- - * A regression in Git 2.12 era made "git fsck" fall into an infinite -loop while processing truncated loose objects. -(merge 18ad13e5b2 jk/detect-truncated-zlib-input later to maint). 2: eccb7edd08 = 2: 3027a34938 RelNotes 2.20: clarify sentence 3: 78f3043b65 = 3: a5e2df91b4 RelNotes 2.20: drop spurious double quote -- 2.20.0.rc2.1.gc81af441bb
Re: [PATCH v3] range-diff: always pass at least minimal diff options
On Mon, 3 Dec 2018 at 22:21, Eric Sunshine wrote:
> [es: retain diff coloring when going to stdout]
>
> Signed-off-by: Martin Ågren
> Signed-off-by: Eric Sunshine
> ---
>
> This is a re-roll of Martin's v2[1]. The only difference from v2 is that
> it retains coloring when emitting to the terminal (plus an in-code
> comment was simplified).
Thank you so much for this.
> if (rev->rdiff1) {
> + /*
> +* Pass minimum required diff-options to range-diff; others
> +* can be added later if deemed desirable.
> +*/
Agreed.
> + struct diff_options opts;
> + diff_setup();
> + opts.file = rev->diffopt.file;
> + opts.use_color = rev->diffopt.use_color;
Ah, s/0/rev->diffopt.use_color/, well that's obvious.
Thanks!
Martin
Re: sharedrepository=group not working
On Dec 3, 2018, at 8:50 PM, Jeff King wrote: > > I don't suppose this is leaving those incoming-* directories sitting > around so we can inspect their permissions (it's suppose to clean them > up, so I doubt it). If you're up for it, it might be interesting to > patch Git to inspect the umask and "ls -l" the objects/ directory at the > problematic moment. The interesting point is when we call into > tmp-objdir.c:setup_tmp_objdir(). The problem was that Apache was setting my umask to 113! With that: + ls -ldF ./objects/incoming-w7agmb/pack objects/incoming-w7agmb ls: cannot access ./objects/incoming-w7agmb/pack: Permission denied drw---S--- 2 apache cvs 4096 Dec 3 21:14 objects/incoming-w7agmb/ error: remote unpack failed: unable to create temporary object directory With 002 it succeeds: + ls -ldF ./objects/incoming-IbGS6h/pack objects/incoming-IbGS6h drwx--S--- 3 apache cvs 4096 Dec 3 21:19 objects/incoming-IbGS6h/ drwxrwsr-x 2 apache cvs 4096 Dec 3 21:19 ./objects/incoming-IbGS6h/pack/ So I fixed my umask and got it working, but maybe a test for "your umask is dumb" is worthwhile. Thanks for your help! -- Jamie Zawinski https://www.jwz.org/ https://www.dnalounge.com/
Re: sharedrepository=group not working
On Mon, Dec 03, 2018 at 08:19:12PM -0800, Jamie Zawinski wrote: > On Dec 3, 2018, at 8:09 PM, Jeff King wrote: > > > > but it works fine. Might there be some effective-uid trickiness with the > > way the server side of git is invoked? Or is this a network mount where > > the filesystem uid might not match the process uid? > > Huh. They're on the same ext4 fs (it's an AWS EBS sc1 volume, but I > think that still counts as "not a network mount" as far as Linux is > concerned.) Yeah, I think we can discount any oddness there. > The way I was seeing this fail was a CGI invoking "git push", as user > "httpd" (and I verified that when the cgi was invoked, "groups" > reported that "httpd" was a member of group "cvs") but when I tried to > reproduce the error with "sudo -u apache git push" it didn't fail. So > possibly something hinky is going on with group permissions when httpd > invokes git, but I did verify that whoami, groups and pwd were as > expected, so I couldn't tell what that might be... (Oh, I didn't check > what umask was, but it should have been 022...) Hrm. I don't think group permissions would even matter. We asked to mkdir() with 0700 anyway, so we know they'd be zero. But a funny umask does seem like a likely candidate for causing the problem. We asked for 0700, but if there were bits set in umask (say, 0200 or something), that would restrict that further. And it would explain what you're seeing (inability to write into a directory we just created), and it might have worked with previous versions (which was less strict on the group permissions). I don't suppose this is leaving those incoming-* directories sitting around so we can inspect their permissions (it's suppose to clean them up, so I doubt it). If you're up for it, it might be interesting to patch Git to inspect the umask and "ls -l" the objects/ directory at the problematic moment. The interesting point is when we call into tmp-objdir.c:setup_tmp_objdir(). -Peff
Re: sharedrepository=group not working
On Dec 3, 2018, at 8:19 PM, Jamie Zawinski wrote: > > (Oh, I didn't check what umask was, but it should have been 022...) Typo, I mean to say 002. -- Jamie Zawinski https://www.jwz.org/ https://www.dnalounge.com/
Re: sharedrepository=group not working
On Dec 3, 2018, at 8:09 PM, Jeff King wrote: > > but it works fine. Might there be some effective-uid trickiness with the > way the server side of git is invoked? Or is this a network mount where > the filesystem uid might not match the process uid? Huh. They're on the same ext4 fs (it's an AWS EBS sc1 volume, but I think that still counts as "not a network mount" as far as Linux is concerned.) The way I was seeing this fail was a CGI invoking "git push", as user "httpd" (and I verified that when the cgi was invoked, "groups" reported that "httpd" was a member of group "cvs") but when I tried to reproduce the error with "sudo -u apache git push" it didn't fail. So possibly something hinky is going on with group permissions when httpd invokes git, but I did verify that whoami, groups and pwd were as expected, so I couldn't tell what that might be... (Oh, I didn't check what umask was, but it should have been 022...) -- Jamie Zawinski https://www.jwz.org/ https://www.dnalounge.com/
Re: sharedrepository=group not working
On Mon, Dec 03, 2018 at 07:27:13PM -0800, Jamie Zawinski wrote:
> I think sharedrepository=group stopped working some time between
> 2.10.5 (works) and 2.12.4 (does not). 2.19.2 also does not.
Hmm. Given the time-frame and the fact that your strace shows problems
writing into the objects/incoming-* directory, it's likely caused by
722ff7f876 (receive-pack: quarantine objects until pre-receive accepts,
2016-10-03).
The big change there is that instead of writing directly into objects/,
we create a temporary objects/incoming-* directory, write there, and
then migrate the objects over after we determine they're sane.
So in your strace we see the temp directory get created:
> mkdir("./objects/incoming-U5EN8D", 0700
> <... mkdir resumed> ) = 0
The permissions are tighter than we ultimately want, but that's OK.
This tempdir is just for this process (and its children) to look at, and
then we'd eventually migrate the files out.
I could definitely imagine there being a bug in which we don't then
properly loosen permissions when we move things out of the tempdir, but
we don't even get that far. We fail immediately:
> mkdir("./objects/incoming-U5EN8D/pack", 0777) = -1 EACCES (Permission denied)
That seems strange. The outer directory is only 0700, but the user
permissions should be sufficient. Even with the g+s bit set, it should
still be owned by the same user, shouldn't it?
I tried reproducing your state like this:
git init --bare dst.git
git -C dst.git config core.sharedrepository group
chgrp -R somegroup dst.git
find dst.git -type f | xargs chmod g+rw
find dst.git -type d | xargs chmod g+srw
# push works from original user
git clone dst.git client
(
cd client &&
git commit --allow-empty -m foo
git push
)
# push works from alternate user
sudo su anotheruser sh -c '
git clone dst.git /tmp/other &&
cd /tmp/other &&
git commit --allow-empty -m foo &&
git push --receive-pack="strace -e mkdir git-receive-pack"
'
but it works fine. Might there be some effective-uid trickiness with the
way the server side of git is invoked? Or is this a network mount where
the filesystem uid might not match the process uid?
-Peff
sharedrepository=group not working
I think sharedrepository=group stopped working some time between 2.10.5 (works)
and 2.12.4 (does not). 2.19.2 also does not.
I have a user trying to push to a shared repo; the user is not the owner of the
files but it is in the same group. All the repo files are g+rw and all the repo
directories are g+srw.
drwxrwsr-x. 252 jwz cvs 4096 Dec 3 18:53 /cvsroot/dna.git/objects/
I am getting:
error: remote unpack failed: unable to create temporary object directory
To /cvsroot/dna.git
! [remote rejected] master -> master (unpacker error)
If I'm reading this strace right, it looks like git is successfully creating a
directory under objects/ and then failing to create a subdirectory of it (maybe
because the just-created parent directory ended up with the wrong permissions?)
mkdir("./objects/incoming-U5EN8D", 0700
<... mkdir resumed> ) = 0
rt_sigaction(SIGINT, {0x56a860, [INT], SA_RESTORER|SA_RESTART,
0x7f842cb3b2f0},
<... rt_sigaction resumed> {SIG_IGN, [], 0}, 8) = 0
rt_sigaction(SIGHUP, {0x56a860, [HUP], SA_RESTORER|SA_RESTART,
0x7f842cb3b2f0}, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGTERM, {0x56a860, [TERM], SA_RESTORER|SA_RESTART,
0x7f842cb3b2f0}, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGQUIT, {0x56a860, [QUIT], SA_RESTORER|SA_RESTART,
0x7f842cb3b2f0}, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGPIPE, {0x56a860, [PIPE], SA_RESTORER|SA_RESTART,
0x7f842cb3b2f0}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART, 0x7f842cb3b2f0}, 8)
= 0
mkdir("./objects/incoming-U5EN8D/pack", 0777) = -1 EACCES (Permission denied)
--
Jamie Zawinski https://www.jwz.org/ https://www.dnalounge.com/
Re: [PATCH v3 07/14] checkout: split into switch-branch and restore-files
Elijah Newren writes: >> +Updates files in the working tree to match the version in the index >> +or the specified tree. >> + >> +'git restore-files' [--from=] ...:: > > and ? I understand and , > or but have no clue why it'd be okay to specify > and together. What does that even mean? I have this tree object v2.6.11-tree that is not enclosed in a commit object. I want to take the top-level Makefile out of that tree, stuff it in the index and overwrite the working tree file. $ git checkout v2.6.11-tree Makefile $ git restore-files --from=v2.6.11-tree Makefile >> + Overwrite paths in the working tree by replacing with the >> + contents in the index or in the (most often a >> + commit). When a is given, the paths that >> + match the are updated both in the index and in >> + the working tree. > > Is that the default we really want for this command? Why do we > automatically assume these files are ready for commit? I understand > that it's what checkout did, but I'd find it more natural to only > affect the working tree by default. We can give it an option for > affecting the index instead (or perhaps in addition). Oooah. Now this is getting juicy. I do think supporting "--index" (which would make it more in line with what Duy wrote), with optionally "--cached" as well, and making the "working tree only" mode as default may not be a bad idea. I am offhand not sure how the "working tree only" mode (similar to the default mode of "git apply" that mimics the way "patch -p1" works) should interact with the non-overlay mode of the command, but other than that, I tend to agree with the idea that restore-files is only a part of making the contents into committable shape, not exactly ready for it yet.
Re: [PATCH] sideband: color lines with keyword only
Jonathan Nieder writes:
> Stefan Beller wrote:
>> On Mon, Dec 3, 2018 at 3:23 PM Jonathan Nieder wrote:
>
>>> I was curious about what versions of Gerrit this is designed to
>>> support (or in other words whether it's a bug fix or a feature).
Well, bf1a11f0 ("sideband: highlight keywords in remote sideband
output", 2018-08-07) clearly wanted to allow a keyword followed by
anything !isalnum() to be painted, and we accepted that change
because we thought it was a good idea, so anything that made a
keyword alone not to be painted is a bug, isn't it? Whether output
lines from Gerrit benefits from this fix is a different matter, of
course.
> No worries. Can't hurt for Junio to have a few patches to apply to
> "pu" or "next" to practice using the release candidates. :)
This change falls into "an obvious and small fix to a bug that went
unnoticed and is in an older release (2.19)" category, which is not
eligible for the upcoming release this late in the cycle. I think
enough eyeballs looked at the change already, so let's not waste the
already-spent review braincycle and mark it as "Will merge to 'next'".
Re: [PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
Johannes Sixt writes: > Am 03.12.18 um 21:42 schrieb Martin Ågren: >> On Mon, 3 Dec 2018 at 18:35, Johannes Sixt wrote: >>> I actually did not test the result, because I don't have the >>> infrastructure. >> >> I've tested with asciidoc and Asciidoctor, html and man-page. Looks >> good. > > Thank you so much! > > -- Hannes Thanks, both.
Re: [RFC] git clean --local
Junio C Hamano writes: > If "git clean" takes a pathspec, perhaps you can give a negative > pathspec to exclude whatever you do not want to get cleaned, > something like > > git clean '*.o' ':!precious.o' > > to say "presious.o is ignored (hence normally expendable), but I do > not want to clean it with this invocation of 'git clean'"? Hmph, this leads me to an interesting thought. With today's code, these two commands behave in meaningfully different ways when I mark some paths that match .gitignore patterns with the precious attribute. echo "*.ignored" >>.git/info/exclude echo "precious.* precious" >>.git/info/attributes : >expendable.ignored 2>precious.ignored git clean -n -x git clean -n -x ':(exclude,attr:precious)' I am not suggesting that giving "git clean" a configuration knob that always append pathspec elements, which would allow users to use the mechanism to set the above magic pathspec, would be a good approach. If we were to follow through this line of thought, an obvious thing to do is to always unconditonally append the above magic pathspec internally when running "git clean", which would mean * Existing projects and users' repositories will see no behaviour change, because they are unaware of the "precious" attribute. * People who learn the new feature can start using the "ignored but precious" class, without any need for transition period.
Re: [PATCH 3/3] RelNotes 2.20: drop spurious double quote
Martin Ågren writes: > We have three double-quote characters, which is one too many or too few. > Dropping the last one seems to match the original intention best. Thanks for spotting. The actual original intention was that the user says two things: first saying "add only what does not match '*' out of all branches" and then saying "add all branches, without any exclusion this time". But letting the user first say one thing and then doing another thing without saying it is also fine, which is what your version is. > > Signed-off-by: Martin Ågren > --- > Documentation/RelNotes/2.20.0.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/RelNotes/2.20.0.txt > b/Documentation/RelNotes/2.20.0.txt > index 201135d80c..e71fe3dee1 100644 > --- a/Documentation/RelNotes/2.20.0.txt > +++ b/Documentation/RelNotes/2.20.0.txt > @@ -578,7 +578,7 @@ Fixes since v2.19 > > * "git rev-parse --exclude=* --branches --branches" (i.e. first > saying "add only things that do not match '*' out of all branches" > - and then adding all branches, without any exclusion this time") > + and then adding all branches, without any exclusion this time) > worked as expected, but "--exclude=* --all --all" did not work the > same way, which has been fixed. > (merge 5221048092 ag/rev-parse-all-exclude-fix later to maint).
Re: [PATCH 2/3] RelNotes 2.20: clarify sentence
Martin Ågren writes: > I had to read this sentence a few times to understand it. Let's try to > clarify it. Great. Thanks. > > Signed-off-by: Martin Ågren > --- > Documentation/RelNotes/2.20.0.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/RelNotes/2.20.0.txt > b/Documentation/RelNotes/2.20.0.txt > index e5ab8cc609..201135d80c 100644 > --- a/Documentation/RelNotes/2.20.0.txt > +++ b/Documentation/RelNotes/2.20.0.txt > @@ -305,7 +305,7 @@ Performance, Internal Implementation, Development Support > etc. > > * The overly large Documentation/config.txt file have been split into > million little pieces. This potentially allows each individual piece > - included into the manual page of the command it affects more easily. > + to be included into the manual page of the command it affects more easily. > > * Replace three string-list instances used as look-up tables in "git > fetch" with hashmaps.
Re: [PATCH 1/3] RelNotes 2.20: move some items between sections
Martin Ågren writes: > Some items that should be in "Performance, Internal Implementation, > Development Support etc." have ended up in "UI, Workflows & Features" > and "Fixes since v2.19". Move them, and do s/uses/use/ while at it. > > Signed-off-by: Martin Ågren > --- I agree with the early half of this change; I think it is OK to consider lack of preparation for Travis transition and lack of better testing in the maintenance track as bugs, though. > Documentation/RelNotes/2.20.0.txt | 26 +- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/Documentation/RelNotes/2.20.0.txt > b/Documentation/RelNotes/2.20.0.txt > index b1deaf37da..e5ab8cc609 100644 > --- a/Documentation/RelNotes/2.20.0.txt > +++ b/Documentation/RelNotes/2.20.0.txt > @@ -137,11 +137,6 @@ UI, Workflows & Features > command line, or setting sendemail.suppresscc configuration > variable to "misc-by", can be used to disable this behaviour. > > - * Developer builds now uses -Wunused-function compilation option. > - > - * One of our CI tests to run with "unusual/experimental/random" > - settings now also uses commit-graph and midx. > - > * "git mergetool" learned to take the "--[no-]gui" option, just like > "git difftool" does. > > @@ -185,6 +180,11 @@ UI, Workflows & Features > > Performance, Internal Implementation, Development Support etc. > > + * Developer builds now use -Wunused-function compilation option. > + > + * One of our CI tests to run with "unusual/experimental/random" > + settings now also uses commit-graph and midx. > + > * When there are too many packfiles in a repository (which is not > recommended), looking up an object in these would require > consulting many pack .idx files; a new mechanism to have a single > @@ -387,6 +387,14 @@ Performance, Internal Implementation, Development > Support etc. > two classes to ease code migration process has been proposed and > its support has been added to the Makefile. > > + * The "container" mode of TravisCI is going away. Our .travis.yml > + file is getting prepared for the transition. > + (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). > + > + * Our test scripts can now take the '-V' option as a synonym for the > + '--verbose-log' option. > + (merge a5f52c6dab sg/test-verbose-log later to maint). > + > > Fixes since v2.19 > - > @@ -544,14 +552,6 @@ Fixes since v2.19 > didn't make much sense. This has been corrected. > (merge 669b1d2aae md/exclude-promisor-objects-fix later to maint). > > - * The "container" mode of TravisCI is going away. Our .travis.yml > - file is getting prepared for the transition. > - (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). > - > - * Our test scripts can now take the '-V' option as a synonym for the > - '--verbose-log' option. > - (merge a5f52c6dab sg/test-verbose-log later to maint). > - > * A regression in Git 2.12 era made "git fsck" fall into an infinite > loop while processing truncated loose objects. > (merge 18ad13e5b2 jk/detect-truncated-zlib-input later to maint).
Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
Jeff King writes:
> That said, our C99 designated initializer weather-balloons haven't
> gotten any complaints yet. So I think you could actually do:
>
> struct setup_revision_opt s_r_opt = {
> .allow_exclude_promisor_objects = 1,
> };
> ...
> setup_revisions(...);
>
> which is pretty nice.
Yup. The output from
$ git grep -n ' \.[a-z0-9_]* =' -- \*.[ch]
with a bit of "git blame" tells us that cbc0f81d ("strbuf: use
designated initializers in STRBUF_INIT", 2017-07-10) is the balloon
for this exact feature. The same for array was done in 512f41cf
("clean.c: use designated initializer", 2017-07-14)
[I am writing it down so that I do not have to dig for it every time
and instead can ask the list archive]
Re: [WIP RFC 2/5] Documentation: add Packfile URIs design doc
On Mon, Dec 03, 2018 at 03:37:35PM -0800, Jonathan Tan wrote: > Signed-off-by: Jonathan Tan > --- > Documentation/technical/packfile-uri.txt | 83 > Documentation/technical/protocol-v2.txt | 6 +- > 2 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 Documentation/technical/packfile-uri.txt > > diff --git a/Documentation/technical/packfile-uri.txt > b/Documentation/technical/packfile-uri.txt > new file mode 100644 > index 00..6535801486 > --- /dev/null > +++ b/Documentation/technical/packfile-uri.txt > @@ -0,0 +1,83 @@ > +Packfile URIs > += > + > +This feature allows servers to serve part of their packfile response as URIs. > +This allows server designs that improve scalability in bandwidth and CPU > usage > +(for example, by serving some data through a CDN), and (in the future) > provides > +some measure of resumability to clients. > + > +This feature is available only in protocol version 2. > + > +Protocol > + > + > +The server advertises `packfile-uris`. > + > +If the client replies with the following arguments: > + > + * packfile-uris > + * thin-pack > + * ofs-delta > + > +when the server sends the packfile, it MAY send a `packfile-uris` section > +directly before the `packfile` section (right after `wanted-refs` if it is > +sent) containing HTTP(S) URIs. See protocol-v2.txt for the documentation of > +this section. > + > +Clients then should understand that the returned packfile could be > incomplete, > +and that it needs to download all the given URIs before the fetch or clone is > +complete. Each URI should point to a Git packfile (which may be a thin pack > and > +which may contain offset deltas). Some thoughts here: First, I'd like to see a section (and a bit in the implementation) requiring HTTPS if the original protocol is secure (SSH or HTTPS). Allowing the server to downgrade to HTTP, even by accident, would be a security problem. Second, this feature likely should be opt-in for SSH. One issue I've seen repeatedly is that people don't want to use HTTPS to fetch things when they're using SSH for Git. Many people in corporate environments have proxies that break HTTP for non-browser use cases[0], and using SSH is the only way that they can make a functional Git connection. Third, I think the server needs to be required to both support Range headers and never change the content of a URI, so that we can have resumable clone implicit in this design. There are some places in the world where connections are poor and fetching even the initial packfile at once might be a problem. (I've seen such questions on Stack Overflow, for example.) Having said that, I think overall this is a good idea and I'm glad to see a proposal for it. [0] For example, a naughty-word filter may corrupt or block certain byte sequences that occur incidentally in the pack stream. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v3] range-diff: always pass at least minimal diff options
Eric Sunshine writes: > This is a re-roll of Martin's v2[1]. The only difference from v2 is that > it retains coloring when emitting to the terminal (plus an in-code > comment was simplified). > > The regression introduced by d8981c3f88, in which the range-diff only > ever gets emitted to the terminal, and never to the cover letter or > commentary section of a standalone patch, makes the --range-diff option > rather useless, so this fix probably ought to be fast-tracked. Yup. Thanks. The only thing that makes me wonder is why any of the existing tests (among which I think I saw range-diff driven from format-patch) did not catch this rather obvious glitch. > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > index e497c1358f..048feaf6dd 100755 > --- a/t/t3206-range-diff.sh > +++ b/t/t3206-range-diff.sh > @@ -248,18 +248,24 @@ test_expect_success 'dual-coloring' ' > for prev in topic master..topic > do > test_expect_success "format-patch --range-diff=$prev" ' > - git format-patch --stdout --cover-letter --range-diff=$prev \ > + git format-patch --cover-letter --range-diff=$prev \ > master..unmodified >actual && Ah, of course. Then the "actual" file gets the names of the output files; we expect to see 5 of them. But now we have lost all the range-diff tests run under "--stdout", so next time some other change regresses only that codepath, we will not notice (which is fine for now but would want to be addressed before the end of the year around which time we certainly will all forget). Thanks. > - grep "= 1: .* s/5/A" actual && > - grep "= 2: .* s/4/A" actual && > - grep "= 3: .* s/11/B" actual && > - grep "= 4: .* s/12/B" actual > + test_when_finished "rm 000?-*" && > + test_line_count = 5 actual && > + test_i18ngrep "^Range-diff:$" -* && > + grep "= 1: .* s/5/A" -* && > + grep "= 2: .* s/4/A" -* && > + grep "= 3: .* s/11/B" -* && > + grep "= 4: .* s/12/B" -* > ' > done > > test_expect_success 'format-patch --range-diff as commentary' ' > - git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && > - test_i18ngrep "^Range-diff:$" actual > + git format-patch --range-diff=HEAD~1 HEAD~1 >actual && > + test_when_finished "rm 0001-*" && > + test_line_count = 1 actual && > + test_i18ngrep "^Range-diff:$" 0001-* && > + grep "> 1: .* new message" 0001-* > ' > > test_done
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Thu, Nov 29, 2018 at 2:01 PM Nguyễn Thái Ngọc Duy wrote: > > v3 sees switch-branch go back to switch-branch (in v2 it was > checkout-branch). checkout-files is also renamed restore-files (v1 was > restore-paths). Hopefully we won't see another rename. I started reading through the patches. I also tried to apply them locally, but they had conflicts or missing base file version on both master and next. What version did you base it on? I stopped at 07/14, and dropped my comments all there. I didn't read any further yet, and may wait for your post-2.20 reroll. > I'll try to summarize the differences between the new commands and > 'git checkout' down here, but you're welcome to just head to 07/14 and > read the new man pages. > > 'git switch-branch' > > - does not "do nothing", you have to either switch branch, create a > new branch, or detach. "git switch-branch" with no arguments is > rejected. > > - implicit detaching is rejected. If you need to detach, you need to > give --detach. Or stick to 'git checkout'. > > - -b/-B is renamed to -c/-C with long option names > > - of course does not accept pathspec > > 'git restore-files' > > - takes a ref from --from argument, not as a free ref. As a result, > '--' is no longer needed. All non-option arguments are pathspec > > - pathspec is mandatory, you can't do "git restore-files" without any > pathspec. > > - I just remember -p which is allowed to take no pathspec :( I'll fix > it later. This all looks good. I commented elsewhere but please remember that pathspec implies directories as a possibility and we really need to fix the broken behavior of checkout when given a directory. > - Two more fancy features (the "git checkout --index" being the > default mode and the backup log for accidental overwrites) are of > course still missing. But they are coming. > > I did not go replace "detached HEAD" with "unnamed branch" (or "no > branch") everywhere because I think a unique term is still good to > refer to this concept. Or maybe "no branch" is good enough. I dunno. I personally like "unnamed branch", but "no branch" would still be better than "detached HEAD".
Re: [PATCH v3 07/14] checkout: split into switch-branch and restore-files
On Thu, Nov 29, 2018 at 2:03 PM Nguyễn Thái Ngọc Duy wrote: > > "git checkout" doing too many things is a source of confusion for many > users (and it even bites old timers sometimes). To rememdy that, the > command is now split in two: switch-branch and checkout-files. The "checkout-files" here(will comment more on this below) > good old "git checkout" command is still here and will be until all > (or most of users) are sick of it. > > See the new man pages for the final design of these commands. The > actual implementation though is still pretty much the same as "git > checkout". Following patches will adjust their behavior to match the > man pages. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > .gitignore | 2 + > Documentation/git-checkout.txt | 5 + > Documentation/git-restore-files.txt | 167 > Documentation/git-switch-branch.txt | 289 > Makefile| 2 + > builtin.h | 2 + > builtin/checkout.c | 84 ++-- > command-list.txt| 2 + > git.c | 2 + > 9 files changed, 543 insertions(+), 12 deletions(-) > create mode 100644 Documentation/git-restore-files.txt > create mode 100644 Documentation/git-switch-branch.txt > > diff --git a/.gitignore b/.gitignore > index 0d77ea5894..c63dcb1427 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -143,6 +143,7 @@ > /git-request-pull > /git-rerere > /git-reset > +/git-restore-files ...and "restore-files" here. Should be consistent with whatever name you pick. > /git-rev-list > /git-rev-parse > /git-revert > @@ -167,6 +168,7 @@ > /git-submodule > /git-submodule--helper > /git-svn > +/git-switch-branch > /git-symbolic-ref > /git-tag > /git-unpack-file > diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt > index 25887a6087..25ec7f508f 100644 > --- a/Documentation/git-checkout.txt > +++ b/Documentation/git-checkout.txt > @@ -406,6 +406,11 @@ $ edit frotz > $ git add frotz > > > +SEE ALSO > + > +linkgit:git-switch-branch[1] > +linkgit:git-restore-files[1] > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/Documentation/git-restore-files.txt > b/Documentation/git-restore-files.txt > new file mode 100644 > index 00..03c1250ad0 > --- /dev/null > +++ b/Documentation/git-restore-files.txt > @@ -0,0 +1,167 @@ > +git-restore-files(1) > + > + > +NAME > + > +git-restore-files - Restore working tree files > + > +SYNOPSIS > + > +[verse] > +'git restore-files'
Re: [WIP RFC 3/5] upload-pack: refactor reading of pack-objects out
On Mon, Dec 3, 2018 at 3:37 PM Jonathan Tan wrote: > > Subsequent patches will change how the output of pack-objects is > processed, so extract that processing into its own function. > > Currently, at most 1 character can be buffered (in the "buffered" local > variable). One of those patches will require a larger buffer, so replace > that "buffered" local variable with a buffer array. This buffering sounds oddly similar to the pkt reader which can buffer at most one pkt, the difference being that we'd buffer bytes instead of pkts.
Re: [WIP RFC 2/5] Documentation: add Packfile URIs design doc
Thanks for bringing this design to the list!
> diff --git a/Documentation/technical/protocol-v2.txt
> b/Documentation/technical/protocol-v2.txt
> index 345c00e08c..2cb1c41742 100644
> --- a/Documentation/technical/protocol-v2.txt
> +++ b/Documentation/technical/protocol-v2.txt
> @@ -313,7 +313,8 @@ header. Most sections are sent only when the packfile is
> sent.
>
> output = acknowledgements flush-pkt |
> [acknowledgments delim-pkt] [shallow-info delim-pkt]
> -[wanted-refs delim-pkt] packfile flush-pkt
> +[wanted-refs delim-pkt] [packfile-uris delim-pkt]
> +packfile flush-pkt
While this is an RFC and incomplete, we'd need to remember to
add packfile-uris to the capabilities list above, stating that it requires
thin-pack and ofs-delta to be sent, and what to expect from it.
The mention of --no-packfile-urls in the Client design above
seems to imply we'd want to turn it on by default, which I thought
was not the usual stance how we introduce new things.
An odd way of disabling it would be --no-thin-pack, hoping the
client side implementation abides by the implied requirements.
> acknowledgments = PKT-LINE("acknowledgments" LF)
> (nak | *ack)
> @@ -331,6 +332,9 @@ header. Most sections are sent only when the packfile is
> sent.
> *PKT-LINE(wanted-ref LF)
> wanted-ref = obj-id SP refname
>
> +packfile-uris = PKT-LINE("packfile-uris" LF) *packfile-uri
> +packfile-uri = PKT-LINE("uri" SP *%x20-ff LF)
Is the *%x20-ff a fancy way of saying obj-id?
While the server is configured with pairs of (oid URL),
we would not need to send the exact oid to the client
as that is what the client can figure out on its own by reading
the downloaded pack.
Instead we could send an integrity hash (i.e. the packfile
downloaded from "uri" is expected to hash to $oid here)
Thanks,
Stefan
Re: [WIP RFC 0/5] Design for offloading part of packfile response to CDN
On Mon, Dec 3, 2018 at 3:37 PM Jonathan Tan wrote: > > There is a potential issue: a server which produces both the URIs and > the packfile at roughly the same time (like the implementation in this > patch set) will not have sideband access until it has concluded sending > the URIs. Among other things, this means that the server cannot send > keepalive packets until quite late in the response. One solution to this > might be to add a feature that allows the server to use a sideband > throughout the whole response - and this has other benefits too like > allowing servers to inform the client throughout the whole fetch, not > just at the end. While side band sounds like the right thing to do, we could also sending (NULL)-URIs within this feature.
Re: [PATCH] pack-protocol.txt: accept error packets in any context
> diff --git a/pkt-line.c b/pkt-line.c
> index 04d10bbd0..ce9e42d10 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd,
> char **src_buffer,
> return PACKET_READ_EOF;
> }
>
> + if (starts_with(buffer, "ERR ")) {
> + die(_("remote error: %s"), buffer + 4);
> + }
> +
Handling any ERR line in the pkt reader is okay, as
* we do not pkt-ize pack files and
* we do not have any other parts of the protocol where user data is in
the first four bytes, which could randomly match this pattern and
* the rest of the pkt-ized part of the protocol never sends
ERR lines.
Makes sense.
Re: [PATCH] sideband: color lines with keyword only
Stefan Beller wrote: > On Mon, Dec 3, 2018 at 3:23 PM Jonathan Nieder wrote: >> I was curious about what versions of Gerrit this is designed to >> support (or in other words whether it's a bug fix or a feature). >> Looking at examples like [1], it seems that Gerrit historically always >> used "ERROR:" so the 59a255aef0 logic would work for it. More >> recently, [2] (ReceiveCommits: add a "SUCCESS" marker for successful >> change updates, 2018-08-21) put SUCCESS on a line of its own. That >> puts this squarely in the new-feature category. > > Ooops. From the internal bug, I assumed this to be long standing Gerrit > behavior, which is why I sent it out in -rc to begin with. No worries. Can't hurt for Junio to have a few patches to apply to "pu" or "next" to practice using the release candidates. :) [...] >> In the old code, we would escape early if 'n == len', but we didn't >> need to. If 'n == len', then >> >> src[len] == '\0' > > src[len] could also be one of "\n\r", see the caller > recv_sideband for sidebase case 2. Yes, I noticed too late[*]. Sorry for the noise. The patch still looks good. Jonathan [*] https://public-inbox.org/git/20181203233439.gb157...@google.com/
[WIP RFC 0/5] Design for offloading part of packfile response to CDN
Some of us have been working on a design to improve the scalability of Git servers by allowing them to offload part of the packfile response to CDNs in this way: returning HTTP(S) URIs in fetch responses in addition to packfiles. This can reduce the load on individual Git servers and improves proximity (by having data served from closer to the user). I have included here a design document (patch 2) and a rough implementation of the server (patch 5). Currently, the implementation only allows replacing single blobs with URIs, but the protocol improvement is designed in such a way as to allow independent improvement of Git server implementations. There is a potential issue: a server which produces both the URIs and the packfile at roughly the same time (like the implementation in this patch set) will not have sideband access until it has concluded sending the URIs. Among other things, this means that the server cannot send keepalive packets until quite late in the response. One solution to this might be to add a feature that allows the server to use a sideband throughout the whole response - and this has other benefits too like allowing servers to inform the client throughout the whole fetch, not just at the end. Jonathan Tan (5): Documentation: order protocol v2 sections Documentation: add Packfile URIs design doc upload-pack: refactor reading of pack-objects out upload-pack: refactor writing of "packfile" line upload-pack: send part of packfile response as uri Documentation/technical/packfile-uri.txt | 83 + Documentation/technical/protocol-v2.txt | 22 ++-- builtin/pack-objects.c | 48 fetch-pack.c | 9 ++ t/t5702-protocol-v2.sh | 25 upload-pack.c| 150 --- 6 files changed, 285 insertions(+), 52 deletions(-) create mode 100644 Documentation/technical/packfile-uri.txt -- 2.19.0.271.gfe8321ec05.dirty
[WIP RFC 1/5] Documentation: order protocol v2 sections
The git command line expects Git servers to follow a specific order of
sections when transmitting protocol v2 responses, but this is not
explicit in the documentation. Make the order explicit.
Signed-off-by: Jonathan Tan
---
Documentation/technical/protocol-v2.txt | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/Documentation/technical/protocol-v2.txt
b/Documentation/technical/protocol-v2.txt
index 09e4e0273f..345c00e08c 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -309,11 +309,11 @@ the 'wanted-refs' section in the server's response as
explained below.
The response of `fetch` is broken into a number of sections separated by
delimiter packets (0001), with each section beginning with its section
-header.
+header. Most sections are sent only when the packfile is sent.
-output = *section
-section = (acknowledgments | shallow-info | wanted-refs | packfile)
- (flush-pkt | delim-pkt)
+output = acknowledgements flush-pkt |
+[acknowledgments delim-pkt] [shallow-info delim-pkt]
+[wanted-refs delim-pkt] packfile flush-pkt
acknowledgments = PKT-LINE("acknowledgments" LF)
(nak | *ack)
@@ -335,9 +335,10 @@ header.
*PKT-LINE(%x01-03 *%x00-ff)
acknowledgments section
- * If the client determines that it is finished with negotiations
- by sending a "done" line, the acknowledgments sections MUST be
- omitted from the server's response.
+ * If the client determines that it is finished with negotiations by
+ sending a "done" line (thus requiring the server to send a packfile),
+ the acknowledgments sections MUST be omitted from the server's
+ response.
* Always begins with the section header "acknowledgments"
@@ -388,9 +389,6 @@ header.
which the client has not indicated was shallow as a part of
its request.
- * This section is only included if a packfile section is also
- included in the response.
-
wanted-refs section
* This section is only included if the client has requested a
ref using a 'want-ref' line and if a packfile section is also
--
2.19.0.271.gfe8321ec05.dirty
[WIP RFC 5/5] upload-pack: send part of packfile response as uri
This is a partial implementation of upload-pack sending part of its
packfile response as URIs.
The client is not fully implemented - it knows to ignore the
"packfile-uris" section, but because it does not actually fetch those
URIs, the returned packfile is incomplete. A test is included to show
that the appropriate URI is indeed transmitted, and that the returned
packfile is lacking exactly the expected object.
Signed-off-by: Jonathan Tan
---
builtin/pack-objects.c | 48 ++
fetch-pack.c | 9
t/t5702-protocol-v2.sh | 25 ++
upload-pack.c | 37
4 files changed, 115 insertions(+), 4 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e7ea206c08..2abbddd3cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -117,6 +117,15 @@ enum missing_action {
static enum missing_action arg_missing_action;
static show_object_fn fn_show_object;
+struct configured_exclusion {
+ struct oidmap_entry e;
+ char *uri;
+};
+static struct oidmap configured_exclusions;
+
+static int exclude_configured_blobs;
+static struct oidset excluded_by_config;
+
/*
* stats
*/
@@ -831,6 +840,23 @@ static off_t write_reused_pack(struct hashfile *f)
return reuse_packfile_offset - sizeof(struct pack_header);
}
+static void write_excluded_by_configs(void)
+{
+ struct oidset_iter iter;
+ const struct object_id *oid;
+
+ oidset_iter_init(_by_config, );
+ while ((oid = oidset_iter_next())) {
+ struct configured_exclusion *ex =
+ oidmap_get(_exclusions, oid);
+
+ if (!ex)
+ BUG("configured exclusion wasn't configured");
+ write_in_full(1, ex->uri, strlen(ex->uri));
+ write_in_full(1, "\n", 1);
+ }
+}
+
static const char no_split_warning[] = N_(
"disabling bitmap writing, packs are split due to pack.packSizeLimit"
);
@@ -1124,6 +1150,12 @@ static int want_object_in_pack(const struct object_id
*oid,
}
}
+ if (exclude_configured_blobs &&
+ oidmap_get(_exclusions, oid)) {
+ oidset_insert(_by_config, oid);
+ return 0;
+ }
+
return 1;
}
@@ -2728,6 +2760,19 @@ static int git_pack_config(const char *k, const char *v,
void *cb)
pack_idx_opts.version);
return 0;
}
+ if (!strcmp(k, "uploadpack.blobpackfileuri")) {
+ struct configured_exclusion *ex = xmalloc(sizeof(*ex));
+ const char *end;
+
+ if (parse_oid_hex(v, >e.oid, ) || *end != ' ')
+ die(_("value of uploadpack.blobpackfileuri must be "
+ "of the form ' ' (got '%s')"), v);
+ if (oidmap_get(_exclusions, >e.oid))
+ die(_("object already configured in another "
+ "uploadpack.blobpackfileuri (got '%s')"), v);
+ ex->uri = xstrdup(end + 1);
+ oidmap_put(_exclusions, ex);
+ }
return git_default_config(k, v, cb);
}
@@ -3314,6 +3359,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_("do not pack objects in promisor packfiles")),
OPT_BOOL(0, "delta-islands", _delta_islands,
N_("respect islands during delta compression")),
+ OPT_BOOL(0, "exclude-configured-blobs",
_configured_blobs,
+N_("respect uploadpack.blobpackfileuri")),
OPT_END(),
};
@@ -3487,6 +3534,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
return 0;
if (nr_result)
prepare_pack(window, depth);
+ write_excluded_by_configs();
write_pack_file();
if (progress)
fprintf_ln(stderr,
diff --git a/fetch-pack.c b/fetch-pack.c
index 9691046e64..6e1985ab55 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1413,6 +1413,15 @@ static struct ref *do_fetch_pack_v2(struct
fetch_pack_args *args,
receive_wanted_refs(, sought, nr_sought);
/* get the pack */
+ if (process_section_header(, "packfile-uris",
1)) {
+ /* skip the whole section */
+ process_section_header(,
"packfile-uris", 0);
+ while (packet_reader_read() ==
PACKET_READ_NORMAL) {
+ /* do nothing */
+ }
+ if (reader.status != PACKET_READ_DELIM)
+ die("expected DELIM");
+ }
process_section_header(, "packfile", 0);
[WIP RFC 4/5] upload-pack: refactor writing of "packfile" line
A subsequent patch allows pack-objects to output additional information
(in addition to the packfile that it currently outputs). This means that
we must hold off on writing the "packfile" section header to the client
before we process the output of pack-objects, so move the writing of
the "packfile" section header to read_pack_objects_stdout().
Unfortunately, this also means that we cannot send keepalive packets
until pack-objects starts sending out the packfile, since the sideband
is only established when the "packfile" section header is sent.
Signed-off-by: Jonathan Tan
---
upload-pack.c | 47 ---
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index cec43e0a46..aa2589b858 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -104,9 +104,12 @@ static int write_one_shallow(const struct commit_graft
*graft, void *cb_data)
struct output_state {
char buffer[8193];
int used;
+ unsigned packfile_started : 1;
+ struct strbuf progress_buf;
};
-static int read_pack_objects_stdout(int outfd, struct output_state *os)
+static int read_pack_objects_stdout(int outfd, struct output_state *os,
+ int use_protocol_v2)
{
/* Data ready; we keep the last byte to ourselves
* in case we detect broken rev-list, so that we
@@ -126,6 +129,12 @@ static int read_pack_objects_stdout(int outfd, struct
output_state *os)
}
os->used += readsz;
+ if (!os->packfile_started) {
+ os->packfile_started = 1;
+ if (use_protocol_v2)
+ packet_write_fmt(1, "packfile\n");
+ }
+
if (os->used > 1) {
send_client_data(1, os->buffer, os->used - 1);
os->buffer[0] = os->buffer[os->used - 1];
@@ -138,8 +147,17 @@ static int read_pack_objects_stdout(int outfd, struct
output_state *os)
return readsz;
}
+static void flush_progress_buf(struct strbuf *progress_buf)
+{
+ if (!progress_buf->len)
+ return;
+ send_client_data(2, progress_buf->buf, progress_buf->len);
+ strbuf_reset(progress_buf);
+}
+
static void create_pack_file(const struct object_array *have_obj,
-const struct object_array *want_obj)
+const struct object_array *want_obj,
+int use_protocol_v2)
{
struct child_process pack_objects = CHILD_PROCESS_INIT;
struct output_state output_state = {0};
@@ -260,9 +278,13 @@ static void create_pack_file(const struct object_array
*have_obj,
*/
sz = xread(pack_objects.err, progress,
sizeof(progress));
- if (0 < sz)
- send_client_data(2, progress, sz);
- else if (sz == 0) {
+ if (0 < sz) {
+ if (output_state.packfile_started)
+ send_client_data(2, progress, sz);
+ else
+ strbuf_add(_state.progress_buf,
+ progress, sz);
+ } else if (sz == 0) {
close(pack_objects.err);
pack_objects.err = -1;
}
@@ -273,8 +295,10 @@ static void create_pack_file(const struct object_array
*have_obj,
}
if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
int result = read_pack_objects_stdout(pack_objects.out,
- _state);
-
+ _state,
+ use_protocol_v2);
+ if (output_state.packfile_started)
+ flush_progress_buf(_state.progress_buf);
if (result == 0) {
close(pack_objects.out);
pack_objects.out = -1;
@@ -293,12 +317,14 @@ static void create_pack_file(const struct object_array
*have_obj,
* protocol to say anything, so those clients are just out of
* luck.
*/
- if (!ret && use_sideband) {
+ if (!ret && use_sideband && output_state.packfile_started) {
static const char buf[] = "0005\1";
write_or_die(1, buf, 5);
}
}
+ flush_progress_buf(_state.progress_buf);
+
if (finish_command(_objects)) {
error("git upload-pack: git-pack-objects died with error.");
goto fail;
@@ -1094,7 +1120,7 @@ void upload_pack(struct upload_pack_options
[WIP RFC 2/5] Documentation: add Packfile URIs design doc
Signed-off-by: Jonathan Tan
---
Documentation/technical/packfile-uri.txt | 83
Documentation/technical/protocol-v2.txt | 6 +-
2 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644 Documentation/technical/packfile-uri.txt
diff --git a/Documentation/technical/packfile-uri.txt
b/Documentation/technical/packfile-uri.txt
new file mode 100644
index 00..6535801486
--- /dev/null
+++ b/Documentation/technical/packfile-uri.txt
@@ -0,0 +1,83 @@
+Packfile URIs
+=
+
+This feature allows servers to serve part of their packfile response as URIs.
+This allows server designs that improve scalability in bandwidth and CPU usage
+(for example, by serving some data through a CDN), and (in the future) provides
+some measure of resumability to clients.
+
+This feature is available only in protocol version 2.
+
+Protocol
+
+
+The server advertises `packfile-uris`.
+
+If the client replies with the following arguments:
+
+ * packfile-uris
+ * thin-pack
+ * ofs-delta
+
+when the server sends the packfile, it MAY send a `packfile-uris` section
+directly before the `packfile` section (right after `wanted-refs` if it is
+sent) containing HTTP(S) URIs. See protocol-v2.txt for the documentation of
+this section.
+
+Clients then should understand that the returned packfile could be incomplete,
+and that it needs to download all the given URIs before the fetch or clone is
+complete. Each URI should point to a Git packfile (which may be a thin pack and
+which may contain offset deltas).
+
+Server design
+-
+
+The server can be trivially made compatible with the proposed protocol by
+having it advertise `packfile-uris`, tolerating the client sending
+`packfile-uris`, and never sending any `packfile-uris` section. But we should
+include some sort of non-trivial implementation in the Minimum Viable Product,
+at least so that we can test the client.
+
+This is the implementation: a feature, marked experimental, that allows the
+server to be configured by one or more `uploadpack.blobPackfileUri=
+` entries. Whenever the list of objects to be sent is assembled, a blob
+with the given sha1 can be replaced by the given URI. This allows, for example,
+servers to delegate serving of large blobs to CDNs.
+
+Client design
+-
+
+While fetching, the client needs to remember the list of URIs and cannot
+declare that the fetch is complete until all URIs have been downloaded as
+packfiles.
+
+The division of work (initial fetch + additional URIs) introduces convenient
+points for resumption of an interrupted clone - such resumption can be done
+after the Minimum Viable Product (see "Future work").
+
+The client can inhibit this feature (i.e. refrain from sending the
+`packfile-urls` parameter) by passing --no-packfile-urls to `git fetch`.
+
+Future work
+---
+
+The protocol design allows some evolution of the server and client without any
+need for protocol changes, so only a small-scoped design is included here to
+form the MVP. For example, the following can be done:
+
+ * On the server, a long-running process that takes in entire requests and
+ outputs a list of URIs and the corresponding inclusion and exclusion sets of
+ objects. This allows, e.g., signed URIs to be used and packfiles for common
+ requests to be cached.
+ * On the client, resumption of clone. If a clone is interrupted, information
+ could be recorded in the repository's config and a "clone-resume" command
+ can resume the clone in progress. (Resumption of subsequent fetches is more
+ difficult because that must deal with the user wanting to use the repository
+ even after the fetch was interrupted.)
+
+There are some possible features that will require a change in protocol:
+
+ * Additional HTTP headers (e.g. authentication)
+ * Byte range support
+ * Different file formats referenced by URIs (e.g. raw object)
+
diff --git a/Documentation/technical/protocol-v2.txt
b/Documentation/technical/protocol-v2.txt
index 345c00e08c..2cb1c41742 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -313,7 +313,8 @@ header. Most sections are sent only when the packfile is
sent.
output = acknowledgements flush-pkt |
[acknowledgments delim-pkt] [shallow-info delim-pkt]
-[wanted-refs delim-pkt] packfile flush-pkt
+[wanted-refs delim-pkt] [packfile-uris delim-pkt]
+packfile flush-pkt
acknowledgments = PKT-LINE("acknowledgments" LF)
(nak | *ack)
@@ -331,6 +332,9 @@ header. Most sections are sent only when the packfile is
sent.
*PKT-LINE(wanted-ref LF)
wanted-ref = obj-id SP refname
+packfile-uris = PKT-LINE("packfile-uris" LF) *packfile-uri
+packfile-uri = PKT-LINE("uri" SP *%x20-ff LF)
+
packfile = PKT-LINE("packfile" LF)
*PKT-LINE(%x01-03 *%x00-ff)
--
2.19.0.271.gfe8321ec05.dirty
[WIP RFC 3/5] upload-pack: refactor reading of pack-objects out
Subsequent patches will change how the output of pack-objects is
processed, so extract that processing into its own function.
Currently, at most 1 character can be buffered (in the "buffered" local
variable). One of those patches will require a larger buffer, so replace
that "buffered" local variable with a buffer array.
Signed-off-by: Jonathan Tan
---
upload-pack.c | 80 +--
1 file changed, 46 insertions(+), 34 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 5e81f1ff24..cec43e0a46 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -101,14 +101,51 @@ static int write_one_shallow(const struct commit_graft
*graft, void *cb_data)
return 0;
}
+struct output_state {
+ char buffer[8193];
+ int used;
+};
+
+static int read_pack_objects_stdout(int outfd, struct output_state *os)
+{
+ /* Data ready; we keep the last byte to ourselves
+* in case we detect broken rev-list, so that we
+* can leave the stream corrupted. This is
+* unfortunate -- unpack-objects would happily
+* accept a valid packdata with trailing garbage,
+* so appending garbage after we pass all the
+* pack data is not good enough to signal
+* breakage to downstream.
+*/
+ ssize_t readsz;
+
+ readsz = xread(outfd, os->buffer + os->used,
+ sizeof(os->buffer) - os->used);
+ if (readsz < 0) {
+ return readsz;
+ }
+ os->used += readsz;
+
+ if (os->used > 1) {
+ send_client_data(1, os->buffer, os->used - 1);
+ os->buffer[0] = os->buffer[os->used - 1];
+ os->used = 1;
+ } else {
+ send_client_data(1, os->buffer, os->used);
+ os->used = 0;
+ }
+
+ return readsz;
+}
+
static void create_pack_file(const struct object_array *have_obj,
const struct object_array *want_obj)
{
struct child_process pack_objects = CHILD_PROCESS_INIT;
- char data[8193], progress[128];
+ struct output_state output_state = {0};
+ char progress[128];
char abort_msg[] = "aborting due to possible repository "
"corruption on the remote side.";
- int buffered = -1;
ssize_t sz;
int i;
FILE *pipe_fd;
@@ -235,39 +272,15 @@ static void create_pack_file(const struct object_array
*have_obj,
continue;
}
if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
- /* Data ready; we keep the last byte to ourselves
-* in case we detect broken rev-list, so that we
-* can leave the stream corrupted. This is
-* unfortunate -- unpack-objects would happily
-* accept a valid packdata with trailing garbage,
-* so appending garbage after we pass all the
-* pack data is not good enough to signal
-* breakage to downstream.
-*/
- char *cp = data;
- ssize_t outsz = 0;
- if (0 <= buffered) {
- *cp++ = buffered;
- outsz++;
- }
- sz = xread(pack_objects.out, cp,
- sizeof(data) - outsz);
- if (0 < sz)
- ;
- else if (sz == 0) {
+ int result = read_pack_objects_stdout(pack_objects.out,
+ _state);
+
+ if (result == 0) {
close(pack_objects.out);
pack_objects.out = -1;
- }
- else
+ } else if (result < 0) {
goto fail;
- sz += outsz;
- if (1 < sz) {
- buffered = data[sz-1] & 0xFF;
- sz--;
}
- else
- buffered = -1;
- send_client_data(1, data, sz);
}
/*
@@ -292,9 +305,8 @@ static void create_pack_file(const struct object_array
*have_obj,
}
/* flush the data */
- if (0 <= buffered) {
- data[0] = buffered;
- send_client_data(1, data, 1);
+ if (output_state.used > 0) {
+ send_client_data(1, output_state.buffer, output_state.used);
fprintf(stderr, "flushed.\n");
}
if (use_sideband)
--
2.19.0.271.gfe8321ec05.dirty
Re: [PATCH] sideband: color lines with keyword only
On Mon, Dec 3, 2018 at 3:23 PM Jonathan Nieder wrote:
> I was curious about what versions of Gerrit this is designed to
> support (or in other words whether it's a bug fix or a feature).
> Looking at examples like [1], it seems that Gerrit historically always
> used "ERROR:" so the 59a255aef0 logic would work for it. More
> recently, [2] (ReceiveCommits: add a "SUCCESS" marker for successful
> change updates, 2018-08-21) put SUCCESS on a line of its own. That
> puts this squarely in the new-feature category.
Ooops. From the internal bug, I assumed this to be long standing Gerrit
behavior, which is why I sent it out in -rc to begin with.
> > --- a/sideband.c
> > +++ b/sideband.c
> > @@ -87,7 +87,7 @@ static void maybe_colorize_sideband(struct strbuf *dest,
> > const char *src, int n)
> > struct keyword_entry *p = keywords + i;
> > int len = strlen(p->keyword);
> >
> > - if (n <= len)
> > + if (n < len)
> > continue;
>
> In the old code, we would escape early if 'n == len', but we didn't
> need to. If 'n == len', then
>
> src[len] == '\0'
src[len] could also be one of "\n\r", see the caller
recv_sideband for sidebase case 2.
> src .. [len-1] is a valid buffer to read from
>
> so the strncasecmp and strbuf_add operations used in this function are
> valid. Good.
Yes, they are all valid...
> > - if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len]))
> > {
> > + if (!strncasecmp(p->keyword, src, len) &&
> > + (len == n || !isalnum(src[len]))) {
>
> Our custom isalnum treats '\0' as not alphanumeric (sane_ctype[0] ==
> GIT_CNTRL) so this part of the patch is unnecessary. That said, it's
> good for clarity and defensive programming.
... but here we need to check for src[len] for validity.
I made no assumptions about isalnum, but rather needed to shortcut
the condition, as accessing src[len] would be out of bounds, no?
>
> > strbuf_addstr(dest, p->color);
> > strbuf_add(dest, src, len);
unlike here (or the rest of the block), where len is used correctly.
Re: [PATCH] sideband: color lines with keyword only
Jonathan Nieder wrote:
> Stefan Beller wrote:
>> /*
>> * Match case insensitively, so we colorize output from existing
>> @@ -95,7 +95,8 @@ static void maybe_colorize_sideband(struct strbuf *dest,
>> const char *src, int n)
>> * messages. We only highlight the word precisely, so
>> * "successful" stays uncolored.
>> */
>> -if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) {
>> +if (!strncasecmp(p->keyword, src, len) &&
>> +(len == n || !isalnum(src[len]))) {
>
> Our custom isalnum treats '\0' as not alphanumeric (sane_ctype[0] ==
> GIT_CNTRL) so this part of the patch is unnecessary. That said, it's
> good for clarity and defensive programming.
Correction: I am being silly here. src[len] can be '\0', '\n', or
'\r' --- it's not always '\0'. And the contract of this function is
that src[len] could be anything. Thanks for having handled it
correctly. :)
Jonathan
Re: [PATCH] sideband: color lines with keyword only
Hi,
Stefan Beller wrote:
> When bf1a11f0a1 (sideband: highlight keywords in remote sideband output,
> 2018-08-07) was introduced, it was carefully considered which strings
> would be highlighted. However 59a255aef0 (sideband: do not read beyond
> the end of input, 2018-08-18) brought in a regression that the original
> did not test for. A line containing only the keyword and nothing else
> ("SUCCESS") should still be colored.
>
> Signed-off-by: Stefan Beller
> ---
> sideband.c | 5 +++--
> t/t5409-colorize-remote-messages.sh | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
Thanks for writing this.
I was curious about what versions of Gerrit this is designed to
support (or in other words whether it's a bug fix or a feature).
Looking at examples like [1], it seems that Gerrit historically always
used "ERROR:" so the 59a255aef0 logic would work for it. More
recently, [2] (ReceiveCommits: add a "SUCCESS" marker for successful
change updates, 2018-08-21) put SUCCESS on a line of its own. That
puts this squarely in the new-feature category.
"success" on its own line is even less likely to be a false positive
than "success" followed by punctuation (for example a period marking
the end of a sentence). So I like this change.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/22361
[2] https://gerrit-review.googlesource.com/c/gerrit/+/193570
> diff --git a/sideband.c b/sideband.c
> index 368647acf8..7c3d33d3f8 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -87,7 +87,7 @@ static void maybe_colorize_sideband(struct strbuf *dest,
> const char *src, int n)
> struct keyword_entry *p = keywords + i;
> int len = strlen(p->keyword);
>
> - if (n <= len)
> + if (n < len)
> continue;
In the old code, we would escape early if 'n == len', but we didn't
need to. If 'n == len', then
src[len] == '\0'
src .. [len-1] is a valid buffer to read from
so the strncasecmp and strbuf_add operations used in this function are
valid. Good.
> /*
>* Match case insensitively, so we colorize output from existing
> @@ -95,7 +95,8 @@ static void maybe_colorize_sideband(struct strbuf *dest,
> const char *src, int n)
>* messages. We only highlight the word precisely, so
>* "successful" stays uncolored.
>*/
> - if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) {
> + if (!strncasecmp(p->keyword, src, len) &&
> + (len == n || !isalnum(src[len]))) {
Our custom isalnum treats '\0' as not alphanumeric (sane_ctype[0] ==
GIT_CNTRL) so this part of the patch is unnecessary. That said, it's
good for clarity and defensive programming.
> strbuf_addstr(dest, p->color);
> strbuf_add(dest, src, len);
> strbuf_addstr(dest, GIT_COLOR_RESET);
> diff --git a/t/t5409-colorize-remote-messages.sh
> b/t/t5409-colorize-remote-messages.sh
> index f81b6813c0..2a8c449661 100755
> --- a/t/t5409-colorize-remote-messages.sh
> +++ b/t/t5409-colorize-remote-messages.sh
> @@ -17,6 +17,7 @@ test_expect_success 'setup' '
> echo " " "error: leading space"
> echo ""
> echo Err
> + echo SUCCESS
> exit 0
> EOF
> echo 1 >file &&
> @@ -35,6 +36,7 @@ test_expect_success 'keywords' '
> grep "error: error" decoded &&
> grep "hint:" decoded &&
> grep "success:" decoded &&
> + grep "SUCCESS" decoded &&
> grep "warning:" decoded
> '
Nice tests.
The "hinting: not highlighted" example shows that we aren't
introducing false positives here, so the coverage seems sufficient.
It might be nice to include a line
echo ERROR:
as well to match another idiom that Gerrit sometimes uses.
Reviewed-by: Jonathan Nieder
Thanks again for a pleasant read.
Re: easy way to demonstrate length of colliding SHA-1 prefixes?
On Mon, Dec 03, 2018 at 02:30:44PM -0800, Matthew DeVore wrote:
> Here is a one-liner to do it. It is Perl line noise, so it's not very cute,
> thought that is subjective. The output shown below is for the Git project
> (not Linux) repository as I've currently synced it:
>
> $ git rev-list --objects HEAD | sort | perl -anE 'BEGIN { $prev = ""; $long
> = "" } $n = $F[0]; for my $i (reverse 1..40) {last if $i < length($long); if
> (substr($prev, 0, $i) eq substr($n, 0, $i)) {$long = substr($prev, 0, $i);
> last} } $prev = $n; END {say $long}'
Ooh, object-collision golf.
Try:
git cat-file --batch-all-objects --batch-check='%(objectname)'
instead of "rev-list | sort". It's _much_ faster, because it doesn't
have to actually open the objects and walk the graph.
Some versions of uniq have "-w" (including GNU, but it's definitely not
in POSIX), which lets you do:
git cat-file --batch-all-objects --batch-check='%(objectname)' |
uniq -cdw 7
to list all collisions of length 7 (it will show just the first item
from each group, but you can use -D to see them all).
> > You'll always need to list them all. It's inherently an operation where
> > for each SHA-1 you need to search for other ones with that prefix up to
> > a given length.
> >
> > Perhaps you've missed that you can use --abbrev=N for this, and just
> > grep for things that are loger than that N, e.g. for linux.git:
> >
> > git log --oneline --abbrev=10 --pretty=format:%h |
> > grep -E -v '^.{10}$' |
> > perl -pe 's/^(.{10}).*/$1/'
>
> I think the goal was to search all object hashes, not just commits. And git
> rev-list --objects will do that.
You can add "-t --raw" to see the abbreviated tree and blob names,
though it gets tricky around handling merges.
-Peff
[PATCH] sideband: color lines with keyword only
When bf1a11f0a1 (sideband: highlight keywords in remote sideband output,
2018-08-07) was introduced, it was carefully considered which strings
would be highlighted. However 59a255aef0 (sideband: do not read beyond
the end of input, 2018-08-18) brought in a regression that the original
did not test for. A line containing only the keyword and nothing else
("SUCCESS") should still be colored.
Signed-off-by: Stefan Beller
---
sideband.c | 5 +++--
t/t5409-colorize-remote-messages.sh | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/sideband.c b/sideband.c
index 368647acf8..7c3d33d3f8 100644
--- a/sideband.c
+++ b/sideband.c
@@ -87,7 +87,7 @@ static void maybe_colorize_sideband(struct strbuf *dest,
const char *src, int n)
struct keyword_entry *p = keywords + i;
int len = strlen(p->keyword);
- if (n <= len)
+ if (n < len)
continue;
/*
* Match case insensitively, so we colorize output from existing
@@ -95,7 +95,8 @@ static void maybe_colorize_sideband(struct strbuf *dest,
const char *src, int n)
* messages. We only highlight the word precisely, so
* "successful" stays uncolored.
*/
- if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) {
+ if (!strncasecmp(p->keyword, src, len) &&
+ (len == n || !isalnum(src[len]))) {
strbuf_addstr(dest, p->color);
strbuf_add(dest, src, len);
strbuf_addstr(dest, GIT_COLOR_RESET);
diff --git a/t/t5409-colorize-remote-messages.sh
b/t/t5409-colorize-remote-messages.sh
index f81b6813c0..2a8c449661 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -17,6 +17,7 @@ test_expect_success 'setup' '
echo " " "error: leading space"
echo ""
echo Err
+ echo SUCCESS
exit 0
EOF
echo 1 >file &&
@@ -35,6 +36,7 @@ test_expect_success 'keywords' '
grep "error: error" decoded &&
grep "hint:" decoded &&
grep "success:" decoded &&
+ grep "SUCCESS" decoded &&
grep "warning:" decoded
'
--
2.20.0.rc2.403.gdbc3b29805-goog
Re: easy way to demonstrate length of colliding SHA-1 prefixes?
On 12/02/2018 05:23 AM, Ævar Arnfjörð Bjarmason wrote:
On Sun, Dec 02 2018, Robert P. J. Day wrote:
as part of an upcoming git class i'm delivering, i thought it would
be amusing to demonstrate the maximum length of colliding SHA-1
prefixes in a repository (in my case, i use the linux kernel git repo
for most of my examples).
is there a way to display the objects in the object database that
clash in the longest object name SHA-1 prefix; i mean, short of
manually listing all object names, running that through cut and sort
and uniq and ... you get the idea.
is there a cute way to do that? thanks.
Here is a one-liner to do it. It is Perl line noise, so it's not very
cute, thought that is subjective. The output shown below is for the Git
project (not Linux) repository as I've currently synced it:
$ git rev-list --objects HEAD | sort | perl -anE 'BEGIN { $prev = "";
$long = "" } $n = $F[0]; for my $i (reverse 1..40) {last if $i <
length($long); if (substr($prev, 0, $i) eq substr($n, 0, $i)) {$long =
substr($prev, 0, $i); last} } $prev = $n; END {say $long}'
c68038ef
$ git cat-file -t c68038ef
error: short SHA1 c68038ef is ambiguous
hint: The candidates are:
hint: c68038effe commit 2012-06-01 - vcs-svn: suppress a
signed/unsigned comparison warning
hint: c68038ef00 blob
fatal: Not a valid object name c68038ef
You'll always need to list them all. It's inherently an operation where
for each SHA-1 you need to search for other ones with that prefix up to
a given length.
Perhaps you've missed that you can use --abbrev=N for this, and just
grep for things that are loger than that N, e.g. for linux.git:
git log --oneline --abbrev=10 --pretty=format:%h |
grep -E -v '^.{10}$' |
perl -pe 's/^(.{10}).*/$1/'
I think the goal was to search all object hashes, not just commits. And
git rev-list --objects will do that.
Re: [PATCH v2] revisions.c: put promisor option in specialized struct
On Mon, Dec 03, 2018 at 02:10:19PM -0800, Matthew DeVore wrote: > Put the allow_exclude_promisor_objects flag in setup_revision_opt. When > it was in rev_info, it was unclear when it was used, since rev_info is > passed to functions that don't use the flag. This resulted in > unnecessary setting of the flag in prune.c, so fix that as well. > > Signed-off-by: Matthew DeVore > --- > builtin/pack-objects.c | 6 -- > builtin/prune.c| 1 - > builtin/rev-list.c | 6 -- > revision.c | 10 ++ > revision.h | 4 ++-- > 5 files changed, 16 insertions(+), 11 deletions(-) Thanks, this version looks good to me. One style nit that I don't think is worth a re-roll, but that Junio might want to tweak while applying: > diff --git a/revision.c b/revision.c > index 13e0519c02..f6b32e6a42 100644 > --- a/revision.c > +++ b/revision.c > @@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, > const char *pattern) > } > > static int handle_revision_opt(struct rev_info *revs, int argc, const char > **argv, > -int *unkc, const char **unkv) > +int *unkc, const char **unkv, > +const struct setup_revision_opt* opt) We keep the "*" with the variable name, not the type. -Peff
Re: Confusing inconsistent option syntax
On Sun, Dec 02, 2018 at 09:07:47PM +1100, Robert White wrote: > `git log --pretty short` gives the error message "ambiguous argument > 'short'". To get the expected result, you need to use `git log > --pretty=short`. However, `git log --since yesterday` and `git log > --since=yesterday` both work as expected. > > When is an = needed? What is the reason for these inconsistencies? As Duy mentioned, this has to do with optional arguments for some flags. This is discussed in more detail in "git help cli". Notably (and as advised in that manpage), you should always use the "stuck" form (with the "=") in scripts, in case a flag grows an optional value later. -Peff
[PATCH v2] revisions.c: put promisor option in specialized struct
Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
it was in rev_info, it was unclear when it was used, since rev_info is
passed to functions that don't use the flag. This resulted in
unnecessary setting of the flag in prune.c, so fix that as well.
Signed-off-by: Matthew DeVore
---
builtin/pack-objects.c | 6 --
builtin/prune.c| 1 -
builtin/rev-list.c | 6 --
revision.c | 10 ++
revision.h | 4 ++--
5 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24bba8147f..889df2c755 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3084,14 +3084,16 @@ static void record_recent_commit(struct commit *commit,
void *data)
static void get_object_list(int ac, const char **av)
{
struct rev_info revs;
+ struct setup_revision_opt s_r_opt = {
+ .allow_exclude_promisor_objects = 1,
+ };
char line[1000];
int flags = 0;
int save_warning;
repo_init_revisions(the_repository, , NULL);
save_commit_buffer = 0;
- revs.allow_exclude_promisor_objects_opt = 1;
- setup_revisions(ac, av, , NULL);
+ setup_revisions(ac, av, , _r_opt);
/* make sure shallows are read */
is_repository_shallow(the_repository);
diff --git a/builtin/prune.c b/builtin/prune.c
index e42653b99c..1ec9ddd751 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -120,7 +120,6 @@ int cmd_prune(int argc, const char **argv, const char
*prefix)
save_commit_buffer = 0;
read_replace_refs = 0;
ref_paranoia = 1;
- revs.allow_exclude_promisor_objects_opt = 1;
repo_init_revisions(the_repository, , prefix);
argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3a2c0c23b6..14ef659c12 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -362,6 +362,9 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
{
struct rev_info revs;
struct rev_list_info info;
+ struct setup_revision_opt s_r_opt = {
+ .allow_exclude_promisor_objects = 1,
+ };
int i;
int bisect_list = 0;
int bisect_show_vars = 0;
@@ -375,7 +378,6 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
git_config(git_default_config, NULL);
repo_init_revisions(the_repository, , prefix);
revs.abbrev = DEFAULT_ABBREV;
- revs.allow_exclude_promisor_objects_opt = 1;
revs.commit_format = CMIT_FMT_UNSPECIFIED;
revs.do_not_die_on_missing_tree = 1;
@@ -407,7 +409,7 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
}
}
- argc = setup_revisions(argc, argv, , NULL);
+ argc = setup_revisions(argc, argv, , _r_opt);
memset(, 0, sizeof(info));
info.revs =
diff --git a/revision.c b/revision.c
index 13e0519c02..f6b32e6a42 100644
--- a/revision.c
+++ b/revision.c
@@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const
char *pattern)
}
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
- int *unkc, const char **unkv)
+ int *unkc, const char **unkv,
+ const struct setup_revision_opt* opt)
{
const char *arg = argv[0];
const char *optarg;
@@ -2151,7 +2152,7 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
revs->limited = 1;
} else if (!strcmp(arg, "--ignore-missing")) {
revs->ignore_missing = 1;
- } else if (revs->allow_exclude_promisor_objects_opt &&
+ } else if (opt && opt->allow_exclude_promisor_objects &&
!strcmp(arg, "--exclude-promisor-objects")) {
if (fetch_if_missing)
BUG("exclude_promisor_objects can only be used when
fetch_if_missing is 0");
@@ -2173,7 +2174,7 @@ void parse_revision_opt(struct rev_info *revs, struct
parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
- >cpidx, ctx->out);
+ >cpidx, ctx->out, NULL);
if (n <= 0) {
error("unknown option `%s'", ctx->argv[0]);
usage_with_options(usagestr, options);
@@ -2391,7 +2392,8 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
continue;
}
- opts = handle_revision_opt(revs, argc - i, argv + i,
, argv);
+ opts = handle_revision_opt(revs, argc - i, argv + i,
+ , argv, opt);
Re: [PATCH] revisions.c: put promisor option in specialized struct
On 12/03/2018 01:24 PM, Jeff King wrote:
@@ -297,7 +296,8 @@ struct setup_revision_opt {
const char *def;
void (*tweak)(struct rev_info *, struct setup_revision_opt *);
const char *submodule; /* TODO: drop this and use rev_info->repo */
- int assume_dashdash;
+ int assume_dashdash : 1;
+ int allow_exclude_promisor_objects : 1;
unsigned revarg_opt;
};
I don't know that we need to penny-pinch bytes in this struct, but in
general it shouldn't hurt either awy. However, a signed bit-field with 1
bit is funny. I'm not even sure what the standard has to say, but in
twos-complement that would store "-1" and "0" (gcc -Wpedantic also
complains about overflow in assigning "1" to it).
Interesting. I hadn't suspected this. But I confirmed it with this:
#include
struct x {
int y : 1;
int z : 1;
};
int main() {
struct x x;
x.y = 1;
x.z = 1;
printf("%d %d\n", (int) x.y, (int) x.z);
return 0;
}
-- Output --
-1 -1
So this probably ought to be "unsigned".
Earlier in this file we define bit fields this way:
/* Traversal flags */
unsigned intdense:1,
prune:1,
... using \t to align the field names, so I'll mimic that style.
Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check
On Sun, Dec 02, 2018 at 11:52:50AM +0100, René Scharfe wrote:
> > And for mu.git, a ~20k object repo:
> >
> > Test origin/master
> > peff/jk/loose-cache avar/check-collisions-config
> >
> > -
> > 0008.2: index-pack with 256*1 loose objects 0.59(0.91+0.06)
> > 0.58(0.93+0.03) -1.7% 0.57(0.89+0.04) -3.4%
> > 0008.3: index-pack with 256*10 loose objects 0.59(0.91+0.07)
> > 0.59(0.92+0.03) +0.0% 0.57(0.89+0.03) -3.4%
> > 0008.4: index-pack with 256*100 loose objects0.59(0.91+0.05)
> > 0.81(1.13+0.04) +37.3%0.58(0.91+0.04) -1.7%
> > 0008.5: index-pack with 256*250 loose objects0.59(0.91+0.05)
> > 1.23(1.51+0.08) +108.5% 0.58(0.91+0.04) -1.7%
> > 0008.6: index-pack with 256*500 loose objects0.59(0.90+0.06)
> > 1.96(2.20+0.12) +232.2% 0.58(0.91+0.04) -1.7%
> > 0008.7: index-pack with 256*750 loose objects0.59(0.92+0.05)
> > 2.72(2.92+0.17) +361.0% 0.58(0.90+0.04) -1.7%
> > 0008.8: index-pack with 256*1000 loose objects 0.59(0.90+0.06)
> > 3.50(3.67+0.21) +493.2% 0.57(0.90+0.04) -3.4%
>
> OK, here's another theory: The cache scales badly with increasing
> numbers of loose objects because it sorts the array 256 times as it is
> filled. Loading it fully and sorting once would help, as would using
> one array per subdirectory.
Yeah, that makes sense. This was actually how I had planned to do it
originally, but then I ended up just reusing the existing single-array
approach from the abbrev code.
I hadn't actually thought about the repeated sortings (but that
definitely makes sense that they would hurt in these pathological
cases), but more just that we get a 256x reduction in N for our binary
search (in fact we already do this first-byte lookup-table trick for
pack index lookups).
Your patch looks good to me. We may want to do one thing on top:
> diff --git a/object-store.h b/object-store.h
> index 8dceed0f31..ee67a50980 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -20,7 +20,7 @@ struct object_directory {
>* Be sure to call odb_load_loose_cache() before using.
>*/
> char loose_objects_subdir_seen[256];
> - struct oid_array loose_objects_cache;
> + struct oid_array loose_objects_cache[256];
The comment in the context there is warning callers to remember to load
the cache first. Now that we have individual caches, might it make sense
to change the interface a bit, and make these members private. I.e.,
something like:
struct oid_array *odb_loose_cache(struct object_directory *odb,
int subdir_nr)
{
if (!loose_objects_subdir_seen[subdir_nr])
odb_load_loose_cache(odb, subdir_nr); /* or just inline it here
*/
return >loose_objects_cache[subdir_nr];
}
That's harder to get wrong, and this:
> diff --git a/sha1-file.c b/sha1-file.c
> index 05f63dfd4e..d2f5e65865 100644
> --- a/sha1-file.c
> +++ b/sha1-file.c
> @@ -933,7 +933,8 @@ static int quick_has_loose(struct repository *r,
> prepare_alt_odb(r);
> for (odb = r->objects->odb; odb; odb = odb->next) {
> odb_load_loose_cache(odb, subdir_nr);
> - if (oid_array_lookup(>loose_objects_cache, ) >= 0)
> + if (oid_array_lookup(>loose_objects_cache[subdir_nr],
> + ) >= 0)
> return 1;
> }
becomes:
struct oid_array *cache = odb_loose_cache(odb, subdir_nr);
if (oid_array_lookup(cache, ))
return 1;
(An even simpler interface would be a single function that computes
subdir_nr and does the lookup itself, but that would not be enough for
find_short_object_filename()).
-Peff
Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
On 12/03/2018 01:15 PM, Jeff King wrote:
That said, our C99 designated initializer weather-balloons haven't
gotten any complaints yet. So I think you could actually do:
struct setup_revision_opt s_r_opt = {
.allow_exclude_promisor_objects = 1,
};
I like this way best, so I'll use it. Thank you.
Re: [PATCH/RFC v2 0/7] Introduce new commands switch-branch and checkout-files
On Thu, Nov 29, 2018 at 7:33 AM Duy Nguyen wrote:
>
> On Wed, Nov 28, 2018 at 9:30 PM Stefan Beller wrote:
> >
> > On Wed, Nov 28, 2018 at 12:09 PM Duy Nguyen wrote:
> > >
> > > On Wed, Nov 28, 2018 at 9:01 PM Duy Nguyen wrote:
> > > > should we do
> > > > something about detached HEAD in this switch-branch command (or
> > > > whatever its name will be)?
> > > >
> > > > This is usually a confusing concept to new users
> > >
> > > And it just occurred to me that perhaps we should call this "unnamed
> > > branch" (at least at high UI level) instead of detached HEAD. It is
> > > technically not as accurate, but much better to understand.
> >
> > or 'direct' branch?
>
> makes me think, what is an indirect branch?
I drew the term from HEAD pointing to a branch pointing
to a commit, i.e. HEAD indirectly points to a commit, but
in 'direct' branch mode, HEAD contains the commit id.
So indirect branch would work for our current 'real' branches.
When asked out of context of this discussion, I might add
yet another layer of abstraction to make an 'indirect branch',
i.e. HEAD pointing to a symbolic ref that points at a branch
that points to a commit.
The term symref is what we currently use
(Just looked into gitglossary, where we distinguish
symbolic refs from pseudorefs) for hat I would have called
an indirect branch as well.
So maybe we need to measure the level of indirection
("How often do we need to dereference the ref/object to get
a commit oid?") to come to terms in how to describe
the use cases easily.
Here is a fun-one:
git checkout
git checkout --detach
Currently the --detach option detaches HEAD from
branch pointing at the object id, i.e. it is the same as
git checkout
whereas when we focus on the levels of indirection
it would also be reasonable to have
git checkout
as a reasonable alternative, where is the
branch that is pointed at from the .
Re: [PATCH] rebase -i: introduce the 'test' command
On Mon, Dec 03, 2018 at 06:53:22PM +0100, Duy Nguyen 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. > > And here I've been doing the same by "edit" the first commit, add a > new commit then reorder them in the second interactive rebase :P > > This made me look at git-rebase.txt to really learn about interactive > rebase. I think the interactive rebase section could use some > improvements. Its style looks.. umm.. more story telling than a > reference. Perhaps something like this to at least highlight the > commands. Yeah, I think the typographic change is an improvement that doesn't really have a downside. As Dscho mentioned, sometimes the story style is what you want, so I don't think we'd want to simply rearrange it. It may be useful to present the material twice, though: once as a table/list for reference, and then once as a story working through an example. -Peff
Re: [PATCH] rebase -i: introduce the 'test' command
On Mon, Dec 03, 2018 at 08:01:44PM +0100, Johannes Schindelin wrote: > > 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. > > 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 > > 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 > > (assuming that the latest 3 commits were non-merge commits; It would look > quite a bit more complicated in other situations.) Yeah, I would probably use that if it existed. It would be nicer to have real nested sequencer operations, I think, for other situations. E.g., cherry-picking a sequence of commits while you're in the middle of a rebase. But I suspect getting that right would be _loads_ more work, and probably would involve some funky UI corner cases to handle the stack of operations (so truly aborting a rebase may mean an arbitrary number of "rebase --abort" calls to pop the stack). Your suggestion is probably a reasonable trick in the meantime. -Peff
Re: [PATCH] rebase -i: introduce the 'test' command
On Mon, Dec 03, 2018 at 02:31:37PM +, Phillip Wood wrote: > > How would I move past the test that fails to continue? I guess "git > > rebase --edit-todo" and then manually remove it (and any other remaining > > test lines)? > > Perhaps we could teach git rebase --skip to skip a rescheduled command, it > could be useful if people want to skip rescheduled picks as well (though I > don't think I've ever had that happen in the wild). I can see myself turning > on the rescheduling config setting but occasionally wanting to be able to > skip over the rescheduled exec command. Yeah, I agree that would give a nice user experience. -Peff
Re: [PATCH] revisions.c: put promisor option in specialized struct
On Mon, Dec 03, 2018 at 11:23:56AM -0800, Matthew DeVore wrote:
> Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
> it was in rev_info, it was unclear when it was used, since rev_info is
> passed to functions that don't use the flag. This resulted in
> unnecessary setting of the flag in prune.c, so fix that as well.
>
> Signed-off-by: Matthew DeVore
> ---
> builtin/pack-objects.c | 7 +--
> builtin/prune.c| 1 -
> builtin/rev-list.c | 6 --
> revision.c | 10 ++
> revision.h | 4 ++--
> 5 files changed, 17 insertions(+), 11 deletions(-)
Thanks, this mostly looks good to me (with or without tweaking the
initializers as discussed in the other part of the thread).
One thing I noticed:
> @@ -297,7 +296,8 @@ struct setup_revision_opt {
> const char *def;
> void (*tweak)(struct rev_info *, struct setup_revision_opt *);
> const char *submodule; /* TODO: drop this and use rev_info->repo */
> - int assume_dashdash;
> + int assume_dashdash : 1;
> + int allow_exclude_promisor_objects : 1;
> unsigned revarg_opt;
> };
I don't know that we need to penny-pinch bytes in this struct, but in
general it shouldn't hurt either awy. However, a signed bit-field with 1
bit is funny. I'm not even sure what the standard has to say, but in
twos-complement that would store "-1" and "0" (gcc -Wpedantic also
complains about overflow in assigning "1" to it).
So this probably ought to be "unsigned".
-Peff
[PATCH v3] range-diff: always pass at least minimal diff options
From: Martin Ågren
Commit d8981c3f88 ("format-patch: do not let its diff-options affect
--range-diff", 2018-11-30) taught `show_range_diff()` to accept a
NULL-pointer as an indication that it should use its own "reasonable
default". That fixed a regression from a5170794 ("Merge branch
'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced
a regression of its own.
In particular, it means we forget the `file` member of the diff options,
so rather than placing a range-diff in the cover-letter, we write it to
stdout. In order to fix this, rewrite the two callers adjusted by
d8981c3f88 to instead create a "dummy" set of diff options where they
only fill in the fields we absolutely require, such as output file and
color.
Modify and extend the existing tests to try and verify that the right
contents end up in the right place.
Don't revert `show_range_diff()`, i.e., let it keep accepting NULL.
Rather than removing what is dead code and figuring out it isn't
actually dead and we've broken 2.20, just leave it for now.
[es: retain diff coloring when going to stdout]
Signed-off-by: Martin Ågren
Signed-off-by: Eric Sunshine
---
This is a re-roll of Martin's v2[1]. The only difference from v2 is that
it retains coloring when emitting to the terminal (plus an in-code
comment was simplified).
The regression introduced by d8981c3f88, in which the range-diff only
ever gets emitted to the terminal, and never to the cover letter or
commentary section of a standalone patch, makes the --range-diff option
rather useless, so this fix probably ought to be fast-tracked. An
alternative would be to rip out all the recent "--range-diff"-related
changes and go with the --range-diff implementation which has been in
use for a few months, even if it is not perfect.
[1]:
https://public-inbox.org/git/20181203200734.527341-1-martin.ag...@gmail.com/
builtin/log.c | 11 ++-
log-tree.c| 11 ++-
t/t3206-range-diff.sh | 20 +---
3 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 5ac18e2848..e8e51068bd 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1094,9 +1094,18 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
}
if (rev->rdiff1) {
+ /*
+* Pass minimum required diff-options to range-diff; others
+* can be added later if deemed desirable.
+*/
+ struct diff_options opts;
+ diff_setup();
+ opts.file = rev->diffopt.file;
+ opts.use_color = rev->diffopt.use_color;
+ diff_setup_done();
fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
show_range_diff(rev->rdiff1, rev->rdiff2,
- rev->creation_factor, 1, NULL);
+ rev->creation_factor, 1, );
}
}
diff --git a/log-tree.c b/log-tree.c
index b243779a0b..10680c139e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -755,14 +755,23 @@ void show_log(struct rev_info *opt)
if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
struct diff_queue_struct dq;
+ struct diff_options opts;
memcpy(, _queued_diff, sizeof(diff_queued_diff));
DIFF_QUEUE_CLEAR(_queued_diff);
next_commentary_block(opt, NULL);
fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+ /*
+* Pass minimum required diff-options to range-diff; others
+* can be added later if deemed desirable.
+*/
+ diff_setup();
+ opts.file = opt->diffopt.file;
+ opts.use_color = opt->diffopt.use_color;
+ diff_setup_done();
show_range_diff(opt->rdiff1, opt->rdiff2,
- opt->creation_factor, 1, NULL);
+ opt->creation_factor, 1, );
memcpy(_queued_diff, , sizeof(diff_queued_diff));
}
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e497c1358f..048feaf6dd 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -248,18 +248,24 @@ test_expect_success 'dual-coloring' '
for prev in topic master..topic
do
test_expect_success "format-patch --range-diff=$prev" '
- git format-patch --stdout --cover-letter --range-diff=$prev \
+ git format-patch --cover-letter --range-diff=$prev \
master..unmodified >actual &&
- grep "= 1: .* s/5/A" actual &&
- grep "= 2: .* s/4/A" actual &&
- grep "= 3: .* s/11/B" actual &&
- grep "= 4: .* s/12/B" actual
+ test_when_finished "rm 000?-*" &&
+ test_line_count = 5 actual &&
+ test_i18ngrep "^Range-diff:$" -* &&
+ grep "= 1: .*
Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
On Mon, Dec 03, 2018 at 11:10:49AM -0800, Matthew DeVore wrote:
> > > + memset(_r_opt, 0, sizeof(s_r_opt));
> > > + s_r_opt.allow_exclude_promisor_objects = 1;
> > > + setup_revisions(ac, av, , _r_opt);
> >
> > I wonder if a static initializer for setup_revision_opt is worth it. It
> > would remove the need for this memset. Probably not a big deal either
> > way, though.
> I think you mean something like this:
>
> static struct setup_revision_opt s_r_opt = {NULL, NULL, NULL, 0, 1, 0};
>
> This is a bit cryptic (I have to read the struct declaration in order to
> know what is being set to 1) and if the struct ever gets a new field before
> allow_exclude_promisor_objects, this initializer has to be updated.
I agree that's pretty awful. I meant something like this:
struct setup_revision_opt s_r_opt = { NULL };
...
s_r_opt.allow_exclude_promisor_objects = 1;
setup_revisions(...);
It's functionally equivalent to the memset(), but you don't have to
wonder about whether we peek at the uninitialized state in between.
That said, our C99 designated initializer weather-balloons haven't
gotten any complaints yet. So I think you could actually do:
struct setup_revision_opt s_r_opt = {
.allow_exclude_promisor_objects = 1,
};
...
setup_revisions(...);
which is pretty nice.
-Peff
Re: [PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
Hi Hannes, On Mon, 3 Dec 2018, Johannes Sixt wrote: > The text body of section Behavioral Differences is typeset as code, > but should be regular text. Remove the indentation to achieve that. > > While here, prettify the language: > > - use "the x backend" instead of "x-based rebase"; > - use present tense instead of future tense; > > and use subsections instead of a list. > > Signed-off-by: Johannes Sixt > --- The changes look sensible to me. Thanks, Dscho > Cf. https://git-scm.com/docs/git-rebase#_behavioral_differences > > I actually did not test the result, because I don't have the > infrastructure. > > The sentence "The am backend sometimes does not" is not very useful, > but is not my fault ;) It would be great if it could be made more > specific, but I do not know the details. > > Documentation/git-rebase.txt | 30 +- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 80793bad8d..dff17b3178 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -550,24 +550,28 @@ Other incompatible flag pairs: > BEHAVIORAL DIFFERENCES > --- > > - * empty commits: > +There are some subtle differences how the backends behave. > > -am-based rebase will drop any "empty" commits, whether the > -commit started empty (had no changes relative to its parent to > -start with) or ended empty (all changes were already applied > -upstream in other commits). > +Empty commits > +~ > + > +The am backend drops any "empty" commits, regardless of whether the > +commit started empty (had no changes relative to its parent to > +start with) or ended empty (all changes were already applied > +upstream in other commits). > > -merge-based rebase does the same. > +The merge backend does the same. > > -interactive-based rebase will by default drop commits that > -started empty and halt if it hits a commit that ended up empty. > -The `--keep-empty` option exists for interactive rebases to allow > -it to keep commits that started empty. > +The interactive backend drops commits by default that > +started empty and halts if it hits a commit that ended up empty. > +The `--keep-empty` option exists for the interactive backend to allow > +it to keep commits that started empty. > > - * directory rename detection: > +Directory rename detection > +~~ > > -merge-based and interactive-based rebases work fine with > -directory rename detection. am-based rebases sometimes do not. > +The merge and interactive backends work fine with > +directory rename detection. The am backend sometimes does not. > > include::merge-strategies.txt[] > > -- > 2.19.1.1133.g2dd3d172d2 >
Re: [PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
Am 03.12.18 um 21:42 schrieb Martin Ågren: On Mon, 3 Dec 2018 at 18:35, Johannes Sixt wrote: I actually did not test the result, because I don't have the infrastructure. I've tested with asciidoc and Asciidoctor, html and man-page. Looks good. Thank you so much! -- Hannes
Re: [ANNOUNCE] Git v2.20.0-rc2
Team, Git for Windows v2.20.0-rc2 is available here: https://github.com/git-for-windows/git/releases/tag/v2.20.0-rc2.windows.1 There is already one known issue: the size of the installer increased (see https://github.com/git-for-windows/git/issues/1963). This is in the process of being addressed. Ciao, Johannes On Sat, 1 Dec 2018, Junio C Hamano wrote: > A release candidate Git v2.20.0-rc2 is now available for testing > at the usual places. It is comprised of 934 non-merge commits > since v2.19.0, contributed by 76 people, 25 of which are new faces. > > The tarballs are found at: > > https://www.kernel.org/pub/software/scm/git/testing/ > > The following public repositories all have a copy of the > 'v2.20.0-rc2' tag and the 'master' branch that the tag points at: > > url = https://kernel.googlesource.com/pub/scm/git/git > url = git://repo.or.cz/alt-git.git > url = https://github.com/gitster/git > > New contributors whose contributions weren't in v2.19.0 are as follows. > Welcome to the Git development community! > > Aaron Lindsay, Alexander Pyhalov, Anton Serbulov, Brendan > Forster, Carlo Marcelo Arenas Belón, Daniels Umanovskis, David > Zych, Đoàn Trần Công Danh, Frederick Eaton, Greg Hurrell, > James Knight, Jann Horn, Joshua Watt, Loo Rong Jie, Lucas > De Marchi, Matthew DeVore, Mihir Mehta, Nickolai Belakovski, > Roger Strain, Sam McKelvie, Saulius Gurklys, Shulhan, Steven > Fernandez, Strain, Roger L, and Tim Schumacher. > > Returning contributors who helped this release are as follows. > Thanks for your continued support. > > Ævar Arnfjörð Bjarmason, Alban Gruin, Andreas Gruenbacher, > Andreas Heiduk, Antonio Ospite, Ben Peart, Brandon Williams, > brian m. carlson, Christian Couder, Christian Hesse, Denton Liu, > Derrick Stolee, Elijah Newren, Eric Sunshine, Jean-Noël Avila, > Jeff Hostetler, Jeff King, Johannes Schindelin, Johannes Sixt, > Jonathan Nieder, Jonathan Tan, Josh Steadmon, Junio C Hamano, > Karsten Blees, Luke Diamand, Martin Ågren, Max Kirillov, > Michael Witten, Michał Górny, Nguyễn Thái Ngọc Duy, Noam > Postavsky, Olga Telezhnaya, Phillip Wood, Pratik Karki, Rafael > Ascensão, Ralf Thielow, Ramsay Jones, Rasmus Villemoes, René > Scharfe, Sebastian Staudt, Stefan Beller, Stephen P. Smith, Steve > Hoelzer, Sven Strickroth, SZEDER Gábor, Tao Qingyun, Taylor > Blau, Thomas Gummerer, Todd Zullinger, Torsten Bögershausen, > and Uwe Kleine-König. > > > > Git 2.20 Release Notes (draft) > == > > Backward Compatibility Notes > > > * "git branch -l " used to be a way to ask a reflog to be >created while creating a new branch, but that is no longer the >case. It is a short-hand for "git branch --list " now. > > * "git push" into refs/tags/* hierarchy is rejected without getting >forced, but "git fetch" (misguidedly) used the "fast forwarding" >rule used for the refs/heads/* hierarchy; this has been corrected, >which means some fetches of tags that did not fail with older >version of Git will fail without "--force" with this version. > > * "git help -a" now gives verbose output (same as "git help -av"). >Those who want the old output may say "git help --no-verbose -a".. > > * "git cpn --help", when "cpn" is an alias to, say, "cherry-pick -n", >reported only the alias expansion of "cpn" in earlier versions of >Git. It now runs "git cherry-pick --help" to show the manual page >of the command, while sending the alias expansion to the standard >error stream. > > * "git send-email" learned to grab address-looking string on any >trailer whose name ends with "-by". This is a backward-incompatible >change. Adding "--suppress-cc=misc-by" on the command line, or >setting sendemail.suppresscc configuration variable to "misc-by", >can be used to disable this behaviour. > > > Updates since v2.19 > --- > > UI, Workflows & Features > > * Running "git clone" against a project that contain two files with >pathnames that differ only in cases on a case insensitive >filesystem would result in one of the files lost because the >underlying filesystem is incapable of holding both at the same >time. An attempt is made to detect such a case and warn. > > * "git checkout -b newbranch [HEAD]" should not have to do as much as >checking out a commit different from HEAD. An attempt is made to >optimize this special case. > > * "git rev-list --stdin no output without an error. "git rev-list --stdin --default HEAD" >still falls back to the given default when nothing is given on the >standard input. > > * Lift code from GitHub to restrict delta computation so that an >object that exists in one fork is not made into a delta against >another object that does not appear in the same forked
Re: [PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
On Mon, 3 Dec 2018 at 18:35, Johannes Sixt wrote: > I actually did not test the result, because I don't have the > infrastructure. I've tested with asciidoc and Asciidoctor, html and man-page. Looks good. Martin
[PATCH 1/3] RelNotes 2.20: move some items between sections
Some items that should be in "Performance, Internal Implementation, Development Support etc." have ended up in "UI, Workflows & Features" and "Fixes since v2.19". Move them, and do s/uses/use/ while at it. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index b1deaf37da..e5ab8cc609 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -137,11 +137,6 @@ UI, Workflows & Features command line, or setting sendemail.suppresscc configuration variable to "misc-by", can be used to disable this behaviour. - * Developer builds now uses -Wunused-function compilation option. - - * One of our CI tests to run with "unusual/experimental/random" - settings now also uses commit-graph and midx. - * "git mergetool" learned to take the "--[no-]gui" option, just like "git difftool" does. @@ -185,6 +180,11 @@ UI, Workflows & Features Performance, Internal Implementation, Development Support etc. + * Developer builds now use -Wunused-function compilation option. + + * One of our CI tests to run with "unusual/experimental/random" + settings now also uses commit-graph and midx. + * When there are too many packfiles in a repository (which is not recommended), looking up an object in these would require consulting many pack .idx files; a new mechanism to have a single @@ -387,6 +387,14 @@ Performance, Internal Implementation, Development Support etc. two classes to ease code migration process has been proposed and its support has been added to the Makefile. + * The "container" mode of TravisCI is going away. Our .travis.yml + file is getting prepared for the transition. + (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). + + * Our test scripts can now take the '-V' option as a synonym for the + '--verbose-log' option. + (merge a5f52c6dab sg/test-verbose-log later to maint). + Fixes since v2.19 - @@ -544,14 +552,6 @@ Fixes since v2.19 didn't make much sense. This has been corrected. (merge 669b1d2aae md/exclude-promisor-objects-fix later to maint). - * The "container" mode of TravisCI is going away. Our .travis.yml - file is getting prepared for the transition. - (merge 32ee384be8 ss/travis-ci-force-vm-mode later to maint). - - * Our test scripts can now take the '-V' option as a synonym for the - '--verbose-log' option. - (merge a5f52c6dab sg/test-verbose-log later to maint). - * A regression in Git 2.12 era made "git fsck" fall into an infinite loop while processing truncated loose objects. (merge 18ad13e5b2 jk/detect-truncated-zlib-input later to maint). -- 2.20.0.rc2.1.gfcc5f94f1e
[PATCH 2/3] RelNotes 2.20: clarify sentence
I had to read this sentence a few times to understand it. Let's try to clarify it. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index e5ab8cc609..201135d80c 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -305,7 +305,7 @@ Performance, Internal Implementation, Development Support etc. * The overly large Documentation/config.txt file have been split into million little pieces. This potentially allows each individual piece - included into the manual page of the command it affects more easily. + to be included into the manual page of the command it affects more easily. * Replace three string-list instances used as look-up tables in "git fetch" with hashmaps. -- 2.20.0.rc2.1.gfcc5f94f1e
[PATCH 0/3] Re: [ANNOUNCE] Git v2.20.0-rc2
Hi Junio, > A release candidate Git v2.20.0-rc2 is now available for testing > at the usual places. It is comprised of 934 non-merge commits > since v2.19.0, contributed by 76 people, 25 of which are new faces. Here are a few suggested tweaks after reading the draft release notes. Nothing critical. Martin Martin Ågren (3): RelNotes 2.20: move some items between sections RelNotes 2.20: clarify sentence RelNotes 2.20: drop spurious double quote Documentation/RelNotes/2.20.0.txt | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) -- 2.20.0.rc2.1.gfcc5f94f1e
[PATCH 3/3] RelNotes 2.20: drop spurious double quote
We have three double-quote characters, which is one too many or too few. Dropping the last one seems to match the original intention best. Signed-off-by: Martin Ågren --- Documentation/RelNotes/2.20.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.20.0.txt b/Documentation/RelNotes/2.20.0.txt index 201135d80c..e71fe3dee1 100644 --- a/Documentation/RelNotes/2.20.0.txt +++ b/Documentation/RelNotes/2.20.0.txt @@ -578,7 +578,7 @@ Fixes since v2.19 * "git rev-parse --exclude=* --branches --branches" (i.e. first saying "add only things that do not match '*' out of all branches" - and then adding all branches, without any exclusion this time") + and then adding all branches, without any exclusion this time) worked as expected, but "--exclude=* --all --all" did not work the same way, which has been fixed. (merge 5221048092 ag/rev-parse-all-exclude-fix later to maint). -- 2.20.0.rc2.1.gfcc5f94f1e
[PATCH v2] range-diff: always pass at least minimal diff options
Commit d8981c3f88 ("format-patch: do not let its diff-options affect
--range-diff", 2018-11-30) taught `show_range_diff()` to accept a
NULL-pointer as an indication that it should use its own "reasonable
default". That fixed a regression from a5170794 ("Merge branch
'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced
a regression of its own.
In particular, it means we forget the `file` member of the diff options,
so rather than placing a range-diff in the cover-letter, we write it to
stdout. In order to fix this, rewrite the two callers adjusted by
d8981c3f88 to instead create a "dummy" set of diff options where they
only fill in which file to use.
Plus, turn off coloring to make sure we don't write any color codes.
Maybe we could do `opts.use_color = opts.file != stdout`, but for now,
I'd much rather always write uncolored output than write color codes
where there shouldn't be any.
Modify and extend the existing tests to try and verify that the right
contents end up in the right place.
Don't revert `show_range_diff()`, i.e., let it keep accepting NULL.
Rather than removing what is dead code and figuring out it isn't
actually dead and we've broken 2.20, just leave it for now.
Signed-off-by: Martin Ågren
---
Here's another attempt at fixing this recent regression.
t/t3206-range-diff.sh | 20 +---
builtin/log.c | 13 -
log-tree.c| 13 -
3 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e497c1358f..048feaf6dd 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -248,18 +248,24 @@ test_expect_success 'dual-coloring' '
for prev in topic master..topic
do
test_expect_success "format-patch --range-diff=$prev" '
- git format-patch --stdout --cover-letter --range-diff=$prev \
+ git format-patch --cover-letter --range-diff=$prev \
master..unmodified >actual &&
- grep "= 1: .* s/5/A" actual &&
- grep "= 2: .* s/4/A" actual &&
- grep "= 3: .* s/11/B" actual &&
- grep "= 4: .* s/12/B" actual
+ test_when_finished "rm 000?-*" &&
+ test_line_count = 5 actual &&
+ test_i18ngrep "^Range-diff:$" -* &&
+ grep "= 1: .* s/5/A" -* &&
+ grep "= 2: .* s/4/A" -* &&
+ grep "= 3: .* s/11/B" -* &&
+ grep "= 4: .* s/12/B" -*
'
done
test_expect_success 'format-patch --range-diff as commentary' '
- git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual &&
- test_i18ngrep "^Range-diff:$" actual
+ git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&
+ test_when_finished "rm 0001-*" &&
+ test_line_count = 1 actual &&
+ test_i18ngrep "^Range-diff:$" 0001-* &&
+ grep "> 1: .* new message" 0001-*
'
test_done
diff --git a/builtin/log.c b/builtin/log.c
index 5ac18e2848..e42487b46d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1094,9 +1094,20 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
}
if (rev->rdiff1) {
+ /*
+* (At least for now) we only want to pass down
+* the file handle where we want the range-diff
+* to appear. Avoid any other diff options until
+* we know how we want to handle them.
+*/
+ struct diff_options opts;
+ diff_setup();
+ opts.file = rev->diffopt.file;
+ opts.use_color = 0;
+ diff_setup_done();
fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
show_range_diff(rev->rdiff1, rev->rdiff2,
- rev->creation_factor, 1, NULL);
+ rev->creation_factor, 1, );
}
}
diff --git a/log-tree.c b/log-tree.c
index b243779a0b..fd79a3ec37 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -755,14 +755,25 @@ void show_log(struct rev_info *opt)
if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
struct diff_queue_struct dq;
+ struct diff_options opts;
memcpy(, _queued_diff, sizeof(diff_queued_diff));
DIFF_QUEUE_CLEAR(_queued_diff);
next_commentary_block(opt, NULL);
fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+ /*
+* (At least for now) we only want to pass down
+* the file handle where we want the range-diff
+* to appear. Avoid any other diff options until
+* we know how we want to handle them.
+*/
+ diff_setup();
+ opts.file = opt->diffopt.file;
+ opts.use_color = 0;
+ diff_setup_done();
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
[PATCH] revisions.c: put promisor option in specialized struct
Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
it was in rev_info, it was unclear when it was used, since rev_info is
passed to functions that don't use the flag. This resulted in
unnecessary setting of the flag in prune.c, so fix that as well.
Signed-off-by: Matthew DeVore
---
builtin/pack-objects.c | 7 +--
builtin/prune.c| 1 -
builtin/rev-list.c | 6 --
revision.c | 10 ++
revision.h | 4 ++--
5 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24bba8147f..b22c99f540 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3084,14 +3084,17 @@ static void record_recent_commit(struct commit *commit,
void *data)
static void get_object_list(int ac, const char **av)
{
struct rev_info revs;
+ struct setup_revision_opt s_r_opt;
char line[1000];
int flags = 0;
int save_warning;
repo_init_revisions(the_repository, , NULL);
save_commit_buffer = 0;
- revs.allow_exclude_promisor_objects_opt = 1;
- setup_revisions(ac, av, , NULL);
+
+ memset(_r_opt, 0, sizeof(s_r_opt));
+ s_r_opt.allow_exclude_promisor_objects = 1;
+ setup_revisions(ac, av, , _r_opt);
/* make sure shallows are read */
is_repository_shallow(the_repository);
diff --git a/builtin/prune.c b/builtin/prune.c
index e42653b99c..1ec9ddd751 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -120,7 +120,6 @@ int cmd_prune(int argc, const char **argv, const char
*prefix)
save_commit_buffer = 0;
read_replace_refs = 0;
ref_paranoia = 1;
- revs.allow_exclude_promisor_objects_opt = 1;
repo_init_revisions(the_repository, , prefix);
argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3a2c0c23b6..c3095c6fed 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -362,6 +362,7 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
{
struct rev_info revs;
struct rev_list_info info;
+ struct setup_revision_opt s_r_opt;
int i;
int bisect_list = 0;
int bisect_show_vars = 0;
@@ -375,7 +376,6 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
git_config(git_default_config, NULL);
repo_init_revisions(the_repository, , prefix);
revs.abbrev = DEFAULT_ABBREV;
- revs.allow_exclude_promisor_objects_opt = 1;
revs.commit_format = CMIT_FMT_UNSPECIFIED;
revs.do_not_die_on_missing_tree = 1;
@@ -407,7 +407,9 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
}
}
- argc = setup_revisions(argc, argv, , NULL);
+ memset(_r_opt, 0, sizeof(s_r_opt));
+ s_r_opt.allow_exclude_promisor_objects = 1;
+ argc = setup_revisions(argc, argv, , _r_opt);
memset(, 0, sizeof(info));
info.revs =
diff --git a/revision.c b/revision.c
index 13e0519c02..f6b32e6a42 100644
--- a/revision.c
+++ b/revision.c
@@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const
char *pattern)
}
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
- int *unkc, const char **unkv)
+ int *unkc, const char **unkv,
+ const struct setup_revision_opt* opt)
{
const char *arg = argv[0];
const char *optarg;
@@ -2151,7 +2152,7 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
revs->limited = 1;
} else if (!strcmp(arg, "--ignore-missing")) {
revs->ignore_missing = 1;
- } else if (revs->allow_exclude_promisor_objects_opt &&
+ } else if (opt && opt->allow_exclude_promisor_objects &&
!strcmp(arg, "--exclude-promisor-objects")) {
if (fetch_if_missing)
BUG("exclude_promisor_objects can only be used when
fetch_if_missing is 0");
@@ -2173,7 +2174,7 @@ void parse_revision_opt(struct rev_info *revs, struct
parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
- >cpidx, ctx->out);
+ >cpidx, ctx->out, NULL);
if (n <= 0) {
error("unknown option `%s'", ctx->argv[0]);
usage_with_options(usagestr, options);
@@ -2391,7 +2392,8 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
continue;
}
- opts = handle_revision_opt(revs, argc - i, argv + i,
, argv);
+ opts = handle_revision_opt(revs, argc - i, argv + i,
+
Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
On 12/01/2018 11:44 AM, Jeff King wrote:
repo_init_revisions(the_repository, , NULL);
save_commit_buffer = 0;
- revs.allow_exclude_promisor_objects_opt = 1;
- setup_revisions(ac, av, , NULL);
+
+ memset(_r_opt, 0, sizeof(s_r_opt));
+ s_r_opt.allow_exclude_promisor_objects = 1;
+ setup_revisions(ac, av, , _r_opt);
I wonder if a static initializer for setup_revision_opt is worth it. It
would remove the need for this memset. Probably not a big deal either
way, though.
I think you mean something like this:
static struct setup_revision_opt s_r_opt = {NULL, NULL, NULL, 0, 1, 0};
This is a bit cryptic (I have to read the struct declaration in order to
know what is being set to 1) and if the struct ever gets a new field
before allow_exclude_promisor_objects, this initializer has to be updated.
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
- int *unkc, const char **unkv)
+ int *unkc, const char **unkv,
+ int allow_exclude_promisor_objects)
Why not pass in the whole setup_revision_opt struct? We don't need
anything else from it yet, but it seems like the point of that struct is
to pass around preferences like this.
OK, the code reads better if I do that, so I agree.
-Peff
Re: [PATCH] rebase -i: introduce the 'test' command
Hi Duy, On Mon, 3 Dec 2018, Duy Nguyen 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. > > And here I've been doing the same by "edit" the first commit, add a > new commit then reorder them in the second interactive rebase :P > > This made me look at git-rebase.txt to really learn about interactive > rebase. I think the interactive rebase section could use some > improvements. Its style looks.. umm.. more story telling than a > reference. Perhaps something like this to at least highlight the > commands. And maybe, just maybe, that "story telling" is more useful for users who want to learn about the interactive rebase, just like yourself, when compared to a mere "reference". Ciao, Johannes
Re: [PATCH] rebase -i: introduce the 'test' command
Hi Luc, On Mon, 3 Dec 2018, Luc Van Oostenryck wrote: > 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. Varieties of this have been discussed, but no, not nested rebases. 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 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 (assuming that the latest 3 commits were non-merge commits; It would look quite a bit more complicated in other situations.) Ciao, Dscho
Re: [PATCH] rebase -i: introduce the 'test' command
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.
And here I've been doing the same by "edit" the first commit, add a
new commit then reorder them in the second interactive rebase :P
This made me look at git-rebase.txt to really learn about interactive
rebase. I think the interactive rebase section could use some
improvements. Its style looks.. umm.. more story telling than a
reference. Perhaps something like this to at least highlight the
commands.
-- 8< --
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 80793bad8d..c569b3370b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -637,22 +637,22 @@ The oneline descriptions are purely for your pleasure;
'git rebase' will
not look at them but at the commit names ("deadbee" and "fa1afe1" in this
example), so do not delete or edit the names.
-By replacing the command "pick" with the command "edit", you can tell
+By replacing the command `pick` with the command `edit`, you can tell
'git rebase' to stop after applying that commit, so that you can edit
the files and/or the commit message, amend the commit, and continue
rebasing.
To interrupt the rebase (just like an "edit" command would do, but without
-cherry-picking any commit first), use the "break" command.
+cherry-picking any commit first), use the `break` command.
If you just want to edit the commit message for a commit, replace the
-command "pick" with the command "reword".
+command "pick" with the command `reword`.
-To drop a commit, replace the command "pick" with "drop", or just
+To drop a commit, replace the command "pick" with `drop`, or just
delete the matching line.
If you want to fold two or more commits into one, replace the command
-"pick" for the second and subsequent commits with "squash" or "fixup".
+"pick" for the second and subsequent commits with `squash` or `fixup`.
If the commits had different authors, the folded commit will be
attributed to the author of the first commit. The suggested commit
message for the folded commit is the concatenation of the commit
@@ -693,7 +693,7 @@ $ git rebase -i -p --onto Q O
Reordering and editing commits usually creates untested intermediate
steps. You may want to check that your history editing did not break
anything by running a test, or at least recompiling at intermediate
-points in history by using the "exec" command (shortcut "x"). You may
+points in history by using the `exec` command (shortcut `x`). You may
do so by creating a todo list like this one:
---
-- 8< --
--
Duy
[PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
The text body of section Behavioral Differences is typeset as code, but should be regular text. Remove the indentation to achieve that. While here, prettify the language: - use "the x backend" instead of "x-based rebase"; - use present tense instead of future tense; and use subsections instead of a list. Signed-off-by: Johannes Sixt --- Cf. https://git-scm.com/docs/git-rebase#_behavioral_differences I actually did not test the result, because I don't have the infrastructure. The sentence "The am backend sometimes does not" is not very useful, but is not my fault ;) It would be great if it could be made more specific, but I do not know the details. Documentation/git-rebase.txt | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 80793bad8d..dff17b3178 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -550,24 +550,28 @@ Other incompatible flag pairs: BEHAVIORAL DIFFERENCES --- - * empty commits: +There are some subtle differences how the backends behave. -am-based rebase will drop any "empty" commits, whether the -commit started empty (had no changes relative to its parent to -start with) or ended empty (all changes were already applied -upstream in other commits). +Empty commits +~ + +The am backend drops any "empty" commits, regardless of whether the +commit started empty (had no changes relative to its parent to +start with) or ended empty (all changes were already applied +upstream in other commits). -merge-based rebase does the same. +The merge backend does the same. -interactive-based rebase will by default drop commits that -started empty and halt if it hits a commit that ended up empty. -The `--keep-empty` option exists for interactive rebases to allow -it to keep commits that started empty. +The interactive backend drops commits by default that +started empty and halts if it hits a commit that ended up empty. +The `--keep-empty` option exists for the interactive backend to allow +it to keep commits that started empty. - * directory rename detection: +Directory rename detection +~~ -merge-based and interactive-based rebases work fine with -directory rename detection. am-based rebases sometimes do not. +The merge and interactive backends work fine with +directory rename detection. The am backend sometimes does not. include::merge-strategies.txt[] -- 2.19.1.1133.g2dd3d172d2
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] rebase -i: introduce the 'test' command
On 01/12/2018 20:02, 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. How would I move past the test that fails to continue? I guess "git rebase --edit-todo" and then manually remove it (and any other remaining test lines)? Perhaps we could teach git rebase --skip to skip a rescheduled command, it could be useful if people want to skip rescheduled picks as well (though I don't think I've ever had that happen in the wild). I can see myself turning on the rescheduling config setting but occasionally wanting to be able to skip over the rescheduled exec command. Best Wishes Phillip That's not too bad, but I wonder if people would find it more awkward than the current way (which is to just "rebase --continue" until you get to the end). I dunno. I am not sure if I am for or against changing the default, so these are just some musings. :) -Peff
Re: [PATCH] format-patch: do not let its diff-options affect --range-diff (was Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options)
On Fri, 30 Nov 2018 at 10:32, Eric Sunshine wrote:
>
> On Thu, Nov 29, 2018 at 11:27 PM Junio C Hamano wrote:
> > Junio C Hamano writes:
> > So how about doing this on top of 'master' instead? As this leaks
> > *no* information wrt how range-diff machinery should behave from the
> > format-patch side by not passing any diffopt, as long as the new
> > code I added to show_range_diff() comes up with a reasonable default
> > diffopts (for which I really would appreciate extra sets of eyes to
> > make sure), this change by definition cannot be wrong (famous last
> > words).
> Another benefit of calling show_range_diff() directly is that when
> "git format-patch --stdout --range-diff=..." is sent to the terminal,
> the range-diff gets colored output for free. I'm pleased to see that
> that still works after this change.
(If the patch below makes any sense to you and you know more about this
diff/color thing, the fourth bullet in the log message below might
interest you.)
> > diff --git a/range-diff.h b/range-diff.h
> > @@ -5,6 +5,11 @@
> > +/*
> > + * Compare series of commmits in RANGE1 and RANGE2, and emit to the
> > + * standard output. NULL can be passed to DIFFOPT to use the built-in
> > + * default.
> > + */
>
> It is more correct to say that the range-diff is emitted to
> diffopt->file (which may be stdout).
This seems to be an important remark. There's a pretty bad regression
here since when `diffopt` is NULL, we've lost our original, intended
`diffopt->file` and the range-diff ends up on stdout.
Here's my whitespace-damaged WIP. I would be able to pick this up again
in about 6h, but anyone is more than welcome to pick this up and run
with it in the meantime.
This is not a corner of the code that I'm particularly familiar with...
-->8--
Subject: [PATCH] range-diff: always pass at least minimal diff options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit d8981c3f88 ("format-patch: do not let its diff-options affect
--range-diff", 2018-11-30) taught `show_range_diff()` to accept a
NULL-pointer as an indication that it should use its own "reasonable
default". That fixed a regression from a5170794 ("Merge branch
'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced
a regression of its own.
In particular, it means we forget the `file` member of the diff options,
so rather than placing a range-diff in the cover-letter, we write it to
stdout. In order to fix this, rewrite the two callers adjusted by
d8981c3f88 to create a "dummy" set of diff options where they only fill
in which file to use.
A couple of remarks about this commit:
* No tests. The change in builtin/log.c has been tested manually, the
one in log-tree.c not at all, other than by running existing tests.
* I have not convinced myself 100% that there aren't other things that
are just as important as `file` to pass down.
* `show_range_diff()` can still take NULL, although that is now dead
code. If something like this here commit is deemed the proper fix
for this, that code path could also go, either as part of this
commit, or separately, once we've cut 2.20.
* The range-diff is written colored regardless of destination, i.e.,
when written to a file, it contains garbage.
Signed-off-by: Martin Ågren
---
builtin/log.c | 12 +++-
log-tree.c| 12 +++-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 5ac18e2848..0609e41ae5 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1094,9 +1094,19 @@ static void make_cover_letter(struct rev_info
*rev, int use_stdout,
}
if (rev->rdiff1) {
+/**
+ * (At least for now) we only want to pass down
+ * the file handle where we want the range-diff
+ * to appear. Avoid any other diff options until
+ * we know how we want to handle them.
+ */
+struct diff_options opts;
+diff_setup();
+opts.file = rev->diffopt.file;
+diff_setup_done();
fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
show_range_diff(rev->rdiff1, rev->rdiff2,
-rev->creation_factor, 1, NULL);
+rev->creation_factor, 1, );
}
}
diff --git a/log-tree.c b/log-tree.c
index b243779a0b..bc355a4e91 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -755,14 +755,24 @@ void show_log(struct rev_info *opt)
if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
struct diff_queue_struct dq;
+struct diff_options opts;
memcpy(, _queued_diff, sizeof(diff_queued_diff));
DIFF_QUEUE_CLEAR(_queued_diff);
next_commentary_block(opt, NULL);
fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+/**
+ * (At least for now) we only want to pass down
+ * the file handle where we want the range-diff
+ * to appear. Avoid any other diff options until
+ * we know how
My Greetings
Beloved, I am writing this mail to you with heavy tears in my eyes and great sorrow in my heart. As I informed you earlier, I am (Mrs.)Marianne Jeanne, suffering from long time Cancer. From all indications my condition is really deteriorating and it's quite obvious that I won't live more than 2 months according to my doctors. I have some funds I inherited from my late loving husband Mr. Jeanne Smith, the sum of (€8.5000.000) which he deposited in a Bank. I need a very honest and God fearing person that can use these funds for Charity work, helping the Less Privileges, and 20% of this money will be for your time and expenses, while 80% goes to charities. Please let me know if I can TRUST YOU ON THIS to carry out this favour for me. I look forward to your prompt reply for more details. Yours sincerely Marianne Jeanne
