Re: [bug] assertion in 2.8.4 triggering on old-ish worktree
On Thu, Jun 16, 2016 at 4:59 PM, Chris Packham wrote: > Hi All, > > I have the git-sh-prompt configured in my .bashrc today I visited an > old worktree that I haven't really touched in a few years (sorry can't > remember the git version I was using back then). I received the > following output when changing to the directory > > git: pathspec.c:317: prefix_pathspec: Assertion `item->nowildcard_len > <= item->len && item->prefix <= item->len' failed. > > I assume it's one of the git invocations in git-sh-prompt that's > hitting the assertion. Any thoughts on what might be triggering it? > Any debug I can gather? A bit more info. The directory in question is a uninitialised submodule. It doesn't trigger in the root of the parent project. -- 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
[bug] assertion in 2.8.4 triggering on old-ish worktree
Hi All, I have the git-sh-prompt configured in my .bashrc today I visited an old worktree that I haven't really touched in a few years (sorry can't remember the git version I was using back then). I received the following output when changing to the directory git: pathspec.c:317: prefix_pathspec: Assertion `item->nowildcard_len <= item->len && item->prefix <= item->len' failed. I assume it's one of the git invocations in git-sh-prompt that's hitting the assertion. Any thoughts on what might be triggering it? Any debug I can gather? -- 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
[no subject]
w -- 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 2/2] archive-tar: write extended headers for far-future mtime
The ustar format represents timestamps as seconds since the epoch, but only has room to store 11 octal digits. To express anything larger, we need to use an extended header. This is exactly the same case we fixed for the size field in the previous commit, and the solution here follows the same pattern. This is even mentioned as an issue in f2f0267 (archive-tar: use xsnprintf for trivial formatting, 2015-09-24), but since it only affected things far in the future, it wasn't worth dealing with. But note that my calculations claiming thousands of years were off there; because our xsnprintf produces a NUL byte, we only have until the year 2242 to fix this. Given that this is just around the corner (geologically speaking), and because the fix for "size" makes doing this on top trivial, let's just make it work. Note that we don't (and never did) handle negative timestamps (i.e., before 1970). This would probably not be too hard to support in the same way, but since git does not support negative timestamps at all, I didn't bother here. Unlike the last patch for "size", this one is easy to test efficiently (the magic date is octal 010001): $ echo content >file $ git add file $ GIT_COMMITTER_DATE='@68719476737 +' \ git commit -q -m 'tempori parendum' $ git archive HEAD | tar tvf - -rw-rw-r-- root/root 8 4147-08-20 03:32 file With the current tip of master, this dies in xsnprintf (and prior to f2f0267, it overflowed into the checksum field, and the resulting tarfile claimed to be from the year 2242). However, I decided not to include this in the test suite, because I suspect it will create portability headaches, including: 1. The exact format of the system tar's "t" output. 2. System tars that cannot handle pax headers. 3. System tars tha cannot handle dates that far in the future. 4. If we replace "tar t" with "tar x", then filesystems that cannot handle dates that far in the future. In other words, we do not really have a reliable tar reader for these corner cases, so the best we could do is a byte-for-byte comparison of the output. Signed-off-by: Jeff King --- archive-tar.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/archive-tar.c b/archive-tar.c index 7340b64..749722f 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -185,6 +185,14 @@ static inline unsigned long ustar_size(uintmax_t size) return 0; } +static inline unsigned long ustar_mtime(time_t mtime) +{ + if (mtime < 0777) + return mtime; + else + return 0; +} + static void prepare_header(struct archiver_args *args, struct ustar_header *header, unsigned int mode, unsigned long size) @@ -192,7 +200,8 @@ static void prepare_header(struct archiver_args *args, xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0); xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? ustar_size(size) : 0); - xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned long) args->time); + xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", + ustar_mtime(args->time)); xsnprintf(header->uid, sizeof(header->uid), "%07o", 0); xsnprintf(header->gid, sizeof(header->gid), "%07o", 0); @@ -292,6 +301,8 @@ static int write_tar_entry(struct archiver_args *args, if (ustar_size(size) != size) strbuf_append_ext_header_uint(&ext_header, "size", size); + if (ustar_mtime(args->time) != args->time) + strbuf_append_ext_header_uint(&ext_header, "mtime", args->time); prepare_header(args, &header, mode, size); -- 2.9.0.150.g8bd4cf6 -- 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 1/2] archive-tar: write extended headers for file sizes >= 8GB
The ustar format has a fixed-length field for the size of each file entry which is supposed to contain up to 11 bytes of octal-formatted data plus a NUL or space terminator. These means that the largest size we can represent is 0777, or 1 byte short of 8GB. The correct solution for a larger file, according to POSIX.1-2001, is to add an extended pax header, similar to how we handle long filenames. This patch does that, and writes zero for the size field in the ustar header (the last bit is not mentioned by POSIX, but it matches how GNU tar behaves with --format=pax). This should be a strict improvement over the current behavior, which is to die in xsnprintf with a "BUG". However, there's some interesting history here. Prior to f2f0267 (archive-tar: use xsnprintf for trivial formatting, 2015-09-24), we silently overflowed the "size" field. The extra bytes ended up in the "mtime" field of the header, which was then immediately written itself, overwriting our extra bytes. What that means depends on how many bytes we wrote. If the size was 64GB or greater, then we actually overflowed digits into the mtime field, meaning our value was was effectively right-shifted by those lost octal digits. And this patch is again a strict improvement over that. But if the size was between 8GB and 64GB, then our 12-byte field held all of the actual digits, and only our NUL terminator overflowed. According to POSIX, there should be a NUL or space at the end of the field. However, GNU tar seems to be lenient here, and will correctly parse a size up 64GB (minus one) from the field. So sizes in this range might have just worked, depending on the implementation reading the tarfile. This patch is mostly still an improvement there, as the 8GB limit is specifically mentioned in POSIX as the correct limit. But it's possible that it could be a regression (versus the pre-f2f0267 state) if all of the following are true: 1. You have a file between 8GB and 64GB. 2. Your tar implementation _doesn't_ know about pax extended headers. 3. Your tar implementation _does_ parse 12-byte sizes from the ustar header without a delimiter. It's probably not worth worrying about such an obscure set of conditions, but I'm documenting it here just in case. There's no test included here. I did confirm that this works (using GNU tar) with: $ dd if=/dev/zero seek=64G bs=1 count=1 of=huge $ git add huge $ git commit -q -m foo $ git archive HEAD | head -c 1 | tar tvf - 2>/dev/null -rw-rw-r-- root/root 68719476737 2016-06-15 21:07 huge Pre-f2f0267, this would yield a bogus size of 8GB, and post-f2f0267, git-archive simply dies. Unfortunately, it's quite an expensive test to run. For one thing, unless your filesystem supports files with holes, it takes 64GB of disk space (you might think piping straight to `hash-object --stdin` would be better, but it's not; that tries to buffer all 64GB in RAM!). Furthermore, hashing and compressing the object takes several minutes of CPU time. We could ship just the resulting compressed object data as a loose object, but even that takes 64MB. So sadly, this code path remains untested in the test suite. Signed-off-by: Jeff King --- archive-tar.c | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/archive-tar.c b/archive-tar.c index cb99df2..7340b64 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -137,6 +137,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, strbuf_addch(sb, '\n'); } +/* + * Like strbuf_append_ext_header, but for numeric values. + */ +static void strbuf_append_ext_header_uint(struct strbuf *sb, + const char *keyword, + uintmax_t value) +{ + char buf[40]; /* big enough for 2^128 in decimal, plus NUL */ + int len; + + len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value); + strbuf_append_ext_header(sb, keyword, buf, len); +} + static unsigned int ustar_header_chksum(const struct ustar_header *header) { const unsigned char *p = (const unsigned char *)header; @@ -163,12 +177,21 @@ static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen) return i; } +static inline unsigned long ustar_size(uintmax_t size) +{ + if (size < 0777UL) + return size; + else + return 0; +} + static void prepare_header(struct archiver_args *args, struct ustar_header *header, unsigned int mode, unsigned long size) { xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0); - xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? size : 0); + xsnprintf(header->size, sizeof(header->size), "%011lo", + S_ISREG(mode) ? ustar_size(size) : 0); xsnprintf(header->mtime, sizeof(header->mtime), "
[PATCH 0/2] friendlier handling of overflows in archive-tar
The ustar format has some fixed-length numeric fields, and it's possible to generate a git tree that can't be represented (namely file size and mtime). Since f2f0267 (archive-tar: use xsnprintf for trivial formatting, 2015-09-24), we detect and die() in these cases. But we can actually do the friendly (and POSIX-approved) thing, and add extended pax headers to represent the correct values. [1/2]: archive-tar: write extended headers for file sizes >= 8GB [2/2]: archive-tar: write extended headers for far-future mtime -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2016, #04; Tue, 14)
On Wed, Jun 15, 2016 at 11:32:39AM -0700, Junio C Hamano wrote: > Mike Hommey writes: > > > On Tue, Jun 14, 2016 at 03:08:04PM -0700, Junio C Hamano wrote: > > >> * mh/connect (2016-06-06) 10 commits > >> - connect: [host:port] is legacy for ssh > >> ... > >> - connect: document why we sometimes call get_port after get_host_and_port > >> > >> Ok, folks, is everybody happy with this version? > > > > $gmane/296609 > > $gmane/296610 > > Oh, I have seen these, and I know you two are happy. > > But I am having a hard time coming up with a few-line summary for > this topic. I can write the beginning part, i.e. "Git-URL parsing > routine has been rewritten", but the concluding part of the sentence > cannot be "... has been rewritten for no good reason." if I were to > mark the topic as "Will merge to 'next'". The best I can come up > with is "... has been rewritten (hopefully) without changing the > benaviour.", but that is not a strong-enough justificaiton to make > the change to the codebase, either. > > In short, while the update may not introduce new bugs, why would we > want to have this change in the first place? My original motivation was to avoid having to copy code from connect.c into git-cinnabar, which is what I'm currently doing[1]. Things derailed a little, and we got ourselves somewhat in the middle of a refactor, that I'm willing to push a little further (like, refactor things such that host_end only happens once). My hope is that this makes the code more maintainable. Mike 1. https://github.com/glandium/git-cinnabar/blob/master/connect.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 16/38] resolve_gitlink_ref(): implement using resolve_ref_recursively()
On 06/14/2016 07:03 AM, Eric Sunshine wrote: > On Fri, Jun 3, 2016 at 5:03 PM, Michael Haggerty wrote: >> resolve_ref_recursively() can handle references in arbitrary files >> reference stores, so use it to resolve "gitlink" (i.e., submodule) >> references. Aside from removing redundant code, this allows submodule >> lookups to benefit from the much more robust code that we use for >> reading non-submodule references. And, since the code is now agnostic >> about reference backends, it will work for any future references >> backend (so move its definition to refs.c). >> >> Signed-off-by: Michael Haggerty >> --- >> diff --git a/refs.c b/refs.c >> @@ -1299,6 +1299,30 @@ const char *resolve_ref_unsafe(const char *refname, >> int resolve_flags, >> +int resolve_gitlink_ref(const char *path, const char *refname, unsigned >> char *sha1) >> +{ >> + int len = strlen(path); >> + struct strbuf submodule = STRBUF_INIT; >> + struct ref_store *refs; >> + int flags; >> + >> + while (len && path[len-1] == '/') >> + len--; >> + if (!len) >> + return -1; >> + >> + strbuf_add(&submodule, path, len); > > It took me a moment to figure out that you're using the strbuf only > for its side-effect of giving you a NUL-terminated string needed by > get_ref_store(), and not because you need any fancy functionality of > strbuf. I wonder if xstrndup() would have made this clearer. > > Not worth a re-roll. I agree both that xstrndup() (or, in this case, xmemdupz()) would have been clearer, and also that it's not worth a re-roll. I'll keep it in mind if I touch this code again. Thanks for your comment. 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: Easiest way to clone over an existing directory?
On Wed, Jun 15, 2016 at 08:51:34AM -0700, Josh Triplett wrote: > Currently, every time I set up a new system, I run the following: > > git clone $MY_HOMEDIR > mv home/.git . > rm -r home > git checkout -f > > This seems like an odd dance to go through. But I can't just git clone > into ~ directly, because git clone will not clone into an existing > non-empty directory. > > (I could use "git clone -n" to avoid the unnecessary checkout, but the > files are small, and it wouldn't remove the need to rmdir so the number > of commands would remain the same.) > > Does some better way exist to handle this? And if not, would it make > sense for git clone to have an option to clone into an existing > directory (which should also avoid setting junk_work_tree)? My typical technique is something like the following: git init git remote add origin https://git.crustytoothpaste.net/git/bmc/homedir.git git pull origin master I'm not sure if that's the officially sanctioned way to do it, but it does work reliably. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v4 6/6] send-email: add option --cite to quote the message body
Le 09/06/2016 à 13:49, Matthieu Moy a écrit : > Samuel GROOT writes: > >> If used with `in-reply-to=`, cite the message body of the given >> email file. Otherwise, do nothing. > > It should at least warn when --in-reply-to= is not given > (either no --in-reply-to or --in-reply-to=). I don't see any > use-case where a user would want --cite on the command-line and not want > --in-reply-to=. OTOH, it seems a plausible user-error, and > the user would appreciate a message saying what's going on. You're right. We'll warn the user with the next version. >> @@ -56,6 +57,8 @@ git send-email --dump-aliases >> --subject * Email "Subject:" >> --in-reply-to * Email "In-Reply-To:" >> --in-reply-to* Populate header fields appropriately. >> +--cite * Quote the message body in the cover if >> + --compose is set, else in the first >> patch. >> --[no-]xmailer * Add "X-Mailer:" header (default). >> --[no-]annotate* Review each patch that will be sent in >> an editor. >> --compose * Open an editor for introduction. > > Just wondering: would it make sense to activate --cite by default when > --in-reply-to=file is used, and to allow --no-cite to disable this? > This is something we can easily do now without breaking backward > compatibility (--in-reply-to=file doesn't exist yet), but would be more > painful to do later. Indeed, it can be more intuitive especially for a user who is used to cite messages. >> @@ -640,6 +644,7 @@ if (@files) { >> usage(); >> } >> >> +my $message_cited; > > Nit: I read "$message_cited" as "Boolean saying whether the message was > cited". $cited_message would be clearer to me (but this is to be taken > with a grain of salt as I'm not a native speaker), since the variable > holds the content of the cited message. Sorry, French habits.. :-) >> +sub do_insert_cited_message { >> +my $tmp_file = shift; >> +my $original_file = shift; >> + >> +open my $c, "<", $original_file >> +or die "Failed to open $original_file: " . $!; >> + >> +open my $c2, ">", $tmp_file >> +or die "Failed to open $tmp_file: " . $!; >> + >> +# Insertion after the triple-dash >> +while (<$c>) { >> +print $c2 $_; >> +last if (/^---$/); >> +} >> +print $c2 $message_cited; > > I would add a newline here to get a blank line between the message cited > and the diffstat. Agreed. > I think non-ascii characters would deserve particular attention here > too. For example, if the patch contain only ascii and the cited part > contains UTF-8, does the generated patch have a proper Content-type: > header? > > I can imagine worse, like a patch containing latin1 character and a > cited message with another 8-bit encoding. I tried to manage them with the built-in Base64 module but there is still work in progress. >> +test_expect_success $PREREQ 'correct cited message with --in-reply-to and >> --compose' ' >> +grep "> On Sat, 12 Jun 2010 15:53:58 +0200, aut...@example.com wrote:" >> msgtxt3 && > > I would prefer to have the full address including the real name here (A > ) in this example. Actually, after a quick look at > the code, I don't understand where the name has gone (what's shown here > is extracted from the From: header). Agreed, I'll figure out where the problem is. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"git-rebase -i --autostash" will leave dangling stash when editor is aborted
TEST CASE: 1. Modify a file that need to get stashed on rebasing 2. Run "EDITOR=vim git rebase -i --autostash" 3. Abort with ":cq", which will make Vim exit non-zero Git then will create an autostash, but aborts with "Could not execute editor", via the following code in git-rebase--interactive.sh, and does not restore the autostash - it does not show up with the regular stashes, like when there is a conflict during the rebase: git_sequence_editor "$todo" || die_abort "Could not execute editor" Step 2 and 3 and probably merged into a single one for a test case, but that's how I keep triggering it. Instead of adding it to the list of stashes, it should probably get restored/re-applied right away, since no rebase actions have been triggered. Please CC me in replies. Cheers, Daniel. -- 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] verify-tag: allow to verify signed blob objects
On Wed, Jun 15, 2016 at 12:24 PM, Junio C Hamano wrote: > Michael J Gruber writes: > >>> Or even >>> >>> if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB)) >>> "you told me to check blob but didn't give me one"; >>> } else if (type != OBJ_TAG) >>> "you didn't give me a tag"; >>> >> >> I just tried to stay as close to the original as possible, but I don't >> care either way. Your latter version is more strict and would require a >> slight documentation change, but would be fine, too. > > Actually, the message you reused is not reusable for this new mode. > I guess starting from more strict (which makes sense, as you do not > want to silently say "Yeah, the blob verifies OK" when the user > tells you "I want you to verify this blob, and here it is" and hands > you a tag. If that were an acceptable behaviour, you do not even > need VERIFY_BLOB as an option, do you? > > So I do not care too strongly about this feature, if it were to be > added, I think you would need to separate error messages and type > verification should not be lax, I would think. > I agree that Junio's suggestion is (a) both easier to read and (b) more clear to the end user, and thus preferable. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: > Reimplement `is_expected_rev` & `check_expected_revs` shell function in > C and add a `--check-expected-revs` subcommand to `git bisect--helper` to > call it from git-bisect.sh . > [...] > Signed-off-by: Pranit Bauva > --- > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > @@ -162,13 +162,44 @@ static int bisect_reset(const char *commit) > +static int is_expected_rev(const char *expected_hex) > +{ > + struct strbuf actual_hex = STRBUF_INIT; > + int res; > + > + if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) > < 0) { > + strbuf_release(&actual_hex); > + return 0; > + } > + > + strbuf_trim(&actual_hex); > + res = !strcmp(actual_hex.buf, expected_hex); > + strbuf_release(&actual_hex); > + return res; > +} Not worth a re-roll, but this could be re-structured to avoid having to remember to release the strbuf at all exits: struct strbuf actual_hex = ...; int res = 0; if (strbuf_read_file(...) >= 0) { strbuf_trim(...); res = !strcmp(...); } strbuf_release(...); return res; Alternately: if (strbuf_read_file(...) < 0) goto done; strbuf_trim(...); res = !strcmp(...); done: strbuf_release(...); return res; which is a bit less compact. > +static int check_expected_revs(const char **revs, int rev_nr) > +{ > + int i; > + > + for (i = 0; i < rev_nr; i++) { > + if (!is_expected_rev(revs[i])) { > + remove_path(git_path_bisect_ancestors_ok()); > + remove_path(git_path_bisect_expected_rev()); > + return 0; > + } > + } > + return 0; > +} Hmm, all execution paths return 0, so it feels a bit pointless to have this function return a value at all. You could also use a 'break' inside the loop rather than 'return' since the return value is the same inside or outside the loop and nothing else happens after the loop. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] bisect--helper: `bisect_reset` shell function in C
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: > Reimplement `bisect_reset` shell function in C and add a `--bisect-reset` > subcommand to `git bisect--helper` to call it from git-bisect.sh . > [...] > Signed-off-by: Pranit Bauva > --- > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > @@ -123,12 +127,48 @@ static int bisect_clean_state(void) > +static int bisect_reset(const char *commit) > +{ > + struct strbuf branch = STRBUF_INIT; > + > + if (!commit) { > + if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < > 1) { > + printf("We are not bisecting.\n"); > + return 0; > + } > + strbuf_rtrim(&branch); > + Style: unnecessary blank line > + } else { > + struct object_id oid; > + if (get_oid(commit, &oid)) > + return error(_("'%s' is not a valid commit"), commit); > + strbuf_addf(&branch, "%s", commit); strbuf_addstr(&branch, commit); > + } > + > + if (!file_exists(git_path_bisect_head())) { > + struct argv_array argv = ARGV_ARRAY_INIT; > + argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL); > + if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { > + error(_("Could not check out original HEAD '%s'. Try" > + "'git bisect reset '."), > branch.buf); > + strbuf_release(&branch); > + argv_array_clear(&argv); > + return -1; > + } > + argv_array_clear(&argv); > + } > + > + strbuf_release(&branch); > + return bisect_clean_state(); > +} -- 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] Document the 'svn propset' command.
> On Jun 15, 2016, at 1:24 PM, Alfred Perlstein wrote: > >> On Jun 15, 2016, at 1:21 PM, Junio C Hamano wrote: >> >> Eric Wong writes: >> >>> Thanks Alfred, >>> >>> I've removed the '.' from the commit subject, signed-off, >>> and pushed to my repo for Junio: >>> >>> The following changes since commit 05219a1276341e72d8082d76b7f5ed394b7437a4: >>> >>> Git 2.9 (2016-06-13 10:42:13 -0700) >>> >>> are available in the git repository at: >>> >>> git://bogomips.org/git-svn.git svn-propset-doc >>> >>> for you to fetch changes up to f3961b2eba8ba6aa2fddc827ddf5c26b41391872: >>> >>> Document the 'svn propset' command (2016-06-15 20:11:22 +) >> >> I actually queued it directly on top of v2.3.0-rc0~32^2 (git-svn: >> support for git-svn propset, 2014-12-07) so that it could go to >> older maintenance tracks. >> >> I will pick up your Reviewed-by: and redo it. Thanks. >> > > Thank you, always great working with the git project! > > -Alfred Thanks for addressing this! - Joe -- 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] Document the 'svn propset' command.
> On Jun 15, 2016, at 1:21 PM, Junio C Hamano wrote: > > Eric Wong writes: > >> Thanks Alfred, >> >> I've removed the '.' from the commit subject, signed-off, >> and pushed to my repo for Junio: >> >> The following changes since commit 05219a1276341e72d8082d76b7f5ed394b7437a4: >> >> Git 2.9 (2016-06-13 10:42:13 -0700) >> >> are available in the git repository at: >> >> git://bogomips.org/git-svn.git svn-propset-doc >> >> for you to fetch changes up to f3961b2eba8ba6aa2fddc827ddf5c26b41391872: >> >> Document the 'svn propset' command (2016-06-15 20:11:22 +) > > I actually queued it directly on top of v2.3.0-rc0~32^2 (git-svn: > support for git-svn propset, 2014-12-07) so that it could go to > older maintenance tracks. > > I will pick up your Reviewed-by: and redo it. Thanks. > Thank you, always great working with the git project! -Alfred -- 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] Document the 'svn propset' command.
Alfred Perlstein wrote: > Add example usage to the git-svn documentation. > > Reported-by: Joseph Pecoraro > Signed-off-by: Alfred Perlstein > --- > > Junio, Pranit, + all, > > A week ago I was requested to provide documentation for the > 'svn propset' command. I have attached a diff off of the > 'maint' branch for this, however it seems to apply cleanly > to 'master' as well. > > Thank you for your patience. Thanks Alfred, I've removed the '.' from the commit subject, signed-off, and pushed to my repo for Junio: The following changes since commit 05219a1276341e72d8082d76b7f5ed394b7437a4: Git 2.9 (2016-06-13 10:42:13 -0700) are available in the git repository at: git://bogomips.org/git-svn.git svn-propset-doc for you to fetch changes up to f3961b2eba8ba6aa2fddc827ddf5c26b41391872: Document the 'svn propset' command (2016-06-15 20:11:22 +) Alfred Perlstein (1): Document the 'svn propset' command Documentation/git-svn.txt | 14 ++ 1 file changed, 14 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] bisect--helper: `bisect_clean_state` shell function in C
On Wed, Jun 15, 2016 at 2:47 PM, Pranit Bauva wrote: > On Wed, Jun 15, 2016 at 11:34 PM, Eric Sunshine > wrote: >> On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva >> wrote: >>> Reimplement `bisect_clean_state` shell function in C and add a >>> `bisect-clean-state` subcommand to `git bisect--helper` to call it from >>> git-bisect.sh . >>> [...] >>> Signed-off-by: Pranit Bauva >>> --- >>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c >>> +static int mark_for_removal(const char *refname, const struct object_id >>> *oid, >>> + int flag, void *cb_data) >>> +{ >>> + struct string_list *refs = cb_data; >>> + char *ref = xstrfmt("refs/bisect/%s", refname); >>> + string_list_append(refs, ref); >>> + return 0; >>> +} >>> + >>> +static int bisect_clean_state(void) >>> +{ >>> + int result = 0; >>> + >>> + /* There may be some refs packed during bisection */ >>> + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP; >>> + for_each_ref_in("refs/bisect/", mark_for_removal, (void *) >>> &refs_for_removal); >>> + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD")); >>> + result = delete_refs(&refs_for_removal); >>> + string_list_clear(&refs_for_removal, 0); >> >> This is leaking all the strings added to 'refs_for_removal', isn't it? >> Either you need to loop over the items and free the strings manually, >> or (if it's not too ugly), set 'strdup_strings' before invoking >> string_list_clear(). > > I didn't carefully see that in the function string_list_clear() it > only free()'s the memory if strdup_strings is 1. I think changing > strdup_strings to 1 would be an easy way out but it would make the > code very ugly and non-trivial. I disagree about it making the code "*very* ugly and non-trivial". It is quite trivial. What I meant by "ugly" was that it may be too intimate with the implementation of string_list. However, since the solution is already used in the codebase, it may be acceptable. For instance, in builtin/fetch.c: /* All names were strdup()ed or strndup()ed */ list.strdup_strings = 1; string_list_clear(&list, 0); which is exactly the approach I was suggesting. You'll find the same pattern in builtin/shortlog.c. > On the other hand, I can initialize > the string as STRING_LIST_INIT_DUP which will automatically set > strdup_strings as 1 and then also free the memory of ref at that point > after the string ref was appended to the list. Personally, I will > prefer the latter one. Meh. -- 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] Document the 'svn propset' command.
Eric Wong writes: > Thanks Alfred, > > I've removed the '.' from the commit subject, signed-off, > and pushed to my repo for Junio: > > The following changes since commit 05219a1276341e72d8082d76b7f5ed394b7437a4: > > Git 2.9 (2016-06-13 10:42:13 -0700) > > are available in the git repository at: > > git://bogomips.org/git-svn.git svn-propset-doc > > for you to fetch changes up to f3961b2eba8ba6aa2fddc827ddf5c26b41391872: > > Document the 'svn propset' command (2016-06-15 20:11:22 +) I actually queued it directly on top of v2.3.0-rc0~32^2 (git-svn: support for git-svn propset, 2014-12-07) so that it could go to older maintenance tracks. I will pick up your Reviewed-by: and redo it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] push: deny policy to prevent pushes to unwanted remotes.
>> ... >> > > Hello Rémi, thanks you for your input ! I'll make the appropriate changes > and send a new version as soon as i can ! Hi Antoine, do you have an updated version already or is this the one I should look at? http://article.gmane.org/gmane.comp.version-control.git/296445 Thanks, Lars-- 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] push: deny policy to prevent pushes to unwanted remotes.
On 06 Jun 2016, at 16:00, Antoine Queru wrote: > Hello Lars, thanks for your reply. >> >> >>> On 30 May 2016, at 06:45, Antoine Queru >>> wrote: >>> >>> Currently, a user wanting to prevent accidental pushes to the wrong remote >>> has to create a pre-push hook. >>> The feature offers a configuration to allow users to prevent accidental >>> pushes >>> to the wrong remote. The user may define a list of whitelisted remotes, a >>> list >>> of blacklisted remotes and a default policy ("allow" or "deny"). A push >>> is denied if the remote is explicitely blacklisted or if it isn't >>> whitelisted and the default policy is "deny". >>> >>> This feature is intended as a safety net more than a real security, the >>> user >>> will always be able to modify the config if he wants to. It is here for him >>> to >>> consciously restrict his push possibilities. For example, it may be useful >>> for an unexperimented user fearing to push to the wrong remote, or for >>> companies wanting to avoid unintentionnal leaking of private code on public >>> repositories. >> >> Thanks for working on this feature. Unfortunately I won't be able to test and >> review it before June 14. I am traveling without laptop and only very >> sporadic internet access :) >> >> One thing that I noticed already: I think a custom warning/error message for >> rejected pushes would be important because, as you wrote above, this feature >> does not provide real security. That means if a push is rejected for someone >> in an organization then the user needs to understand what is going on. E.g. >> in my organization I would point the user to the open source contribution >> guidelines etc. >> >> Thanks, >> Lars > > I might not understand what you've said, but I think this feature is already > implemented in our version, with remote.pushDenyMessage. Is this what you're > talking about ? You are right. I was skimming the diff on a very small screen and missed that for some reason. Sorry! -- 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] verify-tag: allow to verify signed blob objects
Michael J Gruber writes: >> Or even >> >> if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB)) >> "you told me to check blob but didn't give me one"; >> } else if (type != OBJ_TAG) >> "you didn't give me a tag"; >> > > I just tried to stay as close to the original as possible, but I don't > care either way. Your latter version is more strict and would require a > slight documentation change, but would be fine, too. Actually, the message you reused is not reusable for this new mode. I guess starting from more strict (which makes sense, as you do not want to silently say "Yeah, the blob verifies OK" when the user tells you "I want you to verify this blob, and here it is" and hands you a tag. If that were an acceptable behaviour, you do not even need VERIFY_BLOB as an option, do you? So I do not care too strongly about this feature, if it were to be added, I think you would need to separate error messages and type verification should not be lax, I would think. -- 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] verify-tag: allow to verify signed blob objects
Junio C Hamano venit, vidit, dixit 15.06.2016 20:39: > Michael J Gruber writes: > >> diff --git a/tag.c b/tag.c >> index d1dcd18..d5f090b 100644 >> --- a/tag.c >> +++ b/tag.c >> @@ -39,7 +39,7 @@ int gpg_verify_tag(const unsigned char *sha1, const char >> *name_to_report, >> int ret; >> >> type = sha1_object_info(sha1, NULL); >> -if (type != OBJ_TAG) >> +if ((type != OBJ_TAG) && ((type != OBJ_BLOB) || !(flags & >> GPG_VERIFY_BLOB))) >> return error("%s: cannot verify a non-tag object of type %s.", >> name_to_report ? >> name_to_report : > > The double negation is very hard to read. I wonder > > if ((type != OBJ_TAG) && > !((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB))) > > is easier to follow? "It is not a tag object, and it's not like we > were asked to verify blob and the user gave us a blob, either, so > let's complain" is easier to follow, at least for me. As a further exercise in boolean algebra, you can pull out the negation completely: if (!( (type == OBJ_TAG) || ((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB)) )) > Or even > > if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB)) > "you told me to check blob but didn't give me one"; > } else if (type != OBJ_TAG) > "you didn't give me a tag"; > I just tried to stay as close to the original as possible, but I don't care either way. Your latter version is more strict and would require a slight documentation change, but would be fine, too. 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 v2 1/6] bisect--helper: `bisect_clean_state` shell function in C
Hey Eric, On Wed, Jun 15, 2016 at 11:34 PM, Eric Sunshine wrote: > On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: >> Reimplement `bisect_clean_state` shell function in C and add a >> `bisect-clean-state` subcommand to `git bisect--helper` to call it from >> git-bisect.sh . >> [...] >> Signed-off-by: Pranit Bauva >> --- >> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c >> +static int mark_for_removal(const char *refname, const struct object_id >> *oid, >> + int flag, void *cb_data) >> +{ >> + struct string_list *refs = cb_data; >> + char *ref = xstrfmt("refs/bisect/%s", refname); >> + string_list_append(refs, ref); >> + return 0; >> +} >> + >> +static int bisect_clean_state(void) >> +{ >> + int result = 0; >> + >> + /* There may be some refs packed during bisection */ >> + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP; >> + for_each_ref_in("refs/bisect/", mark_for_removal, (void *) >> &refs_for_removal); >> + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD")); >> + result = delete_refs(&refs_for_removal); >> + string_list_clear(&refs_for_removal, 0); > > This is leaking all the strings added to 'refs_for_removal', isn't it? > Either you need to loop over the items and free the strings manually, > or (if it's not too ugly), set 'strdup_strings' before invoking > string_list_clear(). I didn't carefully see that in the function string_list_clear() it only free()'s the memory if strdup_strings is 1. I think changing strdup_strings to 1 would be an easy way out but it would make the code very ugly and non-trivial. On the other hand, I can initialize the string as STRING_LIST_INIT_DUP which will automatically set strdup_strings as 1 and then also free the memory of ref at that point after the string ref was appended to the list. Personally, I will prefer the latter one. Regards, 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
Re: What's cooking in git.git (Jun 2016, #04; Tue, 14)
Duy Nguyen writes: >> * nd/worktree-cleanup-post-head-protection (2016-05-24) 6 commits >> - worktree: simplify prefixing paths >> - worktree: avoid 0{40}, too many zeroes, hard to read >> - worktree.c: use is_dot_or_dotdot() >> - git-worktree.txt: keep subcommand listing in alphabetical order >> - worktree.c: rewrite mark_current_worktree() to avoid strbuf >> - completion: support git-worktree >> (this branch is used by nd/worktree-lock.) >> >> Further preparatory clean-up for "worktree" feature. >> >> Expecting a reroll. >> ($gmane/294136, etc.) > > Hmm.. I think what's in 'pu' (which is v2, $gmane/295260) is ok now. Yes, thanks for spotting. I should have updated the status text when I picked up the reroll. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] wrapper: move is_empty_file() from builtin/am.c
Hey Eric, On Wed, Jun 15, 2016 at 11:52 PM, Eric Sunshine wrote: > On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: >> is_empty_file() can help to refactor a lot of code. Also it is quite >> helpful while converting shell scripts which use `test -s`. Since > > As justification, "can help to refactor a lot of code" is very > nebulous. It would be better to give a concrete reason for moving the > function, such as explaining that the functionality will be needed by > the "git bisect" port to C. Sure I can include that. >> is_empty_file() is now a "library" function, its inappropriate to die() so >> instead error_errno() is used to convey the message to stderr while the >> appropriate boolean value is returned. >> >> Signed-off-by: Pranit Bauva >> --- >> diff --git a/builtin/am.c b/builtin/am.c >> @@ -30,22 +30,6 @@ >> /** >> - * Returns 1 if the file is empty or does not exist, 0 otherwise. >> - */ >> -static int is_empty_file(const char *filename) >> -{ >> - struct stat st; >> - >> - if (stat(filename, &st) < 0) { >> - if (errno == ENOENT) >> - return 1; >> - die_errno(_("could not stat %s"), filename); >> - } >> - >> - return !st.st_size; >> -} > > So, the original function die()'d for unexpected errors, but the > rewrite does not. This is a big behavior change. To account for such a > change in behavior I'd expect to see git-am updated to die() on its > own for such failures, but no such changes are present in this patch. > More about this below... > >> diff --git a/wrapper.c b/wrapper.c >> @@ -696,3 +696,16 @@ void sleep_millisec(int millisec) >> +int is_empty_file(const char *filename) >> +{ >> + struct stat st; >> + >> + if (stat(filename, &st) < 0) { >> + if (errno == ENOENT) >> + return 1; >> + error_errno(_("could not stat %s"), filename); > > Mental note: There is no 'return' in front of error_errno(), so the > function does not exit here... This is purposely as I want to keep this function to return only boolean values ( 0 or 1). >> + } >> + >> + return !st.st_size; >> +} > > If stat() returns some error other than ENOENT, then the value of 'st' > will be undefined, yet this return statement accesses its 'st_size' > field, which is clearly a bad thing to do. > > You either need to return a designated value (such as -1) upon errors > other than ENOENT (and update the documentation to mention -1) so that > the caller can decided what to do, or die() as the original did. While > it's true that die()'ing is not necessarily friendly in library code, > it may be acceptable until such time that you find a caller which > needs different behavior. I didn't realize the complexity the patch carried with itself before. I probably shouldn't fidget with am code right now, its a work better left to who are converting the code to library code. I think its the best fit for this situation to leave it as die()'ing. Regards, 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
Re: [PATCH] verify-tag: allow to verify signed blob objects
Michael J Gruber writes: > diff --git a/tag.c b/tag.c > index d1dcd18..d5f090b 100644 > --- a/tag.c > +++ b/tag.c > @@ -39,7 +39,7 @@ int gpg_verify_tag(const unsigned char *sha1, const char > *name_to_report, > int ret; > > type = sha1_object_info(sha1, NULL); > - if (type != OBJ_TAG) > + if ((type != OBJ_TAG) && ((type != OBJ_BLOB) || !(flags & > GPG_VERIFY_BLOB))) > return error("%s: cannot verify a non-tag object of type %s.", > name_to_report ? > name_to_report : The double negation is very hard to read. I wonder if ((type != OBJ_TAG) && !((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB))) is easier to follow? "It is not a tag object, and it's not like we were asked to verify blob and the user gave us a blob, either, so let's complain" is easier to follow, at least for me. Or even if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB)) "you told me to check blob but didn't give me one"; } else if (type != OBJ_TAG) "you didn't give me a tag"; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2016, #04; Tue, 14)
Mike Hommey writes: > On Tue, Jun 14, 2016 at 03:08:04PM -0700, Junio C Hamano wrote: >> * mh/connect (2016-06-06) 10 commits >> - connect: [host:port] is legacy for ssh >> ... >> - connect: document why we sometimes call get_port after get_host_and_port >> >> Ok, folks, is everybody happy with this version? > > $gmane/296609 > $gmane/296610 Oh, I have seen these, and I know you two are happy. But I am having a hard time coming up with a few-line summary for this topic. I can write the beginning part, i.e. "Git-URL parsing routine has been rewritten", but the concluding part of the sentence cannot be "... has been rewritten for no good reason." if I were to mark the topic as "Will merge to 'next'". The best I can come up with is "... has been rewritten (hopefully) without changing the benaviour.", but that is not a strong-enough justificaiton to make the change to the codebase, either. In short, while the update may not introduce new bugs, why would we want to have this change in the first place? By the way, please do not quote the whole thing when you are responding to a tiny part of the original 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 v2 3/6] wrapper: move is_empty_file() from builtin/am.c
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: > is_empty_file() can help to refactor a lot of code. Also it is quite > helpful while converting shell scripts which use `test -s`. Since As justification, "can help to refactor a lot of code" is very nebulous. It would be better to give a concrete reason for moving the function, such as explaining that the functionality will be needed by the "git bisect" port to C. > is_empty_file() is now a "library" function, its inappropriate to die() so > instead error_errno() is used to convey the message to stderr while the > appropriate boolean value is returned. > > Signed-off-by: Pranit Bauva > --- > diff --git a/builtin/am.c b/builtin/am.c > @@ -30,22 +30,6 @@ > /** > - * Returns 1 if the file is empty or does not exist, 0 otherwise. > - */ > -static int is_empty_file(const char *filename) > -{ > - struct stat st; > - > - if (stat(filename, &st) < 0) { > - if (errno == ENOENT) > - return 1; > - die_errno(_("could not stat %s"), filename); > - } > - > - return !st.st_size; > -} So, the original function die()'d for unexpected errors, but the rewrite does not. This is a big behavior change. To account for such a change in behavior I'd expect to see git-am updated to die() on its own for such failures, but no such changes are present in this patch. More about this below... > diff --git a/wrapper.c b/wrapper.c > @@ -696,3 +696,16 @@ void sleep_millisec(int millisec) > +int is_empty_file(const char *filename) > +{ > + struct stat st; > + > + if (stat(filename, &st) < 0) { > + if (errno == ENOENT) > + return 1; > + error_errno(_("could not stat %s"), filename); Mental note: There is no 'return' in front of error_errno(), so the function does not exit here... > + } > + > + return !st.st_size; > +} If stat() returns some error other than ENOENT, then the value of 'st' will be undefined, yet this return statement accesses its 'st_size' field, which is clearly a bad thing to do. You either need to return a designated value (such as -1) upon errors other than ENOENT (and update the documentation to mention -1) so that the caller can decided what to do, or die() as the original did. While it's true that die()'ing is not necessarily friendly in library code, it may be acceptable until such time that you find a caller which needs different behavior. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] wrapper: move is_empty_file() from builtin/am.c
Hey Junio, On Wed, Jun 15, 2016 at 11:42 PM, Junio C Hamano wrote: > Pranit Bauva writes: > >> diff --git a/builtin/am.c b/builtin/am.c >> index 3dfe70b..84f21d0 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -30,22 +30,6 @@ >> #include "mailinfo.h" >> >> /** >> - * Returns 1 if the file is empty or does not exist, 0 otherwise. >> - */ >> -static int is_empty_file(const char *filename) >> -{ >> - struct stat st; >> - >> - if (stat(filename, &st) < 0) { >> - if (errno == ENOENT) >> - return 1; >> - die_errno(_("could not stat %s"), filename); >> - } >> - >> - return !st.st_size; >> -} >> - > > This is perfectly fine in the context of "git am", but as a public > function that is called is_empty_file(), callers can come from two > camps. One is like the caller of this function in "am" where an > empty and a missing file are treated equivalently. The other would > want to act differently. > > Renaming it "is-empty-or-missing" is necessary in order to make it > clear that this helper function is not targetted for the latter > callers. Yes, its better to rename the function as is_empty_or_missing() . Thanks! Regards, 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
Re: [PATCH v2 3/6] wrapper: move is_empty_file() from builtin/am.c
Pranit Bauva writes: > diff --git a/builtin/am.c b/builtin/am.c > index 3dfe70b..84f21d0 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -30,22 +30,6 @@ > #include "mailinfo.h" > > /** > - * Returns 1 if the file is empty or does not exist, 0 otherwise. > - */ > -static int is_empty_file(const char *filename) > -{ > - struct stat st; > - > - if (stat(filename, &st) < 0) { > - if (errno == ENOENT) > - return 1; > - die_errno(_("could not stat %s"), filename); > - } > - > - return !st.st_size; > -} > - This is perfectly fine in the context of "git am", but as a public function that is called is_empty_file(), callers can come from two camps. One is like the caller of this function in "am" where an empty and a missing file are treated equivalently. The other would want to act differently. Renaming it "is-empty-or-missing" is necessary in order to make it clear that this helper function is not targetted for the latter callers. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] convert various shell functions in git-bisect to C
Hey Eric, On Wed, Jun 15, 2016 at 11:23 PM, Eric Sunshine wrote: > On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: >> Changes wrt previous version: >> * Use STRING_LIST_INIT_NODUP to avoid leaks in bisect_clean_state() >> * Use test_path_is_missing in the patch 2/6 >> * drop file_size() >> * move is_empty_file() method from builtin/am.c to wrapper.c >> * use static for methods >> * remove the variable status in bisect_reset() altogether and put the whole >>thing inside the if block. >> * one more method converted namely bisect_write(). > > As an aid to reviewers, in the future, please also include in the > cover letter an interdiff between the previous and current version of > the patch series. Thanks. Sure! Regards, 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
Re: [PATCH v2 1/6] bisect--helper: `bisect_clean_state` shell function in C
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: > Reimplement `bisect_clean_state` shell function in C and add a > `bisect-clean-state` subcommand to `git bisect--helper` to call it from > git-bisect.sh . > [...] > Signed-off-by: Pranit Bauva > --- > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > +static int mark_for_removal(const char *refname, const struct object_id *oid, > + int flag, void *cb_data) > +{ > + struct string_list *refs = cb_data; > + char *ref = xstrfmt("refs/bisect/%s", refname); > + string_list_append(refs, ref); > + return 0; > +} > + > +static int bisect_clean_state(void) > +{ > + int result = 0; > + > + /* There may be some refs packed during bisection */ > + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP; > + for_each_ref_in("refs/bisect/", mark_for_removal, (void *) > &refs_for_removal); > + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD")); > + result = delete_refs(&refs_for_removal); > + string_list_clear(&refs_for_removal, 0); This is leaking all the strings added to 'refs_for_removal', isn't it? Either you need to loop over the items and free the strings manually, or (if it's not too ugly), set 'strdup_strings' before invoking string_list_clear(). > + remove_path(git_path_bisect_expected_rev()); > + remove_path(git_path_bisect_ancestors_ok()); > + remove_path(git_path_bisect_log()); > + remove_path(git_path_bisect_names()); > + remove_path(git_path_bisect_run()); > + remove_path(git_path_bisect_write_terms()); > + /* Cleanup head-name if it got left by an old version of git-bisect */ > + remove_path(git_path_head_name()); > + /* > +* Cleanup BISECT_START last to support the --no-checkout option > +* introduced in the commit 4796e823a. > +*/ > + remove_path(git_path_bisect_start()); > + > + return result; > +} -- 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] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name
dexteritas writes: > After the ASCII-check, test the windows compatibility of file names. > Can be disabled by: > git config hooks.allownonwindowschars true > --- Missing sign off. > templates/hooks--pre-commit.sample | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/templates/hooks--pre-commit.sample > b/templates/hooks--pre-commit.sample > index 68d62d5..120daf1 100755 > --- a/templates/hooks--pre-commit.sample > +++ b/templates/hooks--pre-commit.sample > @@ -17,6 +17,7 @@ fi > > # If you want to allow non-ASCII filenames set this variable to true. > allownonascii=$(git config --bool hooks.allownonascii) > +allownonwindowschars=$(git config --bool hooks.allownonwindowschars) > > # Redirect output to stderr. > exec 1>&2 > @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check > using: >git config hooks.allownonascii true > EOF > exit 1 > +elif [ "$allownonwindowschars" != "true" ] && This line is doubly irritating because (1) the double negation is somewhat hard to grok, and (2) [] is not part of our CodingStyle. Because you inherited this from the existing "allow-non-ascii" one, however, I do not want to see you change this line, if you need to reroll. > + # If you work with linux and windows, there is a problem, if you use > + # chars like \ / : * ? " < > | There is no reason to single out Linux, is there? This new check is only about Windows and because people on other platforms, not limited to Linux, may not be aware of some characters that are not usable on Windows, you are trying to help them, no? > + # Check if there are used only windows compatible chars > + test $(git diff --cached --name-only --diff-filter=A -z $against | > + LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0 Because this is in "elif", we know we are allowing non-ascii characters when we come here. So you need to do a quite similar check from scratch, which is sensible. I do not offhand know if this covers all the characters that Windows users cannot use, though. > +then > + cat <<\EOF > +Error: Attempt to add a chars that are not allowed for a windows file name. > + > +This can cause problems if you want to work with people on other platforms. > + > +To be portable it is advisable to rename the file. > + > +Check your filenames for: \ / : * ? " < > | > + > +If you know what you are doing you can disable this check using: > + > + git config hooks.allownonwindowschars true > +EOF > + exit 2 Why 2? > fi > > # If there are whitespace errors, print the offending file names and fail. When the user says [hooks] allownonascii = false allownonwindowschars = false what happens? The user's intention clearly is that the project wants to be usable on Windows and also wants to exclude characters from codepages that are not ASCII. I however suspect that the hook with your patch will allow people to create a "path>like?this.txt" happily. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] convert various shell functions in git-bisect to C
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva wrote: > Changes wrt previous version: > * Use STRING_LIST_INIT_NODUP to avoid leaks in bisect_clean_state() > * Use test_path_is_missing in the patch 2/6 > * drop file_size() > * move is_empty_file() method from builtin/am.c to wrapper.c > * use static for methods > * remove the variable status in bisect_reset() altogether and put the whole >thing inside the if block. > * one more method converted namely bisect_write(). As an aid to reviewers, in the future, please also include in the cover letter an interdiff between the previous and current version of the patch series. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Easiest way to clone over an existing directory?
Currently, every time I set up a new system, I run the following: git clone $MY_HOMEDIR mv home/.git . rm -r home git checkout -f This seems like an odd dance to go through. But I can't just git clone into ~ directly, because git clone will not clone into an existing non-empty directory. (I could use "git clone -n" to avoid the unnecessary checkout, but the files are small, and it wouldn't remove the need to rmdir so the number of commands would remain the same.) Does some better way exist to handle this? And if not, would it make sense for git clone to have an option to clone into an existing directory (which should also avoid setting junk_work_tree)? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/6] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
Reimplement `is_expected_rev` & `check_expected_revs` shell function in C and add a `--check-expected-revs` subcommand to `git bisect--helper` to call it from git-bisect.sh . Using `--check-expected-revs` subcommand is a temporary measure to port shell functions to C so as to use the existing test suite. As more functions are ported, this subcommand would be retired and will be called by some other method. Helped-by: Eric Sunshine Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/bisect--helper.c | 37 - git-bisect.sh| 20 ++-- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 14043a8..f11c247 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -162,13 +162,44 @@ static int bisect_reset(const char *commit) return bisect_clean_state(); } +static int is_expected_rev(const char *expected_hex) +{ + struct strbuf actual_hex = STRBUF_INIT; + int res; + + if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) < 0) { + strbuf_release(&actual_hex); + return 0; + } + + strbuf_trim(&actual_hex); + res = !strcmp(actual_hex.buf, expected_hex); + strbuf_release(&actual_hex); + return res; +} + +static int check_expected_revs(const char **revs, int rev_nr) +{ + int i; + + for (i = 0; i < rev_nr; i++) { + if (!is_expected_rev(revs[i])) { + remove_path(git_path_bisect_ancestors_ok()); + remove_path(git_path_bisect_expected_rev()); + return 0; + } + } + return 0; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { NEXT_ALL = 1, WRITE_TERMS, BISECT_CLEAN_STATE, - BISECT_RESET + BISECT_RESET, + CHECK_EXPECTED_REVS } cmdmode = 0; int no_checkout = 0; struct option options[] = { @@ -180,6 +211,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("cleanup the bisection state"), BISECT_CLEAN_STATE), OPT_CMDMODE(0, "bisect-reset", &cmdmode, N_("reset the bisection state"), BISECT_RESET), + OPT_CMDMODE(0, "check-expected-revs", &cmdmode, +N_("check for expected revs"), CHECK_EXPECTED_REVS), OPT_BOOL(0, "no-checkout", &no_checkout, N_("update BISECT_HEAD instead of checking out the current commit")), OPT_END() @@ -206,6 +239,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) if (argc > 1) die(_("--bisect-reset requires either zero or one arguments")); return bisect_reset(argc ? argv[0] : NULL); + case CHECK_EXPECTED_REVS: + return check_expected_revs(argv, argc); default: die("BUG: unknown subcommand '%d'", cmdmode); } diff --git a/git-bisect.sh b/git-bisect.sh index 18580b7..4f6545e 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -238,22 +238,6 @@ bisect_write() { test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG" } -is_expected_rev() { - test -f "$GIT_DIR/BISECT_EXPECTED_REV" && - test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV") -} - -check_expected_revs() { - for _rev in "$@"; do - if ! is_expected_rev "$_rev" - then - rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" - rm -f "$GIT_DIR/BISECT_EXPECTED_REV" - return - fi - done -} - bisect_skip() { all='' for arg in "$@" @@ -280,7 +264,7 @@ bisect_state() { rev=$(git rev-parse --verify $(bisect_head)) || die "$(gettext "Bad rev input: $(bisect_head)")" bisect_write "$state" "$rev" - check_expected_revs "$rev" ;; + git bisect--helper --check-expected-revs "$rev" ;; 2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip) shift hash_list='' @@ -294,7 +278,7 @@ bisect_state() { do bisect_write "$state" "$rev" done - check_expected_revs $hash_list ;; + git bisect--helper --check-expected-revs $hash_list ;; *,"$TERM_BAD") die "$(eval_gettext "'git bisect \$TERM_BAD' can take only one argument.")" ;; *) -- 2.9.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 v2 4/6] bisect--helper: `bisect_reset` shell function in C
Reimplement `bisect_reset` shell function in C and add a `--bisect-reset` subcommand to `git bisect--helper` to call it from git-bisect.sh . Using `bisect_reset` subcommand is a temporary measure to port shell functions to C so as to use the existing test suite. As more functions are ported, this subcommand would be retired and will be called by some other method. Note: --bisect-clean-state subcommand has not been retired as there are still a function namely `bisect_start()` which still uses this subcommand. Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/bisect--helper.c | 48 +++- git-bisect.sh| 28 ++-- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 3e4a458..14043a8 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -4,6 +4,8 @@ #include "bisect.h" #include "refs.h" #include "dir.h" +#include "argv-array.h" +#include "run-command.h" static GIT_PATH_FUNC(git_path_bisect_write_terms, "BISECT_TERMS") static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") @@ -13,11 +15,13 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN") static GIT_PATH_FUNC(git_path_head_name, "head-name") static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START") +static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD") static const char * const git_bisect_helper_usage[] = { N_("git bisect--helper --next-all [--no-checkout]"), N_("git bisect--helper --write-terms "), N_("git bisect--helper --bisect-clean-state"), + N_("git bisect--helper --bisect-reset []"), NULL }; @@ -123,12 +127,48 @@ static int bisect_clean_state(void) return result; } +static int bisect_reset(const char *commit) +{ + struct strbuf branch = STRBUF_INIT; + + if (!commit) { + if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) { + printf("We are not bisecting.\n"); + return 0; + } + strbuf_rtrim(&branch); + + } else { + struct object_id oid; + if (get_oid(commit, &oid)) + return error(_("'%s' is not a valid commit"), commit); + strbuf_addf(&branch, "%s", commit); + } + + if (!file_exists(git_path_bisect_head())) { + struct argv_array argv = ARGV_ARRAY_INIT; + argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL); + if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { + error(_("Could not check out original HEAD '%s'. Try" + "'git bisect reset '."), branch.buf); + strbuf_release(&branch); + argv_array_clear(&argv); + return -1; + } + argv_array_clear(&argv); + } + + strbuf_release(&branch); + return bisect_clean_state(); +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { NEXT_ALL = 1, WRITE_TERMS, - BISECT_CLEAN_STATE + BISECT_CLEAN_STATE, + BISECT_RESET } cmdmode = 0; int no_checkout = 0; struct option options[] = { @@ -138,6 +178,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS), OPT_CMDMODE(0, "bisect-clean-state", &cmdmode, N_("cleanup the bisection state"), BISECT_CLEAN_STATE), + OPT_CMDMODE(0, "bisect-reset", &cmdmode, +N_("reset the bisection state"), BISECT_RESET), OPT_BOOL(0, "no-checkout", &no_checkout, N_("update BISECT_HEAD instead of checking out the current commit")), OPT_END() @@ -160,6 +202,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) if (argc != 0) die(_("--bisect-clean-state requires no arguments")); return bisect_clean_state(); + case BISECT_RESET: + if (argc > 1) + die(_("--bisect-reset requires either zero or one arguments")); + return bisect_reset(argc ? argv[0] : NULL); default: die("BUG: unknown subcommand '%d'", cmdmode); } diff --git a/git-bisect.sh b/git-bisect.sh index bbc57d2..18580b7 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -409,35 +409,11 @@ bisect_visualize() { eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES") } -bisect_reset() { - test -s "$GIT_DIR/BISECT_START" || {
[PATCH v2 0/6] convert various shell functions in git-bisect to C
A previous version is available here[1]. Changes wrt previous version: * Use STRING_LIST_INIT_NODUP to avoid leaks in bisect_clean_state() * Use test_path_is_missing in the patch 2/6 * drop file_size() * move is_empty_file() method from builtin/am.c to wrapper.c * use static for methods * remove the variable status in bisect_reset() altogether and put the whole thing inside the if block. * one more method converted namely bisect_write(). An important thing to be discussed: In shell script while reading/writing files, no error is reported but we can actually report it in C. In this series I have tried to output error wherever possible. What are your views? [1]: http://thread.gmane.org/gmane.comp.version-control.git/296717 Pranit Bauva (6): bisect--helper: `bisect_clean_state` shell function in C t6030: explicitly test for bisection cleanup wrapper: move is_empty_file() from builtin/am.c bisect--helper: `bisect_reset` shell function in C bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C bisect--helper: `bisect_write` shell function in C builtin/am.c| 16 builtin/bisect--helper.c| 194 +++- cache.h | 3 + git-bisect.sh | 97 +++--- t/t6030-bisect-porcelain.sh | 17 wrapper.c | 13 +++ 6 files changed, 236 insertions(+), 104 deletions(-) -- 2.9.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 v2 3/6] wrapper: move is_empty_file() from builtin/am.c
is_empty_file() can help to refactor a lot of code. Also it is quite helpful while converting shell scripts which use `test -s`. Since is_empty_file() is now a "library" function, its inappropriate to die() so instead error_errno() is used to convey the message to stderr while the appropriate boolean value is returned. Suggested-by: Torsten Bögershausen Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/am.c | 16 cache.h | 3 +++ wrapper.c| 13 + 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 3dfe70b..84f21d0 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -30,22 +30,6 @@ #include "mailinfo.h" /** - * Returns 1 if the file is empty or does not exist, 0 otherwise. - */ -static int is_empty_file(const char *filename) -{ - struct stat st; - - if (stat(filename, &st) < 0) { - if (errno == ENOENT) - return 1; - die_errno(_("could not stat %s"), filename); - } - - return !st.st_size; -} - -/** * Returns the length of the first line of msg. */ static int linelen(const char *msg) diff --git a/cache.h b/cache.h index 6049f86..8eaad70 100644 --- a/cache.h +++ b/cache.h @@ -1870,4 +1870,7 @@ void sleep_millisec(int millisec); */ void safe_create_dir(const char *dir, int share); +/* Return 1 if the file is empty or does not exists, 0 otherwise. */ +extern int is_empty_file(const char *filename); + #endif /* CACHE_H */ diff --git a/wrapper.c b/wrapper.c index 5dc4e15..36a3eeb 100644 --- a/wrapper.c +++ b/wrapper.c @@ -696,3 +696,16 @@ void sleep_millisec(int millisec) { poll(NULL, 0, millisec); } + +int is_empty_file(const char *filename) +{ + struct stat st; + + if (stat(filename, &st) < 0) { + if (errno == ENOENT) + return 1; + error_errno(_("could not stat %s"), filename); + } + + return !st.st_size; +} -- 2.9.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 v2 2/6] t6030: explicitly test for bisection cleanup
This is not an improvement in the test coverage but it helps in making it explicit as to what exactly would be the error as other tests are focussed on testing other things. Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- I faced this problem while converting `bisect_clean_state` and the tests where showing breakages but it wasn't clear as to where exactly are they breaking. This will patch will help in that. Also I tested the test coverage of the test suite before this patch and it covers this (I did this by purposely changing names of files in git-bisect.sh and running the test suite). Signed-off-by: Pranit Bauva --- t/t6030-bisect-porcelain.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index e74662b..a17f7a6 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -894,4 +894,21 @@ test_expect_success 'bisect start takes options and revs in any order' ' test_cmp expected actual ' +test_expect_success 'git bisect reset cleans bisection state properly' ' + git bisect reset && + git bisect start && + git bisect good $HASH1 && + git bisect bad $HASH4 && + git bisect reset && + test -z "$(git for-each-ref "refs/bisect/*")" && + test_path_is_missing "$GIT_DIR/BISECT_EXPECTED_REV" && + test_path_is_missing "$GIT_DIR/BISECT_ANCESTORS_OK" && + test_path_is_missing "$GIT_DIR/BISECT_LOG" && + test_path_is_missing "$GIT_DIR/BISECT_RUN" && + test_path_is_missing "$GIT_DIR/BISECT_TERMS" && + test_path_is_missing "$GIT_DIR/head-name" && + test_path_is_missing "$GIT_DIR/BISECT_HEAD" && + test_path_is_missing "$GIT_DIR/BISECT_START" +' + test_done -- 2.9.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 v2 1/6] bisect--helper: `bisect_clean_state` shell function in C
Reimplement `bisect_clean_state` shell function in C and add a `bisect-clean-state` subcommand to `git bisect--helper` to call it from git-bisect.sh . Using `--bisect-clean-state` subcommand is a measure to port shell function to C so as to use the existing test suite. As more functions are ported, this subcommand will be retired and will be called by bisect_reset() and bisect_start(). Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/bisect--helper.c | 54 +++- git-bisect.sh| 26 +++ 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 91027b0..3e4a458 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -3,12 +3,21 @@ #include "parse-options.h" #include "bisect.h" #include "refs.h" +#include "dir.h" static GIT_PATH_FUNC(git_path_bisect_write_terms, "BISECT_TERMS") +static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") +static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK") +static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG") +static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") +static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN") +static GIT_PATH_FUNC(git_path_head_name, "head-name") +static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START") static const char * const git_bisect_helper_usage[] = { N_("git bisect--helper --next-all [--no-checkout]"), N_("git bisect--helper --write-terms "), + N_("git bisect--helper --bisect-clean-state"), NULL }; @@ -78,11 +87,48 @@ static int write_terms(const char *bad, const char *good) return (res < 0) ? -1 : 0; } +static int mark_for_removal(const char *refname, const struct object_id *oid, + int flag, void *cb_data) +{ + struct string_list *refs = cb_data; + char *ref = xstrfmt("refs/bisect/%s", refname); + string_list_append(refs, ref); + return 0; +} + +static int bisect_clean_state(void) +{ + int result = 0; + + /* There may be some refs packed during bisection */ + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP; + for_each_ref_in("refs/bisect/", mark_for_removal, (void *) &refs_for_removal); + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD")); + result = delete_refs(&refs_for_removal); + string_list_clear(&refs_for_removal, 0); + remove_path(git_path_bisect_expected_rev()); + remove_path(git_path_bisect_ancestors_ok()); + remove_path(git_path_bisect_log()); + remove_path(git_path_bisect_names()); + remove_path(git_path_bisect_run()); + remove_path(git_path_bisect_write_terms()); + /* Cleanup head-name if it got left by an old version of git-bisect */ + remove_path(git_path_head_name()); + /* +* Cleanup BISECT_START last to support the --no-checkout option +* introduced in the commit 4796e823a. +*/ + remove_path(git_path_bisect_start()); + + return result; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { NEXT_ALL = 1, - WRITE_TERMS + WRITE_TERMS, + BISECT_CLEAN_STATE } cmdmode = 0; int no_checkout = 0; struct option options[] = { @@ -90,6 +136,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("perform 'git bisect next'"), NEXT_ALL), OPT_CMDMODE(0, "write-terms", &cmdmode, N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS), + OPT_CMDMODE(0, "bisect-clean-state", &cmdmode, +N_("cleanup the bisection state"), BISECT_CLEAN_STATE), OPT_BOOL(0, "no-checkout", &no_checkout, N_("update BISECT_HEAD instead of checking out the current commit")), OPT_END() @@ -108,6 +156,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) if (argc != 2) die(_("--write-terms requires two arguments")); return write_terms(argv[0], argv[1]); + case BISECT_CLEAN_STATE: + if (argc != 0) + die(_("--bisect-clean-state requires no arguments")); + return bisect_clean_state(); default: die("BUG: unknown subcommand '%d'", cmdmode); } diff --git a/git-bisect.sh b/git-bisect.sh index cd39bd0..bbc57d2 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -187,7 +187,7 @@ bisect_start() { # # Get rid of any old bisect state. # - bisect_clean_state || exit + git bisect--helper --bisect-clean-state || exit # # Change state. @@ -196,7 +196,7 @
[PATCH v2 6/6] bisect--helper: `bisect_write` shell function in C
Reimplement the `bisect_write` shell function in C and add a `bisect-write` subcommand to `git bisect--helper` to call it from git-bisect.sh Using `--bisect-write` subcommand is a temporary measure to port shell function in C so as to use the existing test suite. As more functions are ported, this subcommand will be retired and will be called by some other methods. Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD from the global shell script thus we need to pass it to the subcommand using the arguments. After the whole conversion, we can remove the extra arguments and make the method use the two variables from the global scope within the C code. Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/bisect--helper.c | 61 +++- git-bisect.sh| 25 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index f11c247..eebfcf0 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -22,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = { N_("git bisect--helper --write-terms "), N_("git bisect--helper --bisect-clean-state"), N_("git bisect--helper --bisect-reset []"), + N_("git bisect--helper --bisect-write []"), NULL }; @@ -192,6 +193,55 @@ static int check_expected_revs(const char **revs, int rev_nr) return 0; } +static int bisect_write(const char *state, const char *rev, + const char *term_good, const char *term_bad, + int nolog) +{ + struct strbuf tag = STRBUF_INIT; + struct strbuf commit_name = STRBUF_INIT; + struct object_id oid; + struct commit *commit; + struct pretty_print_context pp = {0}; + FILE *fp; + + if (!strcmp(state, term_bad)) + strbuf_addf(&tag, "refs/bisect/%s", state); + else if(one_of(state, term_good, "skip", NULL)) + strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev); + else + return error(_("Bad bisect_write argument: %s"), state); + + if (get_oid(rev, &oid)) { + strbuf_release(&tag); + return error(_("couldn't get the oid of the rev '%s'"), rev); + } + + if (update_ref(NULL, tag.buf, oid.hash, NULL, 0, + UPDATE_REFS_MSG_ON_ERR)) { + strbuf_release(&tag); + return -1; + } + + fp = fopen(git_path_bisect_log(), "a"); + if (!fp) { + strbuf_release(&tag); + return error_errno(_("couldn't open the file '%s'"), git_path_bisect_log()); + } + + commit = lookup_commit_reference(oid.hash); + format_commit_message(commit, "%s", &commit_name, &pp); + fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash), + commit_name.buf); + + if (!nolog) + fprintf(fp, "git bisect %s %s\n", state, rev); + + strbuf_release(&commit_name); + strbuf_release(&tag); + fclose(fp); + return 0; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { @@ -199,7 +249,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) WRITE_TERMS, BISECT_CLEAN_STATE, BISECT_RESET, - CHECK_EXPECTED_REVS + CHECK_EXPECTED_REVS, + BISECT_WRITE } cmdmode = 0; int no_checkout = 0; struct option options[] = { @@ -213,6 +264,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("reset the bisection state"), BISECT_RESET), OPT_CMDMODE(0, "check-expected-revs", &cmdmode, N_("check for expected revs"), CHECK_EXPECTED_REVS), + OPT_CMDMODE(0, "bisect-write", &cmdmode, +N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE), OPT_BOOL(0, "no-checkout", &no_checkout, N_("update BISECT_HEAD instead of checking out the current commit")), OPT_END() @@ -225,6 +278,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) usage_with_options(git_bisect_helper_usage, options); switch (cmdmode) { + int nolog; case NEXT_ALL: return bisect_next_all(prefix, no_checkout); case WRITE_TERMS: @@ -241,6 +295,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) return bisect_reset(argc ? argv[0] : NULL); case CHECK_EXPECTED_REVS: return check_expected_revs(argv, argc); + case BISECT_WRITE: + if (argc != 4 && argc != 5) + die(_("--bisect-write requires either
Re: Git clone 2.9.0
On Wed, Jun 15, 2016 at 8:08 PM, Wojciech Rybiński wrote: > Hi, > > I have installed version 2.9.0 and when I’m trying to clone repo with > specific tag I get error: unknown option `single-branch’ and next warning: > Remote branch a3.0.26 not found in upstream origin, using HEAD instead. My > command looks like that: git clone --single-branch -b a3.0.26 git@… and it’s > working with git version 2.7.4 on another machine. What should I do to clone > repo with this tag using version 2.9.0? I found info that this option is > enable in this version. I can't reproduce. What does this command show to you? GIT_TRACE=1 git clone -h -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git clone 2.9.0
Hi, I have installed version 2.9.0 and when I’m trying to clone repo with specific tag I get error: unknown option `single-branch’ and next warning: Remote branch a3.0.26 not found in upstream origin, using HEAD instead. My command looks like that: git clone --single-branch -b a3.0.26 git@… and it’s working with git version 2.7.4 on another machine. What should I do to clone repo with this tag using version 2.9.0? I found info that this option is enable in this version. Best, Wojciech Rybiński-- 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: bug: compactionheuristic config var case issue
On Wed, Jun 15, 2016 at 5:39 PM, Brian Lalor wrote: > I’m very happy to see the new compaction heuristic option; it’s the way I > always thought diffs should read! > > The config option in the documentation references “diff.compactionHeuristic”, > but diff.c does a case-sensitive comparison on “diff.compactionheuristic” > (note the case of the “h” in “heuristic”) I think this misled you. All configuration variable names are lower-cased before they reach that strcmp() call, the whole picture is more like strcmp(tolower(var), "diff.compactionheuristic"), which I believe is correct. > and `git diff` does not honor the config. Confusingly, `git config > diff.compactionheuristic` returns true when diff.compactionHeuristic is set > in ~/.gitconfig. When diff.compactionheuristic is set to true in > ~/.gitconfig, the desired behavior is achieved. > > Thank you all for Git: it’s hard to remember the terrible world we lived in > before it existed. :-) -- Duy -- 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
Bug report: stdout vs stderr
Why half of Git output goes to stdout and half to stderr? I suspect this is a bug. Below I call `git pushbug` alias defined it the below presented config file. $ cat .git/config [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true [remote "origin"] url = g...@bitbucket.org:portonv/algebraic-general-topology.git fetch = +refs/heads/*:refs/remotes/origin/* pushurl = g...@github.com:vporton/algebraic-general-topology.git pushurl = g...@bitbucket.org:portonv/algebraic-general- topology.git [gui] wmstate = normal geometry = 1680x957+0+27 189 177 [alias] pushbug = !git push && git checkout prerelease && git merge master && git push && git checkout devel && git merge prerelease && git push && git checkout master [branch "master"] remote = origin merge = refs/heads/master [branch "prerelease"] remote = origin merge = refs/heads/prerelease [branch "devel"] remote = origin merge = refs/heads/devel $ git pushbug 1>$HOME/t/1.txt 2>$HOME/t/2.txt $ cat ~/t/1.txt Your branch is up-to-date with 'origin/prerelease'. Updating ac492a4..c55d1b5 Fast-forward chap-sides.tex | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Your branch is up-to-date with 'origin/devel'. Updating ac492a4..c55d1b5 Fast-forward chap-sides.tex | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Your branch is up-to-date with 'origin/master'. $ cat ~/t/2.txt To g...@github.com:vporton/algebraic-general-topology.git ac492a4..c55d1b5 master -> master To g...@bitbucket.org:portonv/algebraic-general-topology.git ac492a4..c55d1b5 master -> master Switched to branch 'prerelease' To g...@github.com:vporton/algebraic-general-topology.git ac492a4..c55d1b5 prerelease -> prerelease remote: remote: Create pull request for prerelease: remote: https://bitbucket.org/portonv/algebraic-general-topology/pull -requests/new?source=prerelease&t=1 remote: To g...@bitbucket.org:portonv/algebraic-general-topology.git ac492a4..c55d1b5 prerelease -> prerelease Switched to branch 'devel' To g...@github.com:vporton/algebraic-general-topology.git ac492a4..c55d1b5 devel -> devel remote: remote: Create pull request for devel: remote: https://bitbucket.org/portonv/algebraic-general-topology/pull -requests/new?source=devel&t=1 remote: To g...@bitbucket.org:portonv/algebraic-general-topology.git ac492a4..c55d1b5 devel -> devel Switched to branch 'master' -- 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
[ANNOUNCE] Git Rev News edition 16
Hi everyone, I'm happy announce that the 16th edition of Git Rev News is now published: http://git.github.io/rev_news/2016/06/15/edition-16/ Thanks a lot to all the contributors and helpers, especially Duy and the Ensimag students! Enjoy, Christian, Thomas and Nicola. -- 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] verify-tag: allow to verify signed blob objects
Currently, there is no easy way to verify push certificates. They have the same structure as signed tags: "attached detached signatures", that is: the concatenation of the signed material and its detached signature. Introduce a `--blob` option to verify-tag so that it allows to verify tags and blobs. Signed-off-by: Michael J Gruber --- The first outcome of my long announced project to describe our signature formats in Documentation/technical (progress underway) In fact, that whole area is in need of refactoring: gpg related bits are all over the place, including tag.c. The proposed patch neither improves nor worsens the situation in that respect. But, since we make it unnecessarily hard to verify signatures (git cat-file | gpg --verify fails) it's only fair to provide a tool for pre-receive hook writers. Documentation/git-verify-tag.txt | 4 builtin/verify-tag.c | 1 + gpg-interface.h | 1 + t/t5534-push-signed.sh | 3 ++- tag.c| 2 +- 5 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt index d590edc..2e5cf4d 100644 --- a/Documentation/git-verify-tag.txt +++ b/Documentation/git-verify-tag.txt @@ -20,6 +20,10 @@ OPTIONS Print the raw gpg status output to standard error instead of the normal human-readable output. +--blob:: + Allow to verify signed blob objects (in addition to tag objects), such as the + objects containing a push certificate. + -v:: --verbose:: Print the contents of the tag object before validating it. diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..19d26b0 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -33,6 +33,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_BIT(0, "blob", &flags, N_("allow to verify blob objects"), GPG_VERIFY_BLOB), OPT_END() }; diff --git a/gpg-interface.h b/gpg-interface.h index ea68885..a3cbfc3 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_BLOB4 struct signature_check { char *payload; diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh index ecb8d44..de4d38b 100755 --- a/t/t5534-push-signed.sh +++ b/t/t5534-push-signed.sh @@ -94,7 +94,8 @@ test_expect_success GPG 'signed push sends push certificate' ' # record the push certificate if test -n "${GIT_PUSH_CERT-}" then - git cat-file blob $GIT_PUSH_CERT >../push-cert + git cat-file blob $GIT_PUSH_CERT >../push-cert && + git verify-tag --blob $GIT_PUSH_CERT fi && cat >../push-cert-status
Re: [PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name
Am 15.06.2016 um 10:02 schrieb dexteritas: > After the ASCII-check, test the windows compatibility of file names. > Can be disabled by: > git config hooks.allownonwindowschars true > --- > templates/hooks--pre-commit.sample | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/templates/hooks--pre-commit.sample > b/templates/hooks--pre-commit.sample > index 68d62d5..120daf1 100755 > --- a/templates/hooks--pre-commit.sample > +++ b/templates/hooks--pre-commit.sample > @@ -17,6 +17,7 @@ fi > > # If you want to allow non-ASCII filenames set this variable to true. > allownonascii=$(git config --bool hooks.allownonascii) > +allownonwindowschars=$(git config --bool hooks.allownonwindowschars) > > # Redirect output to stderr. > exec 1>&2 > @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check > using: >git config hooks.allownonascii true > EOF > exit 1 > +elif [ "$allownonwindowschars" != "true" ] && > + # If you work with linux and windows, there is a problem, if you use > + # chars like \ / : * ? " < > | > + # Check if there are used only windows compatible chars > + test $(git diff --cached --name-only --diff-filter=A -z $against | > + LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0 > +then > + cat <<\EOF > +Error: Attempt to add a chars that are not allowed for a windows file name. > + > +This can cause problems if you want to work with people on other platforms. > + > +To be portable it is advisable to rename the file. > + > +Check your filenames for: \ / : * ? " < > | > + > +If you know what you are doing you can disable this check using: > + > + git config hooks.allownonwindowschars true > +EOF > + exit 2 > fi > > # If there are whitespace errors, print the offending file names and fail. There are some cases of illegal file names missing. E.g. reserved names, trailing period and space. My trial with a precommit hook for avoiding illegal filenames on windows can be found at [1]. Feel free to loot my version for a reroll. [1]: https://github.com/t-b/git-pre-commit-hook-windows-filenames/blob/master/pre-commit -- 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
bug: compactionheuristic config var case issue
I’m very happy to see the new compaction heuristic option; it’s the way I always thought diffs should read! The config option in the documentation references “diff.compactionHeuristic”, but diff.c does a case-sensitive comparison on “diff.compactionheuristic” (note the case of the “h” in “heuristic”) and `git diff` does not honor the config. Confusingly, `git config diff.compactionheuristic` returns true when diff.compactionHeuristic is set in ~/.gitconfig. When diff.compactionheuristic is set to true in ~/.gitconfig, the desired behavior is achieved. Thank you all for Git: it’s hard to remember the terrible world we lived in before it existed. :-)-- 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: I lost my commit signature
Thank you all very much! -Schrödinger On Wed, Jun 15, 2016 at 3:07 PM, Michael J Gruber wrote: > Jeff King venit, vidit, dixit 15.06.2016 06:34: >> On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote: >> >>> I got two more lines from gpg -v during commit with -S: >>> ``` >>> gpg: writing to stdout >>> gpg: RSA/SHA1 signature from: "2EF2AD6E Tian Zhen " >>> ``` >>> >>> after I commit, I push it to remote, but someone had pushed before to >>> master branch, so I pull on master branch(`git pull --rebase`), then I >>> check my commit via `git log --show-signature`, there is no signature >>> in it, so I commit it with --ament and -S again, the signature is come >>> back. >>> >>> I haven't check signature before push, because I have checked four >>> commits before, every commit is fine. >>> >>> I don't know whether the `git pull` influenced signature or not. >> >> Ah, so the problem is probably that you had a signature _initially_, but >> that it did not survive the rebase. Which makes sense, as rebase would >> need to re-sign. It does not by default, but you can tell it to do so >> with "-S". Or you can set `commit.gpgsign`, which should sign in both >> cases. > > While it's clear that a rebase invalidates the signature, we could try > to be more helpful here, especially given the fact that (with our model) > you can't sign a commit afterwards any more. > > commit.gpgsign signs everything, but there should be a mode for > re-signing signed commits, or at least a warning that rebase dropped a > signature so that you can --amend -S the last commit. > > 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: What's cooking in git.git (Jun 2016, #04; Tue, 14)
On Wed, Jun 15, 2016 at 5:08 AM, Junio C Hamano wrote: > * nd/i-t-a-commitable (2016-06-06) 3 commits > - commit: don't count i-t-a entries when checking if the new commit is empty > - Resurrect "diff-lib.c: adjust position of i-t-a entries in diff" > - diff.h: extend "flags" field to 64 bits because we're out of bits > > "rm .git/index && git add -N * && git commit" allows you to create > an empty commit without --allow-empty; attempt to forbid it. > > Breaks many tests by completely butchering "git commit", it seems. Not surprising. I did run some basic tests, but not the test suite. It was more an excuse to bring up the topic again. Please drop it. I will probably resend (with more or less the same idea, since you haven't given a loud and clear "NO"). > * nd/worktree-cleanup-post-head-protection (2016-05-24) 6 commits > - worktree: simplify prefixing paths > - worktree: avoid 0{40}, too many zeroes, hard to read > - worktree.c: use is_dot_or_dotdot() > - git-worktree.txt: keep subcommand listing in alphabetical order > - worktree.c: rewrite mark_current_worktree() to avoid strbuf > - completion: support git-worktree > (this branch is used by nd/worktree-lock.) > > Further preparatory clean-up for "worktree" feature. > > Expecting a reroll. > ($gmane/294136, etc.) Hmm.. I think what's in 'pu' (which is v2, $gmane/295260) is ok now. -- Duy -- 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] Sharness v1.0.0
>> Is there any word out there from Mathias on making you the designated >> new maintainer? (I cannot tell if this is a genuine maintainer change, or >> a [hostile] fork by reading this email, and I don't know much of the context, Yes, it's 100% genuine. I handed over maintenance to Christian Couder. Here's a signed commit to prove the transfer: https://github.com/chriscool/sharness/pull/55 (Sorry for double posting, the mailing list didn't like my first email.) -- -Mathias https://tinyletter.com/production-ready -- 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] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name
After the ASCII-check, test the windows compatibility of file names. Can be disabled by: git config hooks.allownonwindowschars true --- templates/hooks--pre-commit.sample | 22 ++ 1 file changed, 22 insertions(+) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index 68d62d5..120daf1 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -17,6 +17,7 @@ fi # If you want to allow non-ASCII filenames set this variable to true. allownonascii=$(git config --bool hooks.allownonascii) +allownonwindowschars=$(git config --bool hooks.allownonwindowschars) # Redirect output to stderr. exec 1>&2 @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check using: git config hooks.allownonascii true EOF exit 1 +elif [ "$allownonwindowschars" != "true" ] && + # If you work with linux and windows, there is a problem, if you use + # chars like \ / : * ? " < > | + # Check if there are used only windows compatible chars + test $(git diff --cached --name-only --diff-filter=A -z $against | + LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0 +then + cat <<\EOF +Error: Attempt to add a chars that are not allowed for a windows file name. + +This can cause problems if you want to work with people on other platforms. + +To be portable it is advisable to rename the file. + +Check your filenames for: \ / : * ? " < > | + +If you know what you are doing you can disable this check using: + + git config hooks.allownonwindowschars true +EOF + exit 2 fi # If there are whitespace errors, print the offending file names and fail. -- https://github.com/git/git/pull/252 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2016, #04; Tue, 14)
2016-06-15 0:08 GMT+02:00 Junio C Hamano : > Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. The ones marked with '.' do not appear in any of > the integration branches, but I am still holding onto them. > > Git 2.9 has been tagged. Let's wait for a few days to clean up > possible fallout and then start a new cycle by rewinding the tip of > 'next'. I expect I'd eject a few premature topics out of 'next' > while doing so. > > You can find the changes described here in the integration branches > of the repositories listed at > > http://git-blame.blogspot.com/p/git-public-repositories.html > > -- > [Graduated to "master"] > > * jc/t2300-setup (2016-06-01) 1 commit > (merged to 'next' on 2016-06-06 at 20f7f83) > + t2300: run git-sh-setup in an environment that better mimics the real life > (this branch is used by va/i18n-even-more.) > > A test fix. > > > * jk/diff-compact-heuristic (2016-06-10) 1 commit > - diff: disable compaction heuristic for now > > It turns out that the earlier effort to update the heuristics may > want to use a bit more time to mature. Turn it off by default. > > > * jk/shell-portability (2016-06-01) 2 commits > (merged to 'next' on 2016-06-06 at 5de784e) > + t5500 & t7403: lose bash-ism "local" > + test-lib: add in-shell "env" replacement > > test fixes. > > -- > [New Topics] > > * lv/status-say-working-tree-not-directory (2016-06-09) 1 commit > - Use "working tree" instead of "working directory" for git status > > "git status" used to say "working directory" when it meant "working > tree". > > Will merge to 'next'. > > > * jk/parseopt-string-list (2016-06-13) 3 commits > - blame,shortlog: don't make local option variables static > - interpret-trailers: don't duplicate option strings > - parse_opt_string_list: stop allocating new strings > (this branch is used by jk/string-list-static-init.) > > The command line argument parsing that uses OPT_STRING_LIST() often > made a copy of the argv[] element, which was unnecessary. > > Will merge to 'next'. > > > * jk/repack-keep-unreachable (2016-06-14) 3 commits > - repack: extend --keep-unreachable to loose objects > - repack: add --keep-unreachable option > - repack: document --unpack-unreachable option > > "git repack" learned the "--keep-unreachable" option, which sends > loose unreachable objects to a pack instead of leaving them loose. > This helps heuristics based on the number of loose objects > (e.g. "gc --auto"). > > Will merge to 'next'. > > > * lf/recv-sideband-cleanup (2016-06-13) 1 commit > - sideband.c: refactor recv_sideband() > > Code simplification. It however loses the atomicity of the output > 9ac13ec9 (atomic write for sideband remote messages, 2006-10-11) > tried to add to an once-much-simpler codebase. > > Expecting a reroll. > > > * nd/test-lib-httpd-show-error-log-in-verbose (2016-06-13) 1 commit > - lib-httpd.sh: print error.log on error > > Debugging aid. > > Will merge to 'next'. > > > * pc/occurred (2016-06-10) 2 commits > - config.c: fix misspelt "occurred" in an error message > - refs.h: fix misspelt "occurred" in a comment > > Will merge to 'next'. > > > * sb/submodule-clone-retry (2016-06-13) 2 commits > - submodule update: continue when a clone fails > - submodule--helper: initial clone learns retry logic > (this branch uses sb/submodule-recommend-shallowness.) > > "git submodule update" that drives many "git clone" could > eventually hit flaky servers/network conditions on one of the > submodules; the command learned to retry the attempt. > > > * jc/blame-reverse (2016-06-14) 2 commits > - blame: dwim "blame --reverse OLD" as "blame --reverse OLD.." > - blame: improve diagnosis for "--reverse NEW" > > > * jc/deref-tag (2016-06-14) 1 commit > - blame, line-log: do not loop around deref_tag() > > Code clean-up. > > Will merge to 'next'. > > > * jk/fetch-prune-doc (2016-06-14) 1 commit > - fetch: document that pruning happens before fetching > > Will merge to 'next'. > > > * km/fetch-do-not-free-remote-name (2016-06-14) 1 commit > - builtin/fetch.c: don't free remote->name after fetch > > Will merge to 'next'. > > > * nb/gnome-keyring-build (2016-06-14) 1 commit > - gnome-keyring: Don't hard-code pkg-config executable > > Build improvements for gnome-keyring (in contrib/) > > Will merge to 'next'. > > > * pb/strbuf-read-file-doc (2016-06-14) 1 commit > - strbuf: describe the return value of strbuf_read_file > > Will merge to 'next'. > > -- > [Stalled] > > * sb/bisect (2016-04-15) 22 commits > - SQUASH??? > - bisect: get back halfway shortcut > - bisect: compute best bisection in compute_relevant_weights() > - bisect: use a bottom-up traversal to find relevant weights > - bisect: prepare fo
Re: [PATCHv3] gpg-interface: check gpg signature creation status
Jeff King venit, vidit, dixit 15.06.2016 02:56: > On Tue, Jun 14, 2016 at 04:47:35PM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >>> I'm still undecided on whether it is a better approach than making >>> sure the stdout we got looks sane. In particular I'd worry that it >>> would make things harder for somebody trying to plug in something >>> gpg-like (e.g., if you wanted to do something exotic like call a >>> program which fetched the signature from a remote device or >>> something). But it's probably not _that_ hard for such a script >>> to emulate --status-fd. >> >> I share the same thinking, but at the same time, it already is a >> requirement to give --status-fd output that is close enough on the >> signature verification side, isn't it? > > Yeah, though I could see somebody wanting to sit amidst the signing side > but not verification (e.g., if your keys are elsewhere from the machine > running git). Of course such a case could probably ferry --status-fd > from the other side anyway. > > I admit I don't know of such a case in practice, though, and > implementing a rudimentary --status-fd to say "SIG OK" or whatever on > the signing side is not _that_ big a deal. So if we think this approach > is a more robust solution in the normal case, let's not hold it up over > what-ifs. > > -Peff As for the flexibility: We do code specifically for gpg, which happens to work for gpg2 also. The patch doesn't add any gpg ui requirements that we don't require elsewhere already. More flexibility requires a completely pluggable approach - gpgsm already fails to meet the gpg command line ui. In any case, "status-fd" is *the* way to interface with gpg reliably just like plumbing commands are *the* way to interface with git reliably. As for the read locking: I'm sorry I have no idea about that area at all. I thought that I'm doing the same that we do for verify, but apparently not. My strbuf_read-fu is not that strong either (read: copy&paste). I trust your assessment completely, though. As for the original problem: That had a different cause, as we know now (rebase dropping signatures without hint). I still think we should check gpg status codes properly. 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: I lost my commit signature
Jeff King venit, vidit, dixit 15.06.2016 06:34: > On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote: > >> I got two more lines from gpg -v during commit with -S: >> ``` >> gpg: writing to stdout >> gpg: RSA/SHA1 signature from: "2EF2AD6E Tian Zhen " >> ``` >> >> after I commit, I push it to remote, but someone had pushed before to >> master branch, so I pull on master branch(`git pull --rebase`), then I >> check my commit via `git log --show-signature`, there is no signature >> in it, so I commit it with --ament and -S again, the signature is come >> back. >> >> I haven't check signature before push, because I have checked four >> commits before, every commit is fine. >> >> I don't know whether the `git pull` influenced signature or not. > > Ah, so the problem is probably that you had a signature _initially_, but > that it did not survive the rebase. Which makes sense, as rebase would > need to re-sign. It does not by default, but you can tell it to do so > with "-S". Or you can set `commit.gpgsign`, which should sign in both > cases. While it's clear that a rebase invalidates the signature, we could try to be more helpful here, especially given the fact that (with our model) you can't sign a commit afterwards any more. commit.gpgsign signs everything, but there should be a mode for re-signing signed commits, or at least a warning that rebase dropped a signature so that you can --amend -S the last commit. 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