Re: [PATCH v12 2/5] test-parse-options: print quiet as integer
On Tue, Apr 5, 2016 at 11:39 AM, Pranit Bauvawrote: > On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshine wrote: >> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva wrote: >>> Current implementation of parse-options.c treats OPT__QUIET() as integer >>> and not boolean and thus it is more appropriate to print it as integer >>> to avoid confusion. >> >> I can buy this line of reasoning, however, it would be even easier to >> sell the change if you cited an existing client (a git command) which >> actually respects multiple quiet levels. Are there any? > > I investigated into this. But I was unable to find any git commit > which actually respects mulitple quiet levels. I first did a 'git grep > OPT__QUIET' to find the commands which use this. Then I went through > the documentation which covers it. None of them have any such mention > of multiple quiet levels. But still I dug further and and went through > all the individual source files. I followed the corresponding C source > code for the header file included and also searched there for any > trace of quiet. But I still didn't find any such use of multiple quiet > levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec, > 2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to > say as to why this was introduced and OPT__QUIET which previously used > OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit > In fact git-repack command has quiet but this is not mentioned in the > documentation. If someone could include this it in the documentation. > I would do it but I am not quite familiar with git-repack and haven't > really used it anytime. I didn't find any existing Git command recognizing multiple quiet levels either, however, I think I can still buy the change considering that there is existing precedent in other Unix commands. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable
On Tue, Apr 5, 2016 at 6:21 AM, Elia Pintowrote: > 2016-04-01 22:25 GMT+02:00 Eric Sunshine : >> In addition to review comments by others, why are the new curl_dump() >> and curl_trace() functions duplicated in both patches rather than >> factored out to a shared implementation? > > It's right. Do you think i can use some existing file or should I > create a new object file ? Peff or Junio would be more qualified to answer, but perhaps the shared implementation could go in http.c? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] correct blame for files commited with CRLF
On 05.04.16 23:12, Junio C Hamano wrote: > tbo...@web.de writes: > >> +git config core.autocrlf true && >> +mv crlfinrepo tmp && >> +git checkout crlfinrepo && >> +rm tmp && > > Why not just "rm -f crlfinrepo" and "git checkout crlfinrepo"? The intention was to get a new inode number, which marks the file in the working tree as modified, regardless of mtime. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ATTN: BENEFICIARY
-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Can `git grep` have `--Author` option?
This is a feature request. >From time to time, I want to re-use MY code. So git grep with --Author options >is really useful. My current git version is 2.6.3, looks it doesn't have this >option. -- Best Regards, Chen Bin -- Help me, help you -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff --exit-code does not honour textconv setting
Michael J Gruberwrites: >> The "test" command is used as it does not generate any output on stdout. > > "test" is a bit of a red herring here since it will receive commands. > But your example works even with "true" which ignores its commands and > produces no output. Either sounds strange, as they exit without reading their input at all, so the other side of the pipe may get an write error it has to handle ;-) > The description doesn't make it clear whether exit-code refers to > the actual diff (foo vs. bar) or to the diff after textconv (empty > vs. empty). In any case, "-b" should not make a difference for > your example. > > diff_flush() in diff.c has this piece of code: > It is understandable that "-b" makes difference. The actual short-cut happens a bit before the code you quoted. if (output_format & DIFF_FORMAT_NO_OUTPUT && DIFF_OPT_TST(options, EXIT_WITH_STATUS) && DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) { /* * run diff_flush_patch for the exit status. setting * options->file to /dev/null should be safe, because we * aren't supposed to produce any output anyway. */ if (options->close_file) fclose(options->file); options->file = fopen("/dev/null", "w"); if (!options->file) die_errno("Could not open /dev/null"); options->close_file = 1; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; if (check_pair_status(p)) diff_flush_patch(p, options); if (options->found_changes) break; } } When running without producing output but we need the exit status, unless DIFF_FROM_CONTENTS is set (e.g. "-b" in use), we do not run the code that would inspect the blob contents that would have produced the actual patch. When DIFF_FROM_CONTENTS is set, we need to dig into the blob contents and the body of the above if() gets run only to trigger o->found_changes (e.g. in builtin_diff() that sets ecbdata.found_changesp to point at o->found_changes before calling into the xdiff machinery), but the output is discarded to /dev/null. The textconv filter is an unfortunate beast, in that while some paths use one while others don't, so it won't be as simple as "-b" that flips DIFF_FROM_CONTENTS to say "We need to inspect blobs for ALL FILEPAIRS to see if there is any difference" if we want to keep the optimization as much as possible. Without DIFF_FROM_CONTENTS set, any filepair that point at two different blobs that might end up to be the same after applying textconv will flip the o->found_changes bit on, and with EXIT_WITH_STATUS bit on, we have another optimization to skip checking the remainder of the filepairs. So if you want to support textconv with QUICK, you would also need to disable that optimization by teaching diff_change() to refrain from setting HAS_CHANGES bit, i.e. diff --git a/diff.h b/diff.h index 125447b..f6c0c07 100644 --- a/diff.h +++ b/diff.h @@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) #define DIFF_OPT_RENAME_EMPTY(1 << 8) -/* (1 << 9) unused */ +#define DIFF_OPT_WITH_TEXTCONV (1 << 9) #define DIFF_OPT_HAS_CHANGES (1 << 10) #define DIFF_OPT_QUICK (1 << 11) #define DIFF_OPT_NO_INDEX(1 << 12) diff --git a/diff.c b/diff.c index 4dfe660..0016ad2 100644 --- a/diff.c +++ b/diff.c @@ -5018,6 +5018,11 @@ void diff_change(struct diff_options *options, two->dirty_submodule = new_dirty_submodule; p = diff_queue(_queued_diff, one, two); + if (either path one or path two has textconv defined) { + DIFF_OPT_SET(options, DIFF_WITH_TEXTCONV); + return; + } + if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) return; And then in order to keep the optimization, add this to the above codepath: diff --git a/diff.c b/diff.c index 4dfe660..7b318cc 100644 --- a/diff.c +++ b/diff.c @@ -4598,6 +4598,7 @@ void diff_flush(struct diff_options *options) int i, output_format = options->output_format; int separator = 0; int dirstat_by_line = 0; + int textconv_hack; /* * Order: raw, stat, summary, patch @@ -4652,9 +4653,17 @@ void diff_flush(struct diff_options *options) separator++; } + /* +* If there is any path that needs textconv and we haven't
Re: Triangular workflows and some anecdotes from the trenches
Chris Packhamwrites: > We ran into something at $dayjob the other day. The actual problem was > a developer ended up amending a commit that had already been pushed. > It happens occasionally and is usually recoverable with a simple > rebase and is generally a learning experience. In this particular case > however things were a bit more complicated. Developer ending up amending is not an issue per-se, unless the result is pushed back to the public. However, it would be nicer if "git commit --amend", "git rebase", "git branch -f", and "git checkout -B" notices the situation and warns about what would happen. > git config alias.amend '!git merge-base --is-ancestor HEAD > @{upstream} || git commit --amend' > > I'm just wondering if something more official can be added to git > commit --amend (and probably git rebase). A bigger problem may be how you make sure everybody sets up @{upstream} correctly. You may fork your own copy of a branch from the target branch, start working on it, further fork other branches on your work to experiment different approaches, with the intention to later use the best one to update your first fork. At which point, the @{upstream} of the secondary branches are your own first branch, not the public one--which is not a problem per-se, because your first branch (whose @{upstream} is the remote one) is not yet public and you should be allowed to freely update it to polish it by rewriting. But then after you push out your first branch as an interim snapshot to the public, you no longer want to rewrite the commits reachable from it. So (to put it mildly) it would be quite complex to get all the corner cases right, and the definition of "right" would probably depend on the exact workflow. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
reply
Good day, friend. I'm a Banker with HSBC here in Malaysia.I have an interesting business deal for you that will be of immense benefit to both of us.Although this may be hard for you to believe,we stand to gain $7million in a matter of days. Please grant me the benefit of doubt and hear me out. I need you to signify your interest by replying to this mail. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: run arguments through precompose_argv
Torsten Bögershausenwrites: > On 05.04.16 21:15, Johannes Sixt wrote: >> Am 05.04.2016 um 19:09 schrieb Junio C Hamano: Thanks-to: Torsten Bögershausen >> >> I sense NFD disease: The combining diaresis should combine with the o, not >> the g. Here is a correct line to copy-and-paste if you like: >> >> Thanks-to: Torsten Bögershausen >> >> -- Hannes > > Good eyes. > > And thanks to Alexander for doing this patch Yup. It is funny that I wrote: Thanks. The log message talks about "such a file path", but precompose_argv() applies indiscriminately on any command line arguments, so things like -G would also get the same treatment, which I think is what most users would want. but "git log --grep=" with your name would not have found the one that would have resulted from the original e-mail message applied. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: more meaningful Message-ID
Junio C Hamanowrote: > Eric Wong writes: > > > Using a mmddHHMMSS date representation is more meaningful to > > humans, especially when used for lookups on NNTP servers or linking > > to archive sites via Message-ID (e.g. mid.gmane.org or > > mid.mail-archive.com). This timestamp format more easily gives a > > reader of the URL itself a rough date of a linked message compared > > to having them calculate the seconds since the Unix epoch. > > > > Furthermore, having the MUA name in the Message-ID seems to be a > > rare oddity I haven't noticed outside of git-send-email. We > > already have an optional X-Mailer header field to advertise for > > us, so extending the Message-ID by 15 characters can make for > > unpleasant Message-ID-based URLs to archive sites. > > > > Signed-off-by: Eric Wong > > --- > > Sounds like a sensible goal. Just a few comments. > > - Is it safe to assume that we always can use POSIX::strftime(), or >do we need some fallback? I am guessing that this is safe, as >POSIX has been part of the core modules for a long time, and the >script does "use 5.008" upfront. I'm hoping so :) And none of the format specifiers used here should be subject to locale-dependent weirdness, at least. +Cc both Johannes for Windows knowledge. > - It is my understanding that, as "use" is a compilation-time >thing, hiding it inside a block does not help reducing the >start-up overhead (people can use "require" if they want to do a >lazy loading and optionally a fallback). Is my Perl5 outdated? >Otherwise, let's have it near the beginning of the script, close >to where we use Term::ReadLine and others. You're correct, I'll move the "use" to the top in v2. I could call "require" and call the sub as "POSIX::strftime", but this code path is likely enough that any startup time improvement for uncommon cases wouldn't be worth it. Will wait a bit for strftime portability comments before v2. > > --- a/git-send-email.perl > > +++ b/git-send-email.perl > > @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial); > > sub make_message_id { > > my $uniq; > > if (!defined $message_id_stamp) { > > - $message_id_stamp = sprintf("%s-%s", time, $$); > > + use POSIX qw/strftime/; > > + $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time)); > > $message_id_serial = 0; > > } > > $message_id_serial++; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] correct blame for files commited with CRLF
tbo...@web.de writes: > + git config core.autocrlf true && > + mv crlfinrepo tmp && > + git checkout crlfinrepo && > + rm tmp && Why not just "rm -f crlfinrepo" and "git checkout crlfinrepo"? > + git blame crlfinrepo >actual && > + grep "A U Thor" actual > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: more meaningful Message-ID
Eric Wongwrites: > Using a mmddHHMMSS date representation is more meaningful to > humans, especially when used for lookups on NNTP servers or linking > to archive sites via Message-ID (e.g. mid.gmane.org or > mid.mail-archive.com). This timestamp format more easily gives a > reader of the URL itself a rough date of a linked message compared > to having them calculate the seconds since the Unix epoch. > > Furthermore, having the MUA name in the Message-ID seems to be a > rare oddity I haven't noticed outside of git-send-email. We > already have an optional X-Mailer header field to advertise for > us, so extending the Message-ID by 15 characters can make for > unpleasant Message-ID-based URLs to archive sites. > > Signed-off-by: Eric Wong > --- Sounds like a sensible goal. Just a few comments. - Is it safe to assume that we always can use POSIX::strftime(), or do we need some fallback? I am guessing that this is safe, as POSIX has been part of the core modules for a long time, and the script does "use 5.008" upfront. - It is my understanding that, as "use" is a compilation-time thing, hiding it inside a block does not help reducing the start-up overhead (people can use "require" if they want to do a lazy loading and optionally a fallback). Is my Perl5 outdated? Otherwise, let's have it near the beginning of the script, close to where we use Term::ReadLine and others. Thanks. > git-send-email.perl | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index d356901..23141e7 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial); > sub make_message_id { > my $uniq; > if (!defined $message_id_stamp) { > - $message_id_stamp = sprintf("%s-%s", time, $$); > + use POSIX qw/strftime/; > + $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time)); > $message_id_serial = 0; > } > $message_id_serial++; > @@ -964,7 +965,7 @@ sub make_message_id { > require Sys::Hostname; > $du_part = 'user@' . Sys::Hostname::hostname(); > } > - my $message_id_template = "<%s-git-send-email-%s>"; > + my $message_id_template = "<%s-%s>"; > $message_id = sprintf($message_id_template, $uniq, $du_part); > #print "new message id = $message_id\n"; # Was useful for debugging > } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is OS X still supported?
Thanks for the information that binary builds are availably on SourceForge faster than on git-scm. I can see the v2.8.1 for OS X was uploaded few hours ago to the SF, so my main problem (lack of security fixes in git for OS X) is solved. The automation process should be probably reviewed, though - because all the other folks around the world using git-scm (not the SF) to download OS X builds are still stuck at v2.6.4. Ideally git-scm would point to the new Mac version within single minutes since the release (or even seconds) - not hours, days, or weeks. >From my point of view SourceForge vs GitHub is kinda implementation detail. I'd go with GitHub as it's more convenient to use and supported HTTPS since the beginning. And then SF had really bad idea with pushing malware (see https://sourceforge.net/p/forge/site-support/7414/). But as long as git-scm will be getting binaries on time most folks won't really care about details of delivery process. On Tue, Apr 5, 2016 at 6:43 PM, Tim Harperwrote: > It is still supported. I'm not sure why git-scm is pointing to the wrong > version. There's been some discussion to upload to github instead, which I'm > for, but SourceForge publishing is already automated. > >> On Apr 5, 2016, at 10:38, Junio C Hamano wrote: >> >> Michał Staruch writes: >> >>> I'd like to ask if OS X is still supported platform for git. Sources >>> and Windows build were updated to version 2.8.1, while OS X build >>> stopped at 2.6.4, staying vulnerable to CVE-2016-2315 and >>> CVE-2016-2324. >> >> Thanks for asking. >> >> Tim Harper (CC'ed) helps the OSX users by supplying the OSX >> installer. >> >> I think git-scm.com attempts to show the latest OSX installer from >> https://sourceforge.net/projects/git-osx-installer/. >> >> It's funny that that >> >> https://sourceforge.net/projects/git-osx-installer/files/ >> >> does list 2.7.1 that is newer than 2.6.4, but the quick download >> link on that page points at 2.6.2; there is something screwy >> happening at sourceforge. I am not sure how git-scm.com chooses to >> claim that 2.6.4 is the latest. There seems to be an issue open on >> this. >> >>https://github.com/git/git-scm.com/issues/715 >> >> As I do not do binary packaging for individual platforms, I cannot >> be of more help than what this message says; sorry about that. >> >> Next time please send any message that is related to Git to either >> git@vger.kernel.org mailing list (public) or if you want to >> privately discuss security related issues that are not yet known to >> the public, then to git-secur...@googlegroups.com [*1*]. There are >> at least three reasons to do so: >> >> - A message that is addressed only to gits...@pobox.com and not one >> of these lists are often eaten by spam filters and will not be >> seen by me. >> >> - I am not an expert on everything that is related to Git (this >> topic is a good example), and people more qualified to answer are >> on these lists. >> >> - I suspect that you are not the only Git user on OSX, so there >> must be more people wondering the same thing as you are, so >> asking git@vger.kernel.org would help other OSX users. >> >> I almost added "Cc: git@vger.kernel.org" myself on this response, >> but I didn't because there might be a reason for you to hide your >> e-mail address from the public (some people are weird that way, and >> you might be one of them but I couldn't tell because I do not know >> you). If you do not mind helping other OSX users, I am fine if you >> CC'ed your response to this message to git@vger.kernel.org while >> quoting everything I wrote here. >> >> Thanks. >> >> >> [Footnote] >> >> *1* Both of these two lists accept messages from non-subscribers, >> i.e. you can send messages to them without subscribing to them, and >> you'll be kept in the loop in the discussion by CC'ing the original >> poster. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] correct blame for files commited with CRLF
tbo...@web.de writes: > This fix is completely independent of the rest of the series, > so break out 6/7 from tb/safe-crlf-output. Sounds sensible. It is somewhat sad and strange that we need to rely on what is in the index to show the current working tree state, but this makes the things more consistent. Will queue. Thanks. > builtin/blame.c | 1 + > t/t8003-blame-corner-cases.sh | 14 ++ > 2 files changed, 15 insertions(+) > > diff --git a/builtin/blame.c b/builtin/blame.c > index e982fb8..21f42b0 100644 > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -2307,6 +2307,7 @@ static struct commit *fake_working_tree_commit(struct > diff_options *opt, > unsigned mode; > struct strbuf msg = STRBUF_INIT; > > + read_cache(); > time(); > commit = alloc_commit_node(); > commit->object.parsed = 1; > diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh > index 6568429..a9b266f 100755 > --- a/t/t8003-blame-corner-cases.sh > +++ b/t/t8003-blame-corner-cases.sh > @@ -212,4 +212,18 @@ test_expect_success 'blame file with CRLF attributes > text' ' > grep "A U Thor" actual > ' > > +test_expect_success 'blame file with CRLF core.autocrlf=true' ' > + git config core.autocrlf false && > + printf "testcase\r\n" >crlfinrepo && > + >.gitattributes && > + git add crlfinrepo && > + git commit -m "add crlfinrepo" && > + git config core.autocrlf true && > + mv crlfinrepo tmp && > + git checkout crlfinrepo && > + rm tmp && > + git blame crlfinrepo >actual && > + grep "A U Thor" actual > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] api-trace.txt: fix typo
Eric Sunshinewrites: > On Tue, Apr 5, 2016 at 6:05 AM, Elia Pinto wrote: >> The correct api is trace_printf_key >> >> Signed-off-by: Elia Pinto >> --- >> diff --git a/Documentation/technical/api-trace.txt >> b/Documentation/technical/api-trace.txt >> @@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO); >> static void trace_print_foo(const char *message) >> { >> - trace_print_key(_foo, message); >> + trace_printf_key(_foo, message); >> } > > Since you're touching this already, I wonder if it would make sense to > rewrite this example to avoid the dangerous sending of an arbitrary > string (which might contain %) to a printf-like function. Like this, > for example: > > trace_printf_key(_foo, "%s", message); Thanks, will squash in. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Condominium Owners List
Hi, Would you be interested in reaching out to "Condominium Owners List"? with opt-in verified email addresses from the USA. We also have contacts of Real Estate Investors, Home Owners, Mortgage Holders, Mortgage with Home, Investors List, New Movers List, Golfers Email List, High Net-worth Individuals List, Women's List, and many more... Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address. Please let me know your targeted criteria, so that I can help you out to drive your sales effort in the right direction. Thanks & Regards, Phoebe Calkins --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fsck message override issue
I'm not having luck with overriding the fsck message types (zeroPaddedFilemode specifically). I've tried adding [fsck] zeroPaddedFilemode = ignore to both global and local configs, but it seems to have no effect: $ git --version git version 2.8.0 $ git config --get fetch.fsckobjects true $ git config --get fsck.zeroPaddedFilemode ignore $ git pull remote: Counting objects: 14777, done. remote: Compressing objects: 100% (5495/5495), done. error: object 2b7227859263b6aabcc28355b0b994995b7148b6: zeroPaddedFilemode: contains zero-padded file modes fatal: Error in object fatal: index-pack failed Is this expected behavior and I'm doing something wrong? Thanks. -Seren -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] send-email: more meaningful Message-ID
Using a mmddHHMMSS date representation is more meaningful to humans, especially when used for lookups on NNTP servers or linking to archive sites via Message-ID (e.g. mid.gmane.org or mid.mail-archive.com). This timestamp format more easily gives a reader of the URL itself a rough date of a linked message compared to having them calculate the seconds since the Unix epoch. Furthermore, having the MUA name in the Message-ID seems to be a rare oddity I haven't noticed outside of git-send-email. We already have an optional X-Mailer header field to advertise for us, so extending the Message-ID by 15 characters can make for unpleasant Message-ID-based URLs to archive sites. Signed-off-by: Eric Wong--- git-send-email.perl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index d356901..23141e7 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial); sub make_message_id { my $uniq; if (!defined $message_id_stamp) { - $message_id_stamp = sprintf("%s-%s", time, $$); + use POSIX qw/strftime/; + $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time)); $message_id_serial = 0; } $message_id_serial++; @@ -964,7 +965,7 @@ sub make_message_id { require Sys::Hostname; $du_part = 'user@' . Sys::Hostname::hostname(); } - my $message_id_template = "<%s-git-send-email-%s>"; + my $message_id_template = "<%s-%s>"; $message_id = sprintf($message_id_template, $uniq, $du_part); #print "new message id = $message_id\n"; # Was useful for debugging } -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: run arguments through precompose_argv
On 05.04.16 21:15, Johannes Sixt wrote: > Am 05.04.2016 um 19:09 schrieb Junio C Hamano: >>> Thanks-to: Torsten Bögershausen> > I sense NFD disease: The combining diaresis should combine with the o, not > the g. Here is a correct line to copy-and-paste if you like: > > Thanks-to: Torsten Bögershausen > > -- Hannes Good eyes. And thanks to Alexander for doing this patch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1] correct blame for files commited with CRLF
From: Torsten Bögershausengit blame reports lines as not "Not Committed Yet" when they have CRLF in the index, CRLF in the worktree and core.autocrlf is true. Since commit c48053 "new safer autocrlf handling", files that have CRLF in the index are not normalized at commit when core.autocrl is set. Add a call to read_cache() early in fake_working_tree_commit(), before calling convert_to_git(). Signed-off-by: Torsten Bögershausen --- This fix is completely independent of the rest of the series, so break out 6/7 from tb/safe-crlf-output. The rest of the series will be send in a couple of days, some rework is needed. builtin/blame.c | 1 + t/t8003-blame-corner-cases.sh | 14 ++ 2 files changed, 15 insertions(+) diff --git a/builtin/blame.c b/builtin/blame.c index e982fb8..21f42b0 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2307,6 +2307,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, unsigned mode; struct strbuf msg = STRBUF_INIT; + read_cache(); time(); commit = alloc_commit_node(); commit->object.parsed = 1; diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh index 6568429..a9b266f 100755 --- a/t/t8003-blame-corner-cases.sh +++ b/t/t8003-blame-corner-cases.sh @@ -212,4 +212,18 @@ test_expect_success 'blame file with CRLF attributes text' ' grep "A U Thor" actual ' +test_expect_success 'blame file with CRLF core.autocrlf=true' ' + git config core.autocrlf false && + printf "testcase\r\n" >crlfinrepo && + >.gitattributes && + git add crlfinrepo && + git commit -m "add crlfinrepo" && + git config core.autocrlf true && + mv crlfinrepo tmp && + git checkout crlfinrepo && + rm tmp && + git blame crlfinrepo >actual && + grep "A U Thor" actual +' + test_done -- 2.8.0.rc2.2.g1a4d45a.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: run arguments through precompose_argv
Am 05.04.2016 um 19:09 schrieb Junio C Hamano: Thanks-to: Torsten BögershausenI sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like: Thanks-to: Torsten Bögershausen -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Timestamp of zero in reflog considered invalid
Junio C Hamanowrites: > Checking the value against ULONG_MAX and errno==ERANGE would be an > improvement. It may be debatable if we should silently ignore an > entry with an invalid timestamp, but that is a separate issue. > > refs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/refs.c b/refs.c > index 4e15f60..ff24184 100644 > --- a/refs.c > +++ b/refs.c > @@ -3701,7 +3701,8 @@ static int show_one_reflog_ent(struct strbuf *sb, > each_reflog_ent_fn fn, void *c > get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' || > !(email_end = strchr(sb->buf + 82, '>')) || > email_end[1] != ' ' || > - !(timestamp = strtoul(email_end + 2, , 10)) || > + ((timestamp = strtoul(email_end + 2, , 10)) == ULONG_MAX && > + errno == ERANGE) || You need to set errno = 0 before calling strtoul, to distinguish the valid return of ULONG_MAX (which would keep errno intact) and a real overflow. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] api-trace.txt: fix typo
On Tue, Apr 5, 2016 at 6:05 AM, Elia Pintowrote: > The correct api is trace_printf_key > > Signed-off-by: Elia Pinto > --- > diff --git a/Documentation/technical/api-trace.txt > b/Documentation/technical/api-trace.txt > @@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO); > static void trace_print_foo(const char *message) > { > - trace_print_key(_foo, message); > + trace_printf_key(_foo, message); > } Since you're touching this already, I wonder if it would make sense to rewrite this example to avoid the dangerous sending of an arbitrary string (which might contain %) to a printf-like function. Like this, for example: trace_printf_key(_foo, "%s", message); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: run arguments through precompose_argv
Alexander Rinasswrites: > File paths containing decomposed unicode chars passed to diff > command are not converted to precomposed unicode form. > > As a result, no diff is displayed when feeding such a file path to the > diff command. > > Opposite to most builtin commands, the diff builtin is missing the > parse_options call, which internally runs arguments through the > precompose_argv call, which ensures all arguments are in precomposed > unicode form. > > Fix the problem by adding a precompose_argv call directly, as a call to > parse_options would require additional work to call it. > > Also applies to diff-index, diff-files and diff-tree. Thanks. The log message talks about "such a file path", but precompose_argv() applies indiscriminately on any command line arguments, so things like -G would also get the same treatment, which I think is what most users would want). Will queue. > Signed-off-by: Alexander Rinass > Thanks-to: Torsten Bögershausen > Thanks-to: Junio C Hamano > --- > builtin/diff-files.c | 1 + > builtin/diff-index.c | 1 + > builtin/diff-tree.c | 2 ++ > builtin/diff.c | 1 + > t/t3910-mac-os-precompose.sh | 42 ++ > 5 files changed, 47 insertions(+) > > diff --git a/builtin/diff-files.c b/builtin/diff-files.c > index 8ed2eb8..15c61fd 100644 > --- a/builtin/diff-files.c > +++ b/builtin/diff-files.c > @@ -24,6 +24,7 @@ int cmd_diff_files(int argc, const char **argv, const char > *prefix) > gitmodules_config(); > git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ > rev.abbrev = 0; > + precompose_argv(argc, argv); > > argc = setup_revisions(argc, argv, , NULL); > while (1 < argc && argv[1][0] == '-') { > diff --git a/builtin/diff-index.c b/builtin/diff-index.c > index d979824..1af373d 100644 > --- a/builtin/diff-index.c > +++ b/builtin/diff-index.c > @@ -21,6 +21,7 @@ int cmd_diff_index(int argc, const char **argv, const char > *prefix) > gitmodules_config(); > git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ > rev.abbrev = 0; > + precompose_argv(argc, argv); > > argc = setup_revisions(argc, argv, , NULL); > for (i = 1; i < argc; i++) { > diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c > index 2a12b81..806dd7a 100644 > --- a/builtin/diff-tree.c > +++ b/builtin/diff-tree.c > @@ -114,6 +114,8 @@ int cmd_diff_tree(int argc, const char **argv, const char > *prefix) > opt->disable_stdin = 1; > memset(_r_opt, 0, sizeof(s_r_opt)); > s_r_opt.tweak = diff_tree_tweak_rev; > + > + precompose_argv(argc, argv); > argc = setup_revisions(argc, argv, opt, _r_opt); > > while (--argc > 0) { > diff --git a/builtin/diff.c b/builtin/diff.c > index 52c98a9..d6b8f98 100644 > --- a/builtin/diff.c > +++ b/builtin/diff.c > @@ -319,6 +319,7 @@ int cmd_diff(int argc, const char **argv, const char > *prefix) > if (!no_index) > gitmodules_config(); > git_config(git_diff_ui_config, NULL); > + precompose_argv(argc, argv); > > init_revisions(, prefix); > > diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh > index 8319356..26dd5b7 100755 > --- a/t/t3910-mac-os-precompose.sh > +++ b/t/t3910-mac-os-precompose.sh > @@ -49,12 +49,54 @@ test_expect_success "setup" ' > test_expect_success "setup case mac" ' > git checkout -b mac_os > ' > +# This will test nfd2nfc in git diff > +test_expect_success "git diff f.Adiar" ' > + touch f.$Adiarnfc && > + git add f.$Adiarnfc && > + echo f.Adiarnfc >f.$Adiarnfc && > + git diff f.$Adiarnfd >expect && > + git diff f.$Adiarnfc >actual && > + test_cmp expect actual && > + git reset HEAD f.Adiarnfc && > + rm f.$Adiarnfc expect actual > +' > +# This will test nfd2nfc in git diff-files > +test_expect_success "git diff-files f.Adiar" ' > + touch f.$Adiarnfc && > + git add f.$Adiarnfc && > + echo f.Adiarnfc >f.$Adiarnfc && > + git diff-files f.$Adiarnfd >expect && > + git diff-files f.$Adiarnfc >actual && > + test_cmp expect actual && > + git reset HEAD f.Adiarnfc && > + rm f.$Adiarnfc expect actual > +' > +# This will test nfd2nfc in git diff-index > +test_expect_success "git diff-index f.Adiar" ' > + touch f.$Adiarnfc && > + git add f.$Adiarnfc && > + echo f.Adiarnfc >f.$Adiarnfc && > + git diff-index HEAD f.$Adiarnfd >expect && > + git diff-index HEAD f.$Adiarnfc >actual && > + test_cmp expect actual && > + git reset HEAD f.Adiarnfc && > + rm f.$Adiarnfc expect actual > +' > # This will test nfd2nfc in readdir() > test_expect_success "add file Adiarnfc" ' > echo f.Adiarnfc >f.$Adiarnfc && > git add f.$Adiarnfc && > git commit -m "add f.$Adiarnfc" > ' > +# This will test nfd2nfc in git
Re: [PATCH v5 0/6] tag: move PGP verification code to tag.c
Sorry, forgot to add, this is a follow on to [1]. Thanks, -Santiago. [1]: http://git.661346.n2.nabble.com/PATCH-v4-0-6-tag-move-PGP-verification-code-to-tag-c-td7652451.html On Tue, Apr 05, 2016 at 12:07:23PM -0400, santi...@nyu.edu wrote: > From: Santiago Torres> > v5 (this): > Added helpful feedback by Eric > > * Reordering of the patches, to avoid temporal inclusion of a regression > * Fix typos here and there. > * Review commit messages, as some weren't representative of what the patches >were doing anymore. > * Updated t7030 to include Peff's suggestion, and added a helped-by line here >as it was mostly Peff's code. > * Updated the error-handling/printing issues that were introduced when. >libifying the verify_tag function. > > v4: > > Thanks Eric, Jeff, and Hannes for the feedback. > > * I relocated the sigchain_push call so it comes after the error on >gpg-interface (thanks Hannnes for catching this). > * I updated the unit test to match the discussion on [3]. Now it generates >the expected output of the tag on the fly for comparison. (This is just >copy and paste from [3], but I verified that it works by breaking the >while) > * I split moving the code and renaming the variables into two patches so >these are easier to review. > * I used an adapter on builtin/tag.c instead of redefining all the fn* >declarations everywhere. This introduces an issue with the way git tag -v >resolves refnames though. I added a new commit to restore the previous >behavior of git-tag. I'm not sure if I should've split this into two > commits >though. > > v3: > Thanks Eric, Jeff, for the feedback. > > * I separated the patch in multiple sub-patches. > * I compared the behavior of previous git tag -v and git verify-tag >invocations to make sure the behavior is the same > * I dropped the multi-line comment, as suggested. > * I fixed the issue with the missing brackets in the while (this is >now detected by the test). > > v2: > > * I moved the pgp-verification code to tag.c > * I added extra arguments so git tag -v and git verify-tag both work >with the same function > * Relocated the SIGPIPE handling code in verify-tag to gpg-interface > > v1: > > The verify tag function is just a thin wrapper around the verify-tag > command. We can avoid one fork call by doing the verification inside > the tag builtin instead. > > > This applies on v2.8.0. > > Thanks! > -Santiago > > > Santiago Torres (6): > builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface > t7030-verify-tag: Adds validation for multiple tags > builtin/verify-tag: change variable name for readability > builtin/verify-tag: replace name argument with sha1 > builtin/verify-tag: move verification code to tag.c > tag: use gpg_verify_function in tag -v call > > builtin/tag.c | 8 +-- > builtin/verify-tag.c | 65 > +-- > gpg-interface.c | 2 ++ > t/t7030-verify-tag.sh | 12 ++ > tag.c | 48 + > tag.h | 1 + > 6 files changed, 75 insertions(+), 61 deletions(-) > > -- > 2.8.0 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature request: config option for default git commit -v
W dniu 05.04.2016 o 16:47, Pranit Bauva pisze: > On Tue, Apr 5, 2016 at 8:08 PM, Jacek Wielemborekwrote: >> Hello, >> >> I'm asking for this one because there's quite a lot of interest >> (including me) in this feature and there is no convenient walkaround: >> >> https://stackoverflow.com/questions/5875275/git-commit-v-by-default >> >> Cheers, >> d33tah > > This is currently under progress. I am the one who is working on it. > One of the patches is currently on the pu branch. I am still polishing > it to include some more stuff. You can track its status by reading the > git.git messages by the git maintainer. The latest revision of the > patch is at http://thread.gmane.org/gmane.comp.version-control.git/288820 > > Thanks, > Pranit Bauva Awesome, thanks for the quick answer! I let the StackOverflow folks know. signature.asc Description: OpenPGP digital signature
[PATCH v5 0/6] tag: move PGP verification code to tag.c
From: Santiago Torresv5 (this): Added helpful feedback by Eric * Reordering of the patches, to avoid temporal inclusion of a regression * Fix typos here and there. * Review commit messages, as some weren't representative of what the patches were doing anymore. * Updated t7030 to include Peff's suggestion, and added a helped-by line here as it was mostly Peff's code. * Updated the error-handling/printing issues that were introduced when. libifying the verify_tag function. v4: Thanks Eric, Jeff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago Santiago Torres (6): builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface t7030-verify-tag: Adds validation for multiple tags builtin/verify-tag: change variable name for readability builtin/verify-tag: replace name argument with sha1 builtin/verify-tag: move verification code to tag.c tag: use gpg_verify_function in tag -v call builtin/tag.c | 8 +-- builtin/verify-tag.c | 65 +-- gpg-interface.c | 2 ++ t/t7030-verify-tag.sh | 12 ++ tag.c | 48 + tag.h | 1 + 6 files changed, 75 insertions(+), 61 deletions(-) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
From: Santiago TorresThe verify_signed_buffer comand might cause a SIGPIPE signal when the gpg child process terminates early (due to a bad keyid, for example) and git tries to write to it afterwards. Previously, ignoring SIGPIPE was done on the builtin/verify-tag.c command to avoid this issue. However, any other caller who wanted to use the verify_signed_buffer command would have to include this signal call. Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the verify_signed_buffer call (pretty much like in sign_buffer()) so that any caller is not required to perform this task. This will avoid possible mistakes by further developers using verify_signed_buffer. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 3 --- gpg-interface.c | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 00663f6..77f070a 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - /* sometimes the program was terminated because this signal -* was received in the process of writing the gpg input: */ - signal(SIGPIPE, SIG_IGN); while (i < argc) if (verify_tag(argv[i++], flags)) had_error = 1; diff --git a/gpg-interface.c b/gpg-interface.c index 3dc2fe3..2259938 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size, return error(_("could not run gpg.")); } + sigchain_push(SIGPIPE, SIG_IGN); write_in_full(gpg.in, payload, payload_size); close(gpg.in); @@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size, close(gpg.out); ret = finish_command(); + sigchain_pop(SIGPIPE); unlink_or_warn(path); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/6] t7030-verify-tag: Adds validation for multiple tags
From: Santiago TorresThe verify-tag command supports multiple tag names as an argument. However, existing tests only test for invocation with a single tag, so we add a test invoking with multiple tags. Helped-by: Jeff King Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 4608e71..c01621a 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' ' ) ' +test_expect_success GPG 'verify multiple tags' ' + tags="fourth-signed sixth-signed seventh-signed" && + for i in $tags; do + git verify-tag -v --raw $i || return 1 + done >expect.stdout 2>expect.stderr.1 && + grep "^.GNUPG" expect.stderr && + git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 && + grep "^.GNUPG" actual.stderr && + test_cmp expect.stdout actual.stdout && + test_cmp expect.stderr actual.stderr +' + test_done -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/6] builtin/verify-tag: move verification code to tag.c
From: Santiago TorresThe PGP verification routine for tags could be accessed by other commands that require it. We do this by moving it to the common tag.c module. We rename the verify_tag() function to gpg_verify_tag() to avoid conflicts with the mktag.c function. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 50 +- tag.c| 48 tag.h| 1 + 3 files changed, 50 insertions(+), 49 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 7a7c376..e9a2005 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,54 +18,6 @@ static const char * const verify_tag_usage[] = { NULL }; -static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) -{ - struct signature_check sigc; - int payload_size; - int ret; - - memset(, 0, sizeof(sigc)); - - payload_size = parse_signature(buf, size); - - if (size == payload_size) { - if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, payload_size); - return error("no signature found"); - } - - ret = check_signature(buf, payload_size, buf + payload_size, - size - payload_size, ); - print_signature_buffer(, flags); - - signature_check_clear(); - return ret; -} - -static int verify_tag(const unsigned char *sha1, unsigned flags) -{ - enum object_type type; - char *buf; - char *hex_sha1; - unsigned long size; - int ret; - - hex_sha1 = sha1_to_hex(sha1); - type = sha1_object_info(sha1, NULL); - if (type != OBJ_TAG) - return error("%s: cannot verify a non-tag object of type %s.", - hex_sha1, typename(type)); - - buf = read_sha1_file(sha1, , ); - if (!buf) - return error("%s: unable to read file.", hex_sha1); - - ret = run_gpg_verify(buf, size, flags); - - free(buf); - return ret; -} - static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -103,7 +55,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) had_error = 1; continue; } - if (verify_tag(sha1, flags)) + if (gpg_verify_tag(sha1, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index d72f742..3f7669f 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,54 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int payload_size; + int ret; + + memset(, 0, sizeof(sigc)); + + payload_size = parse_signature(buf, size); + + if (size == payload_size) { + if (flags & GPG_VERIFY_VERBOSE) + write_in_full(1, buf, payload_size); + return error("no signature found"); + } + + ret = check_signature(buf, payload_size, buf + payload_size, + size - payload_size, ); + print_signature_buffer(, flags); + + signature_check_clear(); + return ret; +} + +int gpg_verify_tag(const unsigned char *sha1, unsigned flags) +{ + enum object_type type; + char *buf; + char *hex_sha1; + unsigned long size; + int ret; + + hex_sha1 = sha1_to_hex(sha1); + type = sha1_object_info(sha1, NULL); + if (type != OBJ_TAG) + return error("%s: cannot verify a non-tag object of type %s.", + hex_sha1, typename(type)); + + buf = read_sha1_file(sha1, , ); + if (!buf) + return error("%s: unable to read file.", hex_sha1); + + ret = run_gpg_verify(buf, size, flags); + + free(buf); + return ret; +} + struct object *deref_tag(struct object *o, const char *warn, int warnlen) { while (o && o->type == OBJ_TAG) diff --git a/tag.h b/tag.h index f4580ae..cb643b9 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,6 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si extern int parse_tag(struct tag *item); extern struct object *deref_tag(struct object *, const char *, int); extern struct object *deref_tag_noverify(struct object *); +extern int gpg_verify_tag(const unsigned char *sha1, unsigned flags); #endif /* TAG_H */ -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/6] builtin/verify-tag: replace name argument with sha1
From: Santiago TorresThis change is meant to prepare verify_tag for libification. Many existing modules/commands already do the refname to sha1 resolution, so should avoid resolving the refname twice. To avoid breaking builtin/verify-tag, we move the refname resolution outside of the verify_tag() call. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 1ca9a05..7a7c376 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -42,25 +42,23 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -static int verify_tag(const char *name, unsigned flags) +static int verify_tag(const unsigned char *sha1, unsigned flags) { enum object_type type; - unsigned char sha1[20]; char *buf; + char *hex_sha1; unsigned long size; int ret; - if (get_sha1(name, sha1)) - return error("tag '%s' not found.", name); - + hex_sha1 = sha1_to_hex(sha1); type = sha1_object_info(sha1, NULL); if (type != OBJ_TAG) return error("%s: cannot verify a non-tag object of type %s.", - name, typename(type)); + hex_sha1, typename(type)); buf = read_sha1_file(sha1, , ); if (!buf) - return error("%s: unable to read file.", name); + return error("%s: unable to read file.", hex_sha1); ret = run_gpg_verify(buf, size, flags); @@ -80,6 +78,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(, N_("print tag contents")), OPT_BIT(0, "raw", , N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -96,8 +96,15 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - while (i < argc) - if (verify_tag(argv[i++], flags)) + while (i < argc) { + name = argv[i++]; + if (get_sha1(name, sha1)) { + error("tag '%s' not found.", name); had_error = 1; + continue; + } + if (verify_tag(sha1, flags)) + had_error = 1; + } return had_error; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/6] tag: use gpg_verify_function in tag -v call
From: Santiago TorresInstead of running the verify-tag plumbing command, we use the gpg_verify_tag() function within the verify_tag function to avoid doing an additional fork call. Signed-off-by: Santiago Torres --- builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..398c892 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref, static int verify_tag(const char *name, const char *ref, const unsigned char *sha1) { - const char *argv_verify_tag[] = {"verify-tag", - "-v", "SHA1_HEX", NULL}; - argv_verify_tag[2] = sha1_to_hex(sha1); - - if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD)) - return error(_("could not verify the tag '%s'"), name); - return 0; + return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/6] builtin/verify-tag: change variable name for readability
From: Santiago TorresThe run_gpg_verify function has two variables size, and len. This may come off as confusing when reading the code. We clarify which one pertains to the length of the tag headers by renaming len to payload_length. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 77f070a..1ca9a05 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = { static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) { struct signature_check sigc; - int len; + int payload_size; int ret; memset(, 0, sizeof(sigc)); - len = parse_signature(buf, size); + payload_size = parse_signature(buf, size); - if (size == len) { + if (size == payload_size) { if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); + write_in_full(1, buf, payload_size); return error("no signature found"); } - ret = check_signature(buf, len, buf + len, size - len, ); + ret = check_signature(buf, payload_size, buf + payload_size, + size - payload_size, ); print_signature_buffer(, flags); signature_check_clear(); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 5/5] commit: add a commit.verbose config variable
On Tue, Apr 5, 2016 at 4:59 AM, Eric Sunshinewrote: > On Sun, Apr 3, 2016 at 8:58 PM, Eric Sunshine wrote: >> The fact that the 32 new tests are nearly identical suggests strongly >> that the testing should instead either be table-driven or be done via >> for-loops to systematically cover all cases. Not only would either of >> these approaches be easier to digest, but they would make it easy to >> tell at a glance if any cases were missing. See [2] for an example of >> how the tests can be table-driven, and see the bottom of [3] for an >> example of using for-loops to test systematically (though you'd need >> to use nested for-loops rather than just the one in the example). >> >> I'm leaning toward systematic testing via nested for-loops as the more >> suitable of the two approach for this particular application. > I hadn't thought of this before. I also believe using for-loops will make it more clear, crisp and will avoid the effort of going through the whole patch to find out if some test is missing. > By the way, while this would be a nice change, it doesn't necessarily > have to be part of this series, and could be done as a follow-up by > you or someone else. > > (The other changes suggested in the same review, however, ought to be > fixed in this series; in particular, simplifying the "setup" test and > making the first test after "setup" consistent with the remaining > tests.) I will include it this series only as it will be a bit easier for me to keep a track of the previous reviews. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 4/5] t7507-commit-verbose: improve test coverage by testing number of diffs
On Mon, Apr 4, 2016 at 5:32 AM, Eric Sunshinewrote: > On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva wrote: >> Make the fake "editor" store output of grep in a file so that we can >> see how many diffs were contained in the message and use them in >> individual tests where ever it is required. Also use write_script() >> to create the fake "editor". >> >> A subsequent commit will introduce scenarios where it is important to be >> able to exactly determine how many diffs were present. > > These two sentences are backward. The "subsequent commit" bit is > justification for why you are making the "editor" store the output, > thus it belongs with the first paragraph. The bit about write_script() > is just a minor aside which can go in its own paragraph. > > I think it's also important to explain that you're changing the > behavior of write_script() so that it always succeeds, regardless of > whether grep found diff headers or not, and to give the reason for > making this change ("so that you don't have to use 'test_must_fail' > for cases when no diff headers are expected and can instead easily use > 'test_line_count = 0'"). > > The patch itself looks fine and is easy enough to read. I will include some more explanation as you suggest. > >> Helped-by: Eric Sunshine >> Signed-off-by: Pranit Bauva >> --- >> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh >> index 2ddf28c..0f28a86 100755 >> --- a/t/t7507-commit-verbose.sh >> +++ b/t/t7507-commit-verbose.sh >> @@ -3,11 +3,10 @@ >> test_description='verbose commit template' >> . ./test-lib.sh >> >> -cat >check-for-diff <> -#!$SHELL_PATH >> -exec grep '^diff --git' "\$1" >> +write_script "check-for-diff" <<\EOF && >> +grep '^diff --git' "$1" >out >> +exit 0 >> EOF >> -chmod +x check-for-diff >> test_set_editor "$PWD/check-for-diff" >> >> cat >message <<'EOF' >> @@ -23,7 +22,8 @@ test_expect_success 'setup' ' >> ' >> >> test_expect_success 'initial commit shows verbose diff' ' >> - git commit --amend -v >> + git commit --amend -v && >> + test_line_count = 1 out >> ' >> >> test_expect_success 'second commit' ' >> @@ -39,13 +39,15 @@ check_message() { >> >> test_expect_success 'verbose diff is stripped out' ' >> git commit --amend -v && >> - check_message message >> + check_message message && >> + test_line_count = 1 out >> ' >> >> test_expect_success 'verbose diff is stripped out (mnemonicprefix)' ' >> git config diff.mnemonicprefix true && >> git commit --amend -v && >> - check_message message >> + check_message message && >> + test_line_count = 1 out >> ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Timestamp of zero in reflog considered invalid
Erik Braywrites: > I tracked the issue to refs/files-backend.c in show_one_reflog_ent : > > https://github.com/git/git/blob/11529ecec914d2f0d7575e6d443c2d5a6ff75424/refs/files-backend.c#L2923 > > in which > > !(timestamp = strtoul(email_end + 2, , 10)) || > > implies an invalid reflog entry. Why should 0 be treated as an > invalid timestamp (even if it's unlikely outside of corner cases)? Thanks for a report. I think this dates back to 883d60fa (Sanitize for_each_reflog_ent(), 2007-01-08): commit 883d60fa97c6397450fb129634054e0a6101baac Author: Johannes Schindelin Date: Mon Jan 8 01:59:54 2007 +0100 Sanitize for_each_reflog_ent() It used to ignore the return value of the helper function; now, it expects it to return 0, and stops iteration upon non-zero return values; this value is then passed on as the return value of for_each_reflog_ent(). Further, it makes no sense to force the parsing upon the helper functions; for_each_reflog_ent() now calls the helper function with old and new sha1, the email, the timestamp & timezone, and the message. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano I do not see a corresponding "timestamp must be non-zero" check in the lines removed by that commit. I suspect that there was some confusion as to how strtoul() signals an error condition when the commit was written, or something. I do not think existing implementations of Git supports timestamps before the epoch (the timestamp on the common object headers are read into unsigned long variables and the code is not prepared to see anything negative there), but if anything the code should be detecting errors from strtoul() properly, i.e. if a timestamp is way long into the future and does not fit in "unsigned long", we should detect it. Checking the value against ULONG_MAX and errno==ERANGE would be an improvement. It may be debatable if we should silently ignore an entry with an invalid timestamp, but that is a separate issue. refs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 4e15f60..ff24184 100644 --- a/refs.c +++ b/refs.c @@ -3701,7 +3701,8 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' || !(email_end = strchr(sb->buf + 82, '>')) || email_end[1] != ' ' || - !(timestamp = strtoul(email_end + 2, , 10)) || + ((timestamp = strtoul(email_end + 2, , 10)) == ULONG_MAX && +errno == ERANGE) || !message || message[0] != ' ' || (message[1] != '+' && message[1] != '-') || !isdigit(message[2]) || !isdigit(message[3]) || -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 3/5] parse-options.c: make OPTION_COUNTUP respect "unspecified" values
On Mon, Apr 4, 2016 at 4:40 AM, Eric Sunshinewrote: > On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva wrote: >> The reason to make it respect "unspecified" values is to give the >> ability to differentiate whether `--option` or `--no-option` was >> specified at all. "unspecified" values should be in the form of negative >> values. If initial value is set to negative and `--option` specified >> then it will reflect the number of occurrences (counting done from 0), >> if `--no-option` is specified then it will reflect 0 and if nothing at >> all is given then it will retain its negative value. > > Thanks, this rewrite does a better job of explaining the aim of the > change and how a client can take advantage of it. However, with my > "first-time reader" hat on, I still have some trouble digesting it as > a coherent whole. I wonder if a rewrite like this would help? > > OPT_COUNTUP() merely increments the counter upon --option, and > resets it to 0 upon --no-option, which means that there is no > "unspecified" value with which a client can initialize the > counter to determine whether or not --[no-]option was seen at > all. > > Make OPT_COUNTUP() treat any negative number as an "unspecified" > value to address this shortcoming. In particular, if a client > initializes the counter to -1, then if it is still -1 after > parse_options(), then neither --option nor --no-option was seen; > if it is 0, then --no-option was seen last, and if it is 1 or > greater, than --option was seen last. > >> This change will not affect existing users of COUNTUP, because they all >> use the initial value of 0 (or more). > > "This change does not affect behavior of existing clients of..." > Sure I could do the change. >> Note that builtin/clean.c initializes the variable used with >> OPT__FORCE (which uses OPT_COUNTUP()) to a negative value, but it is set >> to either 0 or 1 by reading the configuration before the code calls >> parse_options(), i.e. as far as parse_options() is concerned, the >> initial value of the variable is not negative. >> >> To test this behavior "verbose" is set to "unspecified" while quiet is >> set to 0 which will test the new behavior with all sets of values. > > I think you need to mention here that you're talking about > test-parse-options.c > (and indirectly t0040) since it's otherwise too easy for the reader to > think that this paragraph is a continuation of the discussion about > OPT_COUNTUP()'s new behavior and how it won't impact existing tests, > rather than a new topic of its own (testing the behavior). Maybe say > something like this: > > To test the new behavior, augment the initial "verbose" setting > of test-parse-options.c to be unspecified... > > and then go on to say that, because "quiet" is still initialized to 0, > you have complete coverage. An alternative would be to split off the > new testing into its own patch, which would make this patch (which is > the real meat of the change) less noisy. I do like including test-parse-options.c in commit message. My main motive behind including the test with this patch was to show the "first-time" reader how to use the changes introduced in this patch. This would also set a complete picture of this commit. And I kind of believe it is much effort for the reader to find the commit whose parent will be this (if it exists at all, as the reader won't know about it) which will give a kind of demonstration to utilizing this change. > > I actually expected you to add new functionality to > test-parse-options.c specifically to test OPT_COUNTUP() directly > rather than indirectly through --verbose and --quiet, however, I think > I can be sold on the approach you took for a couple reasons. First, > you have a genuine use-case for an "unspecified" --verbose value, so > changing test-parse-options.c's --verbose to start at -1 tests what > you ultimately care about. Second, since you retained 0-initialization > of --quiet, that case of OPT_COUNTUP() is still being tested. > > What I find a bit disturbing about this approach is that you are > getting "full coverage" only because of current *implementation*, not > because you are explicitly testing for *expected* behavior. That is, > you get that coverage only while both OPT__VERBOSE() and OPT__QUIET() > are built atop OPT_COUNTUP(); if OPT__QUIET() is ever rewritten so it > no longer uses OPT_COUNTUP(), then the tests silently lose full > coverage. However, I may be over-worrying about the situation... The main reason as I mentioned above was to give a demonstration of how to utilize the change introduced. > >> Signed-off-by: Pranit Bauva >> --- >> diff --git a/Documentation/technical/api-parse-options.txt >> b/Documentation/technical/api-parse-options.txt >> @@ -144,8 +144,12 @@ There are some macros to easily define options: >> `OPT_COUNTUP(short, long, _var,
Re: [PATCH v12 2/5] test-parse-options: print quiet as integer
[+cc:Duy Nguyen, Jonathan Nieder] On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshinewrote: > On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva wrote: >> Current implementation of parse-options.c treats OPT__QUIET() as integer >> and not boolean and thus it is more appropriate to print it as integer >> to avoid confusion. > > I can buy this line of reasoning, however, it would be even easier to > sell the change if you cited an existing client (a git command) which > actually respects multiple quiet levels. Are there any? I investigated into this. But I was unable to find any git commit which actually respects mulitple quiet levels. I first did a 'git grep OPT__QUIET' to find the commands which use this. Then I went through the documentation which covers it. None of them have any such mention of multiple quiet levels. But still I dug further and and went through all the individual source files. I followed the corresponding C source code for the header file included and also searched there for any trace of quiet. But I still didn't find any such use of multiple quiet levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec, 2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to say as to why this was introduced and OPT__QUIET which previously used OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit In fact git-repack command has quiet but this is not mentioned in the documentation. If someone could include this it in the documentation. I would do it but I am not quite familiar with git-repack and haven't really used it anytime. > More importantly, though, this change implies that you should also add > tests to ensure that the quiet level is indeed incremented with each > --quiet, just as "-vv" and "--verbose --verbose" are already tested. > You might be able to include such new tests directly in this patch as > long as the commit message is clear about it, or add them in a > separate patch. I will include the tests for multiple level of quiet in the patch. > By the way, I don't see any tests to ensure that --no-verbose and > --no-quiet reset those respective values to 0. A separate patch which > adds such tests would be nice (unless such tests already exist and I > merely missed them). There are no tests to ensure that --no-verbose and --no-quiet reset those respective values to 0 just before this patch. But I have covered the --no-verbose tests in the upcoming patch 3/5. I will include the tests for --no-quiet in this patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature request: config option for default git commit -v
On Tue, Apr 5, 2016 at 8:08 PM, Jacek Wielemborekwrote: > Hello, > > I'm asking for this one because there's quite a lot of interest > (including me) in this feature and there is no convenient walkaround: > > https://stackoverflow.com/questions/5875275/git-commit-v-by-default > > Cheers, > d33tah This is currently under progress. I am the one who is working on it. One of the patches is currently on the pu branch. I am still polishing it to include some more stuff. You can track its status by reading the git.git messages by the git maintainer. The latest revision of the patch is at http://thread.gmane.org/gmane.comp.version-control.git/288820 Thanks, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Feature request: config option for default git commit -v
Hello, I'm asking for this one because there's quite a lot of interest (including me) in this feature and there is no convenient walkaround: https://stackoverflow.com/questions/5875275/git-commit-v-by-default Cheers, d33tah signature.asc Description: OpenPGP digital signature
Re: git diff --exit-code does not honour textconv setting
Georg Pichler venit, vidit, dixit 20.03.2016 13:43: > Hi, > > I realized that "git diff --exit-code" does not honour textconv settings. > Maybe this behaviour is desired. It can be partially circumvented by using > the "-b" flag if one does not care about whitespace changes. > To reproduce this, create an empty repository and run the following commands: > > (I was using git version 2.7.3) > > $ git config --add diff.void.textconv test > $ echo "foo diff=void" >.gitattributes > $ echo foo >foo > $ git add . && git commit -m "Init" > [master (root-commit) 70c39d9] Init > 2 files changed, 2 insertions(+) > create mode 100644 .gitattributes > create mode 100644 foo > $ echo bar >foo > $ git status > On branch master > Changes not staged for commit: > (use "git add ..." to update what will be committed) > (use "git checkout -- ..." to discard changes in working directory) > > modified: foo > > no changes added to commit (use "git add" and/or "git commit -a") > $ git diff > $ git diff --exit-code > [exits with 1, no output] > $ git diff --exit-code -b > [exits with 0, no output] > > The "test" command is used as it does not generate any output on stdout. "test" is a bit of a red herring here since it will receive commands. But your example works even with "true" which ignores its commands and produces no output. > I would expect "git diff --exit-code" to return with exit code 0. If this is > not desired, it should be clearly stated in the man page, > that "--exit-code" does not honour the textconv setting, except if "-b" is > given. Currently this is not clear: > >--exit-code >Make the program exit with codes similar to diff(1). That is, it > exits >with 1 if there were differences and 0 means no differences. > > Best, > Georg Pichler The description doesn't make it clear whether exit-code refers to the actual diff (foo vs. bar) or to the diff after textconv (empty vs. empty). In any case, "-b" should not make a difference for your example. diff_flush() in diff.c has this piece of code: /* * Report the content-level differences with HAS_CHANGES; * diff_addremove/diff_change does not set the bit when * DIFF_FROM_CONTENTS is in effect (e.g. with -w). */ if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) { if (options->found_changes) DIFF_OPT_SET(options, HAS_CHANGES); else DIFF_OPT_CLR(options, HAS_CHANGES); } So it's clear that depending on "-b" (or "-w") or not, it's taking shortcuts or looking at the textconved diff but I'm not sure where's the proper place to fix that. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git.spec: fix changelog dates
Michael J Gruberwrote: > Remi Galan Alfonso venit, vidit, dixit 05.04.2016 14:28: > > Michael J Gruber wrote: > >> A few changelog entries have inconsistent dates, which rpmlint reports > >> as errors. > >> > >> Fix them based on these assumptions: > >> - It's easier to mistype a number than a weekday abbreviation. > >> - changelog date must be before git commit date > >> - The mistyped date is just a few days off. > >> > >> Signed-off-by: Michael J Gruber > >> --- > >> I dunno if this is worthwhile, but rpmlint is the first thing we tell > >> packagers and reviewers to check. > >> > >> git.spec.in | 8 > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/git.spec.in b/git.spec.in > >> index bfd1cfb..eb581a3 100644 > >> --- a/git.spec.in > >> +++ b/git.spec.in > >> @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT > >> * Sat Jan 30 2010 Junio C Hamano > >> - We don't ship Python bits until a real foreign scm interface comes. > >> > >> -* Mon Feb 04 2009 David J. Mellor > >> +* Mon Feb 02 2009 David J. Mellor > >> - fixed broken git help -w after renaming the git-core package to git. > >> > >> * Fri Sep 12 2008 Quy Tonthat > >> @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT > >> * Thu Jun 21 2007 Shawn O. Pearce > >> - Added documentation files for git-gui > >> > >> -* Tue May 13 2007 Quy Tonthat > >> +* Sun May 13 2007 Quy Tonthat > > > > It is inconsistent with what you said in the commit message ("It's > > easier to mistype a number than a weekday abbreviation."). > > > > Following that logic, it should be: > > * Tue May 15 2007 Quy Tonthat > > > > (or 08, I didn't check the condition "changelog date must be before > > git commit date") > > I did. > > The thing is that you can't always fulfill all of these 3 conditions. > "Easier" doesn't mean that the other case is impossible, just less likely. Ah alright. Makes sense then, it is much harder to mistype 13 from 08 than from 15. Sorry for the noise. Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git.spec: fix changelog dates
Remi Galan Alfonso venit, vidit, dixit 05.04.2016 14:28: > Michael J Gruberwrote: >> A few changelog entries have inconsistent dates, which rpmlint reports >> as errors. >> >> Fix them based on these assumptions: >> - It's easier to mistype a number than a weekday abbreviation. >> - changelog date must be before git commit date >> - The mistyped date is just a few days off. >> >> Signed-off-by: Michael J Gruber >> --- >> I dunno if this is worthwhile, but rpmlint is the first thing we tell >> packagers and reviewers to check. >> >> git.spec.in | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/git.spec.in b/git.spec.in >> index bfd1cfb..eb581a3 100644 >> --- a/git.spec.in >> +++ b/git.spec.in >> @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT >> * Sat Jan 30 2010 Junio C Hamano >> - We don't ship Python bits until a real foreign scm interface comes. >> >> -* Mon Feb 04 2009 David J. Mellor >> +* Mon Feb 02 2009 David J. Mellor >> - fixed broken git help -w after renaming the git-core package to git. >> >> * Fri Sep 12 2008 Quy Tonthat >> @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT >> * Thu Jun 21 2007 Shawn O. Pearce >> - Added documentation files for git-gui >> >> -* Tue May 13 2007 Quy Tonthat >> +* Sun May 13 2007 Quy Tonthat > > It is inconsistent with what you said in the commit message ("It's > easier to mistype a number than a weekday abbreviation."). > > Following that logic, it should be: > * Tue May 15 2007 Quy Tonthat > > (or 08, I didn't check the condition "changelog date must be before > git commit date") I did. The thing is that you can't always fulfill all of these 3 conditions. "Easier" doesn't mean that the other case is impossible, just less likely. > Thanks, > Rémi > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: support block comments in gitconfig
Am 05.04.2016 um 04:19 schrieb Timothee Cour: > Could we have block comments in gitconfig? > It's a nice-to-have supported in most languages. > eg: > > #{ > commented out block > #} > > or other intuitive syntax Maybe not what you're looking for, but couldn't you use the include functionality of gitconfig: [include] path = ~/.another.gitconfig ?? That way you can comment out a lot of configuration with one '#'. Just my €0.02 Stefan -- /dev/random says: Air conditioned environment - Do not open Windows. python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
Re: [RFC/PATCH] git.spec: fix changelog dates
Michael J Gruberwrote: > A few changelog entries have inconsistent dates, which rpmlint reports > as errors. > > Fix them based on these assumptions: > - It's easier to mistype a number than a weekday abbreviation. > - changelog date must be before git commit date > - The mistyped date is just a few days off. > > Signed-off-by: Michael J Gruber > --- > I dunno if this is worthwhile, but rpmlint is the first thing we tell > packagers and reviewers to check. > > git.spec.in | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/git.spec.in b/git.spec.in > index bfd1cfb..eb581a3 100644 > --- a/git.spec.in > +++ b/git.spec.in > @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT > * Sat Jan 30 2010 Junio C Hamano > - We don't ship Python bits until a real foreign scm interface comes. > > -* Mon Feb 04 2009 David J. Mellor > +* Mon Feb 02 2009 David J. Mellor > - fixed broken git help -w after renaming the git-core package to git. > > * Fri Sep 12 2008 Quy Tonthat > @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT > * Thu Jun 21 2007 Shawn O. Pearce > - Added documentation files for git-gui > > -* Tue May 13 2007 Quy Tonthat > +* Sun May 13 2007 Quy Tonthat It is inconsistent with what you said in the commit message ("It's easier to mistype a number than a weekday abbreviation."). Following that logic, it should be: * Tue May 15 2007 Quy Tonthat (or 08, I didn't check the condition "changelog date must be before git commit date") Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] git.spec: fix changelog dates
A few changelog entries have inconsistent dates, which rpmlint reports as errors. Fix them based on these assumptions: - It's easier to mistype a number than a weekday abbreviation. - changelog date must be before git commit date - The mistyped date is just a few days off. Signed-off-by: Michael J Gruber--- I dunno if this is worthwhile, but rpmlint is the first thing we tell packagers and reviewers to check. git.spec.in | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git.spec.in b/git.spec.in index bfd1cfb..eb581a3 100644 --- a/git.spec.in +++ b/git.spec.in @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT * Sat Jan 30 2010 Junio C Hamano - We don't ship Python bits until a real foreign scm interface comes. -* Mon Feb 04 2009 David J. Mellor +* Mon Feb 02 2009 David J. Mellor - fixed broken git help -w after renaming the git-core package to git. * Fri Sep 12 2008 Quy Tonthat @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT * Thu Jun 21 2007 Shawn O. Pearce - Added documentation files for git-gui -* Tue May 13 2007 Quy Tonthat +* Sun May 13 2007 Quy Tonthat - Added lib files for git-gui - Added Documentation/technical (As needed by Git Users Manual) @@ -272,7 +272,7 @@ rm -rf $RPM_BUILD_ROOT * Tue Mar 27 2007 Eygene Ryabinkin - Added the git-p4 package: Perforce import stuff. -* Mon Feb 13 2007 Nicolas Pitre +* Mon Feb 12 2007 Nicolas Pitre - Update core package description (Git isn't as stupid as it used to be) * Mon Feb 12 2007 Junio C Hamano @@ -326,5 +326,5 @@ rm -rf $RPM_BUILD_ROOT * Thu Jul 14 2005 Eric Biederman - Add the man pages, and the --without docs build option -* Wed Jul 7 2005 Chris Wright +* Wed Jul 6 2005 Chris Wright - initial git spec file -- 2.8.1.120.g4a400c2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: support block comments in gitconfig
Hi Timothee, On Mon, 4 Apr 2016, Timothee Cour wrote: > Could we have block comments in gitconfig? > It's a nice-to-have supported in most languages. > eg: > > #{ > commented out block > #} > > or other intuitive syntax You could have such block comments. If you implemented that feature. Having said that, this syntax is distinctly *not* the INI syntax we tried to imitate. Plus, there are Git implementations *other* than core Git in the meantime, and they would need to be taught about this backwards-incompatible syntax, too. And then we still could not turn on that feature by default because there are setups out there where different users use different versions of Git. If you are prepared to push this feature forward, you can make it happen. In that case, you will need to spend a bit of effort and definitely exercise patience because you would likely get this feature accepted only into 2.9.0 (or 2.10.0 or 3.0.0...), i.e. a major version bump. Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Timestamp of zero in reflog considered invalid
Hi all, I found this issue through a test suite for a Python git interface, which during the tests at some point sets GIT_COMMITTER_DATE=1970-01-01T00:00:00 To reproduce the issue: $ git init $ echo foo > testfile $ git add testfile $ git commit -m "test" $ echo bar >> testfile $ export GIT_COMMITTER_DATE=1970-01-01T00:00:00 $ git stash save $ git stash apply refs/stash@{0} is not a valid reference At this point one can see: $ git rev-parse --symbolic --verify 'refs/stash@{0}' fatal: Log for refs/stash is empty. Expected: $ git rev-parse --symbolic --verify 'refs/stash@{0}' refs/stash@{0} I tracked the issue to refs/files-backend.c in show_one_reflog_ent : https://github.com/git/git/blob/11529ecec914d2f0d7575e6d443c2d5a6ff75424/refs/files-backend.c#L2923 in which !(timestamp = strtoul(email_end + 2, , 10)) || implies an invalid reflog entry. Why should 0 be treated as an invalid timestamp (even if it's unlikely outside of corner cases)? Thanks, Erik -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Triangular workflows and some anecdotes from the trenches
Hi, We ran into something at $dayjob the other day. The actual problem was a developer ended up amending a commit that had already been pushed. It happens occasionally and is usually recoverable with a simple rebase and is generally a learning experience. In this particular case however things were a bit more complicated. We had (attempted to) setup a triangular workflow. The developers would fetch from a server that was closer to them but push to the central server that was at the other end of a WAN link. Our build system would update the local server after a successful build for all configurations. The problem was instead of setting branch..pushdefault we were setting remote.origin.pushurl. So now the warning in git-remote(1) comes back to bite us and a lot of head scratching ensues. It appears that the triangular workflow support is under-documented (mentioned in a couple of release notes and gitrevisions). I'm not suggesting we would have done the right thing if the documentation existed but we would have had a chance. Once we get our setup sorted I'll try and send an update for gitworkflows.txt (unless someone else beats me to it). There are a few blog post around the Internet that I might be able to draw upon. The subject of preventing modifying published history has come up on this list before. And in-fact it's relatively simple to implement as an alias git config alias.amend '!git merge-base --is-ancestor HEAD @{upstream} || git commit --amend' I'm just wondering if something more official can be added to git commit --amend (and probably git rebase). Finally I was wondering if there is any way of detecting if remote.*.pushurl and remote.*.url point to the same repo, although I'm not sure when you'd do such verification. Thanks, Chris -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] completion: complete --cherry-mark for git log
Signed-off-by: Michael J Gruber--- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e3918c8..d87cf4d 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1458,6 +1458,7 @@ _git_log () --relative-date --date= --pretty= --format= --oneline --show-signature + --cherry-mark --cherry-pick --graph --decorate --decorate= -- 2.8.1.120.g4a400c2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.8.1 (and call-for-help to "make rpmbuild" users)
2016-04-03 21:21 GMT+02:00 Junio C Hamano: > If you do not build RPM binary packages from our pristine source, > you can safely ignore this release and stop reading this message. > > Now that the audience of this message has been limited to a narrow > target, before I make an announcement, here is a call-for-help to > you. > > Git v2.8 removed README file and added a corresponding README.md > file. The change however did not adjust git.spec.in that still > referred to README, causing "make rpmbuild" to fail. The breakage > was not noticed by anybody who tested v2.8.0-rc0 and later release > candidates, and ended up in the final v2.8 release, and we saw a > handful of bug reports on the list after the release happened. > > This maintenance release is to correct this bug for those who run > "make rpmbuild". It has no other changes. > > It is clear that nobody who relies on being able to "make rpmbuild" > ever tried any of the 5 release candidate snapshots that happened > during Feb 26-Mar 28. We had a whole month and nobody noticed? > > This incident clearly shows that something needs to happen, if > people want "make rpmbuild" to keep working. Even though this > maintenance release may fix this single bug, breakages similar to it > that only affect "make rpmbuild" users are guaranteed to appear in > future releases, unless those who can prevent them from happening > start helping to test at least release candidate snapshots. > > It is even more preferrable if they can test the tip of 'next' > branch regularly, in order to prevent such breakages from hitting > the 'master' branch to be included in the next release, which is > what the other parts of the system aims at. > > The other obvious option is for us to stop pretending that "make > rpmbuild" does anything useful to do and drop the build target and > the unmaintained git.spec.in file on which nobody in the active > development community keeps eyes. I do not mean this as a threat > "help us or else"; there is a precedent. We used to ship our own > debian/rules and friends for those who wanted to debbuild from the > source, but the Debian packagers wanted to have their own proper > ones and ours ended up confusing the users, and we made the world > a better place by removing our copy. If "make rpmbuild" people want > us to take this route, that is also OK for us. > > So that's the call for help. Now to the announcement. How old contributor to rpm5.org (as devzero2000) I have been following these issues in the past. Unfortunately, to my knowledge, no one has ever come to a convergence of views between the distros that use rpm on the various differences. Keeping for a project a spec file that then no distro uses (and i think also a local sysadmin) i do not think it is worth the effort. http://lists.rpm.org/pipermail/rpm-maint/2008-June/002185.html IMHO Best > > The latest maintenance release Git v2.8.1 is now available at > the usual places. > > The tarballs are found at: > > https://www.kernel.org/pub/software/scm/git/ > > The following public repositories all have a copy of the 'v2.8.1' > tag and the 'maint' branch that the tag points at: > > url = https://kernel.googlesource.com/pub/scm/git/git > url = git://repo.or.cz/alt-git.git > url = git://git.sourceforge.jp/gitroot/git-core/git.git > url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core > url = https://github.com/gitster/git > > > > Git v2.8.1 Release Notes > > > Fixes since v2.8 > > > * "make rpmbuild" target was broken as its input, git.spec.in, was >not updated to match a file it describes that has been renamed >recently. This has been fixed. > > > > Changes since v2.8.0 are as follows: > > Junio C Hamano (1): > Git 2.8.1 > > Matthieu Moy (1): > git.spec.in: use README.md, not README > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable
2016-04-01 22:25 GMT+02:00 Eric Sunshine: > On Fri, Apr 1, 2016 at 6:44 AM, Elia Pinto wrote: >> Implements the GIT_CURL_DEBUG environment variable to allow a greater >> degree of detail of GIT_CURL_VERBOSE, in particular the complete >> transport header and all the data payload exchanged. >> It might be useful if a particular situation could require a more >> thorough debugging analysis. > > In addition to review comments by others, why are the new curl_dump() > and curl_trace() functions duplicated in both patches rather than > factored out to a shared implementation? It's right. Do you think i can use some existing file or should I create a new object file ? Thank you very much > >> Signed-off-by: Elia Pinto >> --- >> diff --git a/imap-send.c b/imap-send.c >> @@ -1395,6 +1395,96 @@ static int append_msgs_to_imap(struct >> imap_server_conf *server, >> } >> >> #ifdef USE_CURL_FOR_IMAP_SEND >> + >> +static >> +void curl_dump(const char *text, >> + FILE * stream, unsigned char *ptr, size_t size, char nohex) >> +{ >> + size_t i; >> + size_t c; >> + >> + unsigned int width = 0x10; >> + >> + if (nohex) >> + /* without the hex output, we can fit more on screen */ >> + width = 0x40; >> + >> + fprintf(stream, "%s, %10.10ld bytes (0x%8.8lx)\n", >> + text, (long)size, (long)size); >> + >> + for (i = 0; i < size; i += width) { >> + >> + fprintf(stream, "%4.4lx: ", (long)i); >> + >> + if (!nohex) { >> + /* hex not disabled, show it */ >> + for (c = 0; c < width; c++) >> + if (i + c < size) >> + fprintf(stream, "%02x ", ptr[i + c]); >> + else >> + fputs(" ", stream); >> + } >> + >> + for (c = 0; (c < width) && (i + c < size); c++) { >> + /* check for 0D0A; if found, skip past and start a >> new line of output */ >> + if (nohex && (i + c + 1 < size) && ptr[i + c] == 0x0D >> + && ptr[i + c + 1] == 0x0A) { >> + i += (c + 2 - width); >> + break; >> + } >> + fprintf(stream, "%c", >> + (ptr[i + c] >= 0x20) >> + && (ptr[i + c] < 0x80) ? ptr[i + c] : '.'); >> + /* check again for 0D0A, to avoid an extra \n if >> it's at width */ >> + if (nohex && (i + c + 2 < size) >> + && ptr[i + c + 1] == 0x0D >> + && ptr[i + c + 2] == 0x0A) { >> + i += (c + 3 - width); >> + break; >> + } >> + } >> + fputc('\n', stream);/* newline */ >> + } >> + fflush(stream); >> +} >> + >> +static >> +int curl_trace(CURL * handle, curl_infotype type, >> +char *data, size_t size, void *userp) >> +{ >> + const char *text; >> + (void)handle; /* prevent compiler warning */ >> + >> + switch (type) { >> + case CURLINFO_TEXT: >> + fprintf(stderr, "== Info: %s", data); >> + default:/* in case a new one is introduced to shock >> us */ >> + return 0; >> + >> + case CURLINFO_HEADER_OUT: >> + text = "=> Send header"; >> + break; >> + case CURLINFO_DATA_OUT: >> + text = "=> Send data"; >> + break; >> + case CURLINFO_SSL_DATA_OUT: >> + text = "=> Send SSL data"; >> + break; >> + case CURLINFO_HEADER_IN: >> + text = "<= Recv header"; >> + break; >> + case CURLINFO_DATA_IN: >> + text = "<= Recv data"; >> + break; >> + case CURLINFO_SSL_DATA_IN: >> + text = "<= Recv SSL data"; >> + break; >> + } >> + >> + curl_dump(text, stderr, (unsigned char *)data, size, 1); >> + return 0; >> +} -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] api-trace.txt: fix typo
The correct api is trace_printf_key Signed-off-by: Elia Pinto--- Documentation/technical/api-trace.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/api-trace.txt b/Documentation/technical/api-trace.txt index 389ae16..45a0ecd 100644 --- a/Documentation/technical/api-trace.txt +++ b/Documentation/technical/api-trace.txt @@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO); static void trace_print_foo(const char *message) { - trace_print_key(_foo, message); + trace_printf_key(_foo, message); } + -- 2.8.0.270.g9d4de1f.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.8.1 (and call-for-help to "make rpmbuild" users)
Junio C Hamano venit, vidit, dixit 03.04.2016 21:21: > If you do not build RPM binary packages from our pristine source, > you can safely ignore this release and stop reading this message. > > Now that the audience of this message has been limited to a narrow > target, before I make an announcement, here is a call-for-help to > you. > > Git v2.8 removed README file and added a corresponding README.md > file. The change however did not adjust git.spec.in that still > referred to README, causing "make rpmbuild" to fail. The breakage > was not noticed by anybody who tested v2.8.0-rc0 and later release > candidates, and ended up in the final v2.8 release, and we saw a > handful of bug reports on the list after the release happened. > > This maintenance release is to correct this bug for those who run > "make rpmbuild". It has no other changes. > > It is clear that nobody who relies on being able to "make rpmbuild" > ever tried any of the 5 release candidate snapshots that happened > during Feb 26-Mar 28. We had a whole month and nobody noticed? > > This incident clearly shows that something needs to happen, if > people want "make rpmbuild" to keep working. Even though this > maintenance release may fix this single bug, breakages similar to it > that only affect "make rpmbuild" users are guaranteed to appear in > future releases, unless those who can prevent them from happening > start helping to test at least release candidate snapshots. In RedHat/Fedora land, we use our own spec files, "make" and "make install" into a special purpose tree and package that. E.g. in fedora, the packager noticed the README file name change in 2.8.0, adjusted the corresponding "%doc" line, and that was basically all besides updating the sources. As a fedora user, I either use rpms targetting fedora (possibly patched and rebuilt by myself), or I build from sources (--prefix=$HOME). With "generic" rpms you never know which guidelines they follow or where they install to. Also, naming schemes for dependencies may differ between RedHat land, SuSe land and other rpm based distros. So, just like in the deb case, I'm wondering if there's a use case for make rpmbuild in git.git. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] pretty: test --expand-tabs
Jeff Kingwrote: > > +test_expand --pretty=fuller > > +test_expand --pretty=fuller > > Duplicated fuller? Just brush it off :) [Those too young to get the joke can look up "fuller brush" in Wikipedia.] -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] format-patch: introduce --base=auto option
Ye Xiaolongwrites: > ---1---o---A > /\ / > ---O X > \/ \ > ---2---o---o---B > > For this criss-cross merges, as neither merge base(like 1) is better > than the other(both 1 and 2 are 'best' merge bases), I think it should > be fine to pick a random one as base commit(Or you prefer to show all of > them?) and I'll add this part of discusstion into documentation. I think you should error out; it would give you blatantly wrong to pick either one at random. Suppose A is where your remote-tracking branch is, and B is where the user started working on her serie. We are sending patches built on top of B (not depicted) with "format-patch --base=A B..". If you picked '1' as the base, you'll include commits on the === stretch as prerequisite patches (think of any non-merge --o-- in the picture to consist of multiple commits), but you won't be showing what the merge 'M' between '1' and '2' did to the tree of '1' to arrive at the resulting tree of 'M'. ---1---o---A /\ / ---O X \/ \ ---2---M===o===B...your.patches.here... If you picked '2' as the base, you'll include commits on the === stretch as prerequisite patches, but again you won't be showing what the merge 'M' between '1' and '2' did to the tree of '2' to arrive at the resulting tree of 'M'.. ---1---o---A /\ / ---O X \/ \ ---2===M===o===B...your.patches.here... So either case, you cannot rebuild the tree of B by going from the base and piling on patches in such a case, because you won't have "patch" for merge 'M'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Makefile: fix misdirected redirections
In general "echo 2>&1 $msg" to redirect a possible error message that comes from 'echo' itself into the same standard output stream $msg is getting written to does not make any sense; it is not like we are expecting to see any errors out of 'echo' in these statements, and even if it were the case, there is no reason to prevent the error messages from being sent to the standard error stream. These are clearly meant to send the argument given to echo to the standard error stream as error messages. Correctly redirect by saying "send what is written to the standard output to the standard error", i.e. "1>&2" aka ">&2". Signed-off-by: Junio C Hamano--- * There are two more uses of 2>&1 in the Makefile, and they are correct. - The one in dep_check one is used as dep_check = $(shell ... 2>&1; echo $$?) and it is an attempt to make sure that anything sent to the standard error is not leaked out of the shell substitution. It does want to send whatever is written to the standard error instead written to the standard output. - The other is "cmp ... >/dev/null 2>&1", which is a typical way to squelch both the standard output and the standard error for any command. Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8700db8..296f175 100644 --- a/Makefile +++ b/Makefile @@ -2211,10 +2211,10 @@ sparse: $(SP_OBJ) check: common-cmds.h @if sparse; \ then \ - echo 2>&1 "Use 'make sparse' instead"; \ + echo >&2 "Use 'make sparse' instead"; \ $(MAKE) --no-print-directory sparse; \ else \ - echo 2>&1 "Did you mean 'make test'?"; \ + echo >&2 "Did you mean 'make test'?"; \ exit 1; \ fi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Makefile: stop pretending to support rpmbuild
Dennis Kaarsemakerwrites: > On za, 2016-04-02 at 20:41 -0700, Junio C Hamano wrote: >> I think by now it is very clear that nobody active in the Git >> development community tests RPM binaries built with git.spec.in we >> have in our tree. I suspect RPM based distros are using their own >> RPM build recipe without paying any attention to what we have in our >> tree, and that is why no packager from RPM land gave any bug report >> and correction before the release happened. > > Fedora, RHEL, CentOS, OpenSUSE and Mageia all use their own specfiles > and local patches. I do test and distribute RPM packages based on the > next branch, but use fedora's packaging to do so (which incidentally > also broke and I had to work around this change, but I completely > forgot about git.spec.in). > >> I'd propose that during the cycle for the next feature release, we'd >> remove git.spec.in and stop pretending as if we support RPM >> packaging. > > I would be in favor of that. In general, I find it much better to use a > distro's packaging and simply transplanting a tarball into it. That > keeps the difference with what the distro provides minimal. OK, while we wait for other people to raise their opinions, here is to prepare for the removal, if we decide to follow through. -- >8 -- Nobody in the active development community seems to watch breakages in the rpmbuild target. As most major RPM based distros use their own specfile when packaging us, they aren't looking after us as their pristine upstream tree, either. At this point, it is turning to be a disservice to the users to pretend that our tree natively supports "make rpmbuild" target when we do not properly maintain it. Signed-off-by: Junio C Hamano --- Makefile| 17 +--- git.spec.in | 330 2 files changed, 5 insertions(+), 342 deletions(-) diff --git a/Makefile b/Makefile index 2742a69..23182bc 100644 --- a/Makefile +++ b/Makefile @@ -443,7 +443,6 @@ DIFF = diff TAR = tar FIND = find INSTALL = install -RPMBUILD = rpmbuild TCL_PATH = tclsh TCLTK_PATH = wish XGETTEXT = xgettext @@ -2396,31 +2395,25 @@ quick-install-html: ### Maintainer's dist rules -git.spec: git.spec.in GIT-VERSION-FILE - sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+ - mv $@+ $@ - GIT_TARNAME = git-$(GIT_VERSION) dist: git.spec git-archive$(X) configure ./git-archive --format=tar \ --prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar @mkdir -p $(GIT_TARNAME) - @cp git.spec configure $(GIT_TARNAME) + @cp configure $(GIT_TARNAME) @echo $(GIT_VERSION) > $(GIT_TARNAME)/version @$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version $(TAR) rf $(GIT_TARNAME).tar \ - $(GIT_TARNAME)/git.spec \ $(GIT_TARNAME)/configure \ $(GIT_TARNAME)/version \ $(GIT_TARNAME)/git-gui/version @$(RM) -r $(GIT_TARNAME) gzip -f -9 $(GIT_TARNAME).tar -rpm: dist - $(RPMBUILD) \ - --define "_source_filedigest_algorithm md5" \ - --define "_binary_filedigest_algorithm md5" \ - -ta $(GIT_TARNAME).tar.gz +rpm:: + @echo >&2 "Use distro packaged sources to run rpmbuild" + @false +.PHONY: rpm htmldocs = git-htmldocs-$(GIT_VERSION) manpages = git-manpages-$(GIT_VERSION) diff --git a/git.spec.in b/git.spec.in deleted file mode 100644 index d61d537..000 --- a/git.spec.in +++ /dev/null @@ -1,330 +0,0 @@ -# Pass --without docs to rpmbuild if you don't want the documentation - -Name: git -Version: @@VERSION@@ -Release: 1%{?dist} -Summary: Core git tools -License: GPL -Group: Development/Tools -URL: http://kernel.org/pub/software/scm/git/ -Source:http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz -BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3} -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) - -Requires: perl-Git = %{version}-%{release} -Requires: zlib >= 1.2, rsync, less, openssh-clients, expat -Provides: git-core = %{version}-%{release} -Obsoletes: git-core <= 1.5.4.2 -Obsoletes: git-p4 - -%description -Git is a fast, scalable, distributed revision control system with an -unusually rich command set that provides both high-level operations -and full access to internals. - -The git rpm installs the core tools with minimal dependencies. To -install all git packages, including tools for integrating with other -SCMs, install the git-all meta-package. - -%package all -Summary: Meta-package to pull in all git tools -Group: Development/Tools -Requires: git = %{version}-%{release} -Requires: git-svn = %{version}-%{release} -Requires:
Re: [PATCH v3 3/4] format-patch: introduce --base=auto option
On Fri, Apr 01, 2016 at 09:06:20AM -0700, Junio C Hamano wrote: >Ye Xiaolongwrites: > >> On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote: >>>Xiaolong Ye writes: >>> Introduce --base=auto to record the base commit info automatically, the base_commit will be the merge base of tip commit of the upstream branch and revision-range specified in cmdline. >>> >>>This line is probably a bit too long. >> >> How about simplifying it to "the base_commit is the merge base of upstream >> and >> specified revision-range."? > >What I meant was not that profound. I just wanted you to wrap your >lines a bit shorter so that quoting in the discussion thread like >this would not make the result overlong to fit on a 80-column >terminal ;-) Emm, get your point now, I'll shorten the lines to fit in 80-column. > + base = base_list->item; + free_commit_list(base_list); >>> >>>What should happen when there are multiple merge bases? The code >>>picks one at random and ignores the remainder, if I am reading this >>>correctly. >> >> If there is more than one merge base, commits in base_list should >> be sorted by date, if I am understanding it correctly, so >> base_list->item should be the lastest merge base commit, it should >> be enough for us to used as base commit. > >By definition, when there are multiple merge bases, there is no >latest one among them. > >When the history involves criss-cross merges, there can be more than >one 'best' common ancestor for two commits. For example, with this >topology (note that X is not a commit; it merely denotes crossing of >two lines): > > ---1---o---A > /\ / > ---O X > \/ \ > ---2---o---o---B > >both '1' and '2' are merge-bases of 'A' and 'B'. And the timestamps >on one (be it committer or author timestamp) being later than those >of the other do not make it any more suitable than the other one. > For this criss-cross merges, as neither merge base(like 1) is better than the other(both 1 and 2 are 'best' merge bases), I think it should be fine to pick a random one as base commit(Or you prefer to show all of them?) and I'll add this part of discusstion into documentation. Thanks, Xiaolong. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] pretty: test --expand-tabs
Jeff Kingwrites: > On Mon, Apr 04, 2016 at 05:58:37PM -0700, Junio C Hamano wrote: > >> +count_expand () >> +{ > > This function takes a lot of unnamed arguments that we process with > "shift". It might be nice to give a brief comment describing them. > ... >> +test_expand --pretty=fuller >> +test_expand --pretty=fuller > ... > Duplicated fuller? Thanks. Here is a replacement. -- >8 -- The test prepares a simple commit with HT on its log message lines, and makes sure that - formats that should or should not expand tabs by default do or do not expand tabs respectively, - with explicit --expand-tabs= and short-hands --expand-tabs (equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent to --expand-tabs=0) before or after the explicit --pretty=$fmt, the tabs are expanded (or not expanded) accordingly. The tests use the second line of the log message for formats other than --pretty=short, primarily because the first line of the email format is handled specially to add the [PATCH] prefix, etc. in a separate codepath (--pretty=short uses the first line because there is no other line to test). Signed-off-by: Junio C Hamano --- t/t4213-log-tabexpand.sh | 105 +++ 1 file changed, 105 insertions(+) create mode 100755 t/t4213-log-tabexpand.sh diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh new file mode 100755 index 000..e01a8f6 --- /dev/null +++ b/t/t4213-log-tabexpand.sh @@ -0,0 +1,105 @@ +#!/bin/sh + +test_description='log/show --expand-tabs' + +. ./test-lib.sh + +HT=" " +title='tab indent at the beginning of the title line' +body='tab indent on a line in the body' + +# usage: count_expand $indent $numSP $numHT @format_args +count_expand () +{ + expect= + count=$(( $1 + $2 )) ;# expected spaces + while test $count -gt 0 + do + expect="$expect " + count=$(( $count - 1 )) + done + shift 2 + count=$1 ;# expected tabs + while test $count -gt 0 + do + expect="$expect$HT" + count=$(( $count - 1 )) + done + shift + + # The remainder of the command line is "git show -s" options + case " $* " in + *' --pretty=short '*) + line=$title ;; + *) + line=$body ;; + esac + + # Prefix the output with the command line arguments, and + # replace SP with a dot both in the expecte and actual output + # so that test_cmp would show the differene together with the + # breakage in a way easier to consume by the debugging user. + { + echo "git show -s $*" + echo "$expect$line" + } | sed -e 's/ /./g' >expect + + { + echo "git show -s $*" + git show -s "$@" | + sed -n -e "/$line\$/p" + } | sed -e 's/ /./g' >actual + + test_cmp expect actual +} + +test_expand () +{ + fmt=$1 + case "$fmt" in + *=raw | *=short | *=email) + default="0 1" ;; + *) + default="8 0" ;; + esac + case "$fmt" in + *=email) + in=0 ;; + *) + in=4 ;; + esac + test_expect_success "expand/no-expand${fmt:+ for $fmt}" ' + count_expand $in $default $fmt && + count_expand $in 8 0 $fmt --expand-tabs && + count_expand $in 8 0 --expand-tabs $fmt && + count_expand $in 8 0 $fmt --expand-tabs=8 && + count_expand $in 8 0 --expand-tabs=8 $fmt && + count_expand $in 0 1 $fmt --no-expand-tabs && + count_expand $in 0 1 --no-expand-tabs $fmt && + count_expand $in 0 1 $fmt --expand-tabs=0 && + count_expand $in 0 1 --expand-tabs=0 $fmt && + count_expand $in 4 0 $fmt --expand-tabs=4 && + count_expand $in 4 0 --expand-tabs=4 $fmt + ' +} + +test_expect_success 'setup' ' + test_tick && + sed -e "s/Q/$HT/g" <<-EOF >msg && + Q$title + + Q$body + EOF + git commit --allow-empty -F msg +' + +test_expand "" +test_expand --pretty +test_expand --pretty=short +test_expand --pretty=medium +test_expand --pretty=full +test_expand --pretty=fuller +test_expand --pretty=raw +test_expand --pretty=email + +test_done -- 2.8.1-253-gd0f4798 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] pretty: test --expand-tabs
Jeff Kingwrites: > On Mon, Apr 04, 2016 at 09:10:46PM -0400, Eric Sunshine wrote: > >> > + count=$1 ;# expected tabs >> >> Why semicolon before the hash here and above? > > I am in the habit of doing this, too. I have a vague recollection of > getting bitten by a shell that treated: > > echo foo # bar > > or something similar as not-a-comment. But neither bash, dash, nor ksh > seem to. I think the reason why I started doing the same is because some shells can be configured to lose the comment-introducer-ness of "#" in interactive mode, and I wanted to make sure that many things I write can be tried out by others more easily by copy-and-paste to their interactive session. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: fix shortening of non-remote symrefs
Hello, On Sun, Apr 3, 2016 at 9:44 AM, Jeff Kingwrote: > On Sun, Apr 03, 2016 at 02:54:22PM +1200, Phil Sainty wrote: > >> Given the following symbolic reference: >> >> $ git symbolic-ref refs/heads/m refs/heads/master >> >> >> Correct in 2.6.6: >> >> $ PATH=~/git/git-2.6.6:$PATH git branch >> m -> master >> * master >> >> >> Wrong in 2.7.0: >> >> $ PATH=~/git/git-2.7.0:$PATH git branch >> m -> m >> * master Thanks for reporting this. > > Thanks for an easy test case. Though we don't officially support > arbitrary symrefs in the ref namespace, they do mostly work. And > certainly the current output is nonsense, and it worked before. This > bisects to aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23). > > The fix is below. Karthik, I didn't look at all how this interacts with > your work to convert branch to ref-filter for printing. I imagine it > drops this code completely, but we should make sure that ref-filter gets > this case right. I almost didn't prepare this patch at all, but I > suspect we may want it for "maint", while the full conversion would wait > for "master". > It's dropped in my latest series. I should be able to replicate what you've done here onto ref-filter.c. Since I'm re-rolling my patches, I'll add this one along too. > -- >8 -- > Subject: branch: fix shortening of non-remote symrefs > > Commit aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23) > adjusted the symref-printing code to look like this: > > if (item->symref) { > skip_prefix(item->symref, "refs/remotes/", ); > strbuf_addf(, " -> %s", desc); > } > > This has three bugs in it: > > 1. It always skips past "refs/remotes/", instead of > skipping past the prefix associated with the branch we > are showing (so commonly we see "refs/remotes/" for the > refs/remotes/origin/HEAD symref, but the previous code > would skip "refs/heads/" when showing a symref it found > in refs/heads/. > > 2. If skip_prefix() does not match, it leaves "desc" > untouched, and we show whatever happened to be in it > (which is the refname from a call to skip_prefix() > earlier in the function). > > 3. If we do match with skip_prefix(), we stomp on the > "desc" variable, which is later passed to > add_verbose_info(). We probably want to retain the > original refname there (though it likely doesn't matter > in practice, since after all, one points to the other). > > The fix to match the original code is fairly easy: record > the prefix to strip based on item->kind, and use it here. > However, since we already have a local variable named "prefix", > let's give the two prefixes verbose names so we don't > confuse them. > > Signed-off-by: Jeff King > --- > The test makes sure we restored the v2.6.x behavior, namely that > cross-prefix symrefs will not be shortened at all. It might be nice to > show: > > ref-to-remote -> remotes/origin/branch-one > > or something, but that should be separate from the fix (and I don't > overly care either way, so I probably won't work on it). > > builtin/branch.c | 19 --- > t/t3203-branch-output.sh | 12 > 2 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/builtin/branch.c b/builtin/branch.c > index 7b45b6b..f6c23bf 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -393,22 +393,25 @@ static void format_and_print_ref_item(struct > ref_array_item *item, int maxwidth, > int current = 0; > int color; > struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; > - const char *prefix = ""; > + const char *prefix_to_show = ""; > + const char *prefix_to_skip = NULL; > const char *desc = item->refname; > char *to_free = NULL; > > switch (item->kind) { > case FILTER_REFS_BRANCHES: > - skip_prefix(desc, "refs/heads/", ); > + prefix_to_skip = "refs/heads/"; > + skip_prefix(desc, prefix_to_skip, ); > if (!filter->detached && !strcmp(desc, head)) > current = 1; > else > color = BRANCH_COLOR_LOCAL; > break; > case FILTER_REFS_REMOTES: > - skip_prefix(desc, "refs/remotes/", ); > + prefix_to_skip = "refs/remotes/"; > + skip_prefix(desc, prefix_to_skip, ); > color = BRANCH_COLOR_REMOTE; > - prefix = remote_prefix; > + prefix_to_show = remote_prefix; > break; > case FILTER_REFS_DETACHED_HEAD: > desc = to_free = get_head_description(); > @@ -425,7 +428,7 @@ static void format_and_print_ref_item(struct > ref_array_item *item, int maxwidth, > color = BRANCH_COLOR_CURRENT; > } > > - strbuf_addf(, "%s%s", prefix, desc); > + strbuf_addf(, "%s%s", prefix_to_show, desc);