Re: [PATCHv2] add: ignore only ignored files
+test_expect_success 'error out when attempting to add ignored ones but add others' ' +touch a.if +test_must_fail git add a.?? +! (git ls-files | grep \\.ig) +(git ls-files | grep a.if) +' I am somewhat allergic to pipes in our test suite, because they can mask errors (especially with a negated grep, because we do not know if they correctly produced any output at all). But I guess this is matching the surrounding code, and it is quite unlikely for `ls-files` to fail in any meaningful way here. So I think it's fine. -Peff 2 small comments: Why the escaped \\.ig and the unescaped a.if ? The other question, this is a more general one, strikes me every time I see ! grep Should we avoid it by writing test_must_fail instead of ! ? (The current code base has a mixture of both) The following came into my mind when working on another grepy thing, and it may be unnecessary clumsy: test_expect_success 'error out when attempting to add ignored ones but add others' ' touch a.if test_must_fail git add a.?? git ls-files files.txt test_must_fail grep a.ig files.txt /dev/null grep a.if files.txt /dev/null rm files.txt ' (It feels as if there should be a grepnot ;-) The 3rd comment: Thanks for taking this up! -- 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
recent cygwin breakage
Hi Junio, Just a quick heads-up on a recent cygwin breakage. I updated my (64-bit) cygwin installation yesterday and (along with many other packages) I noticed a new version of gcc (and presumably libc) was installed (gcc v4.8.3-5 x86-64). Unfortunately, this caused new warning to be issued when compiling git. In particular, warnings about the implicit declaration of the memmem(), strlcpy() and strcasestr() functions. As an example: CC archive.o archive.c: In function ‘format_subst’: archive.c:44:3: warning: implicit declaration of function ‘memmem’ [-Wimplicit-function-declaration] b = memmem(src, len, $Format:, 8); ^ archive.c:44:5: warning: assignment makes pointer from integer without a cast [enabled by default] b = memmem(src, len, $Format:, 8); ^ I haven't spent too long on this, but it appears that the _XOPEN_SOURCE setting now trumps _GNU_SOURCE and _BSD_SOURCE settings. (I am guessing that /usr/lib/gcc/x86_64-pc-cygwin/4.8.3/include-fixed/sys/cdefs.h was changed recently to reflect the new priority). Anyway, based on that quick squint, I changed git-compat-util.h, thus: diff --git a/git-compat-util.h b/git-compat-util.h index 400e921..cef2691 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -75,7 +75,8 @@ # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ - !defined(__TANDEM) !defined(__QNX__) !defined(__MirBSD__) + !defined(__TANDEM) !defined(__QNX__) !defined(__MirBSD__) \ + !defined(__CYGWIN__) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */ #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif ... which fixed up the build for me. However, I haven't run any tests yet. Also, I would need to check this out on 32-bit cygwin (I haven't booted that laptop into Win XP for quite some time!). Hmm, I don't really know if this is an unintended side-effect of a recent change to cygwin (or a bug), but I couldn't see any mention of this on the cygwin mailing list. (I don't intend to report this to that mailing list; I don't want to subscribe to (yet another) busy list). :( ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: accept any ref for merge
I see this patch has not been picked up. I would like to lobby for inclusion of this patch. On Sep 19, 2014, at 11:22, Junio C Hamano wrote: Johan Herland jo...@herland.net writes: On Fri, Sep 19, 2014 at 11:39 AM, Jeff King p...@peff.net wrote: On Fri, Sep 19, 2014 at 09:39:45AM +0200, Scott Chacon wrote: This patch changes the expand_notes_ref function to check for simply a leading refs/ instead of refs/notes to check if we're being passed an expanded notes reference. I think this change affects not just git notes merge, but all of the notes lookups (including just git notes show) ... Is it our future direction to set up refs/remote-notes/remote/ namespace? When cloning (without --mirror) Git now sets up a fetch spec like: +refs/heads/*:refs/remotes/origin/* It's unfortunate that it does not preserve the notion of heads and instead set it up like this: +refs/heads/*:refs/remotes/origin/heads/* In which case it would make more sense to then simply clone notes like so: +refs/notes/*:refs/remotes/origin/notes/* That would also clear the way to always fetching all remote tags into refs/remotes/remote/tags/* by default as well even if the local refs/ tags/* do not end up being updated. It seems clumsy to me to use a new remotes-notes ref namespace. What happens if Git grows the ability to store some kind of (bug) tracking ticket in refs/tickets in the future? Does Git then use refs/remote- tickets/remote to store the remote refs rather than simply refs/ remotes/remote/tickets? There are a number of applications that create refs outside of refs/ heads/* and refs/tags/*. GitHub uses refs/pull/*, should Git have a refs/remote-pull/remote/* namespace and for Gerrit refs/remote- changes/remote/* and also refs/remote-cache-automerge/remote/* (for refs/cache-automerge/*)? If so, let's not do it piecemeail in an unorganized guerrilla fashion by starting with a stealth enabler By stealth enabler I mean the removal of refs/notes/ restriction that was originally done as a safety measure to avoid mistakes of storing notes outside. The refs/remote-notes/ future direction declares that it is no longer a mistake to store notes outside refs/notes/, but that does not necessarily have to mean that anywhere under refs/ is fine. It may make more sense to be explicit with the code touched here to allow traditional refs/notes/ and the new hierarchy only. That way, we will still keep the avoid mistakes safety and enable the new layout at the same time. This is the part where I want to lobby for inclusion of this change. I do not believe it is consistent with existing Git practice to enforce restrictions like this (you can only display/edit/etc. notes under refs/notes/*). Already that's not true if you use an ugly symbolic-ref workaround, but that requires polluting your ref namespace. With all the other Git restrictions they are almost always strong guidance, not brutally enforced. Take, for example, the restriction that HEAD should be either detached or a symbolic ref to refs/heads/something. It's not absolutely enforced. If you really want to, you can use git symbolic-ref and set HEAD to somewhere else (even under refs/taga) -- and it mostly works -- `git branch` gets upset but you can commit new changes, view the log, etc. How about the guidance that pushing does not update tags even if the change would be a fast-forward? Again, not enforced, use the -f option or add an explicit refspec to the appropriate remote config. What about the restriction that `git config --get user.name` cannot end in .? (It gets magically stripped off.) That's a toughie, but if you really, really, really want to you can always `git cat-file commit HEAD temp`, add the trailing dot and then git update-ref HEAD $(git hash-object -t commit -w temp)`. Not recommended but possible. So anyway, my point is that arbitrarily forcefully restricting the operation of the various notes commands to refs/notes/* does not seem consistent with the way everything else works. The most important first step for that to happen is to make sure we are on the same page on that future direction. I personally think refs/remote-notes/remote that runs parallel to the remote tracking branch hierarchy refs/remotes/remote is a reasonable way to do this, but my words are no way final. I'd prefer refs/remotes/remote/notes for the reasons stated above. Having a refs/remote-notes/remote/* hierarchy opens the door to a proliferation of refs/remote-whatever/remote/* items in the refs namespace in the future. So in the vein of providing guidance to the user but, in the end, allowing the user to remain in control, I have gussied up Johan's suggested fix for the failing notes test into the following patch. --Kyle -- 8 -- Subject: [PATCH] t/t3308-notes-merge.sh: succeed with relaxed notes refs With the recent change to allow notes refs to
Re: [PATCHv2] add: ignore only ignored files
On Sat, Nov 22, 2014 at 03:59:12PM +0100, Torsten Bögershausen wrote: +test_expect_success 'error out when attempting to add ignored ones but add others' ' + touch a.if + test_must_fail git add a.?? + ! (git ls-files | grep \\.ig) + (git ls-files | grep a.if) +' [...] 2 small comments: Why the escaped \\.ig and the unescaped a.if ? I agree that is inconsistent, and I don't see any reason for it. The other question, this is a more general one, strikes me every time I see ! grep Should we avoid it by writing test_must_fail instead of ! ? No. The point of test_must_fail over ! is to check that not only does the command fail, but it fails with a clean exit rather than a signal death. The general philosophy is that this is useful for git (which we are testing), and not for third-party tools that we are using to check our outputs. In other words, we do not expect grep to segfault, and do not need to bother checking it. I do not think there is a real _downside_ to using test_must_fail for grep, except that it is a bit more verbose. And that describes the goal, of course; actual implementation has been less consistent. Possibly because I do not know that those instructions are written down anywhere. We usually catch such things in review these days, but there are many inconsistent spots in the existing suite. The following came into my mind when working on another grepy thing, and it may be unnecessary clumsy: test_expect_success 'error out when attempting to add ignored ones but add others' ' touch a.if test_must_fail git add a.?? git ls-files files.txt test_must_fail grep a.ig files.txt /dev/null grep a.if files.txt /dev/null rm files.txt Right, my allergic to pipes was basically advocating using a tempfile. But as noted above, it should remain ! grep here. And there is no need to redirect the output of grep, as the test suite does it already (in fact, it is preferable not to, because somebody debugging the test with -v will get more output). -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: How safe are signed git tags? Only as safe as SHA-1 or somehow safer?
On Fri, Nov 21, 2014 at 11:01:26PM +, Patrick Schleizer wrote: Yes, it is only as safe as SHA-1 in the sense that you have GPG-signed only a SHA-1 hash. If somebody can find a collision with a hash you have signed, they can substitute the colliding data for the data you signed. [..] Sounds pretty sad. Isn't this a security issue that should be fixed? Sure, for some definition of should. It's not a problem today. It may be a problem in the future. If we were designing git from scratch today, it would probably make sense to use a different hash, or to somehow parameterize the hash. But we're not starting from scratch. A change like that needs to consider a transition plan. What happens to the existing history? Do we just rewrite it all using the new hash in all object references? If so, what do we do with non-object references to sha1s (in external systems, or even partial sha1s mentioned in commit message)? What do we do with signed tags which are now invalid? Or do we graft history with the new hashes onto the old, letting the two coexist in the same repository? How do we expand the data structures to handle the extra information needed to specify the hash type? None of these problems is insurmountable, but it's going to take real work on the development side, and is going to create incompatibilities and headaches on the user side. It's probably something we'll need to deal with in the next 10-15 years, but nobody knows quite when. If you'd like to start working on it, I'd be happy to review your patches. :) But in the meantime, I don't know that anybody considers it an extremely high priority to work on, versus other fixes and features. Rather than discussing how feasible collisions in SHA-1 are... Attacks on SHA-1 are only getting worse, no? Actually, not really. I do not keep up terribly well with the academic literature, but I don't think that attacks have gotten any worse in the last few years. Computers _are_ getting faster, though, so a number like 2^61 (which is what Wikipedia claims as the best widely accepted value for producing a collision) gets more and more feasible as time passes. Of course, we might find worse attacks (or if you want to put on your tinfoil hat, perhaps certain government organizations already have and are keeping them secret). 2^61 is a best case. And of course there is the question of getting the colliding data to the victim. Git does collision checks whenever a remote (e.g., from a git fetch) gives us data that we already have. So you could poison new cloners with bad data, but you could not convince a repository with the existing good half of the collision to fetch the evil half. Poison git cloners with bad data is exactly my point here. Because sometimes I am a cloner of my own code - cloning it on a separate machine - then verify it using gpg - but don't check it any further. In such cases, I'd prefer if security wouldn't depend on SHA-1. I agree that cloners are an important category of users to clone. But it also means that a single fetcher can detect tampering quite easily. Think about it this way: let's say the Walker/Schneier estimate is right, and in 2021 it will cost ~$43K to find a collision. You spend the money, find a collision on some binary blob that's in the kernel, convince Linus to accept your good version, he signs, and then you hack into kernel.org and replace the blob with your evil version. Now the first time somebody fetches the evil version, their git complains about the collision, kernel.org admins investigate, and the problem is fixed. There's some damage, but ultimately you didn't accomplish much. Or you could spend that $43K hiring somebody to break into Linus's house and manipulate the local copy of the kernel on his computer that he's going to sign. Or buy a zero-day exploit for his browser that gives you remote code execution on his workstation. Don't get me wrong. I think moving away from SHA-1 is a good idea, and something we're going to want to do for security reasons eventually. But we're definitely not at the point of well, all of our signatures are worthless now, and I'm not sure we'll be there sooner than a decade from now. -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: How safe are signed git tags? Only as safe as SHA-1 or somehow safer?
On Fri, Nov 21, 2014 at 06:32:46PM -0500, Jason Pyeron wrote: The whole issue is a lot better than this makes it sound. Yes it is just a SHA1 hash, but it is a hash of a structured data format. You have very observable parts of that well structured data providede to the hash. Yeah, I glossed over that because I don't know enough about the specific attacks. In the worst case, you have a binary file format that lets people stick arbitrary bits of data in the middle (like the MD5 attacks on Postscript and PDF files), and you do the collision on the blobs. But even with that, the sha1s are taken over blob n\0content where n is the number of bytes of content. Depending on the exact scheme for generating probable collisions is less than brute force time, even that amount of structure may prove problematic. I don't know whether that is the case for the best-known attacks or not (remember that nobody has _actually_ generated a sha-1 collision at all yet). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] prune_remote(): exit early if there are no stale references
Michael Haggerty wrote: Aside from making the logic clearer, this avoids a call to warn_dangling_symrefs(), which always does a for_each_rawref() iteration. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/remote.c | 39 +-- 1 file changed, 21 insertions(+), 18 deletions(-) I had been wondering about this but didn't chase it down far enough. Thanks for noticing and cleaning it up. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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 3/6] prune_remote(): sort delete_refs_list references en masse
Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) This and 2/6 are also Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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 4/6] repack_without_refs(): make the refnames argument a string_list
Michael Haggerty wrote:
All of the callers have string_lists available already
Technically ref_transaction_commit doesn't, but that doesn't matter.
Suggested-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/remote.c | 14 ++
refs.c | 38 --
refs.h | 11 ++-
3 files changed, 32 insertions(+), 31 deletions(-)
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
One (optional) nit at the bottom of this message.
[...]
+++ b/refs.c
@@ -2639,22 +2639,25 @@ static int curate_packed_ref_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
-int repack_without_refs(const char **refnames, int n, struct strbuf *err)
+int repack_without_refs(struct string_list *refnames, struct strbuf *err)
{
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
- struct string_list_item *ref_to_delete;
- int i, ret, removed = 0;
+ struct string_list_item *refname, *ref_to_delete;
+ int ret, needs_repacking = 0, removed = 0;
assert(err);
/* Look for a packed ref */
- for (i = 0; i n; i++)
- if (get_packed_ref(refnames[i]))
+ for_each_string_list_item(refname, refnames) {
+ if (get_packed_ref(refname-string)) {
+ needs_repacking = 1;
break;
+ }
+ }
/* Avoid locking if we have nothing to do */
- if (i == n)
+ if (!needs_repacking)
This makes me wish C supported something like Python's for/else
construct. Oh well. :)
[...]
+++ b/refs.h
@@ -163,7 +163,16 @@ extern void rollback_packed_refs(void);
*/
int pack_refs(unsigned int flags);
-extern int repack_without_refs(const char **refnames, int n,
+/*
+ * Remove the refs listed in 'refnames' from the packed-refs file.
+ * On error, packed-refs will be unchanged, the return value is
+ * nonzero, and a message about the error is written to the 'err'
+ * strbuf.
+ *
+ * The refs in 'refnames' needn't be sorted. The err buffer must not be
+ * omitted.
(nit)
s/buffer/strbuf/, or s/The err buffer/'err'/
s/omitted/NULL/
Thanks,
Jonathan
--
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 5/6] prune_remote(): rename local variable
Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/remote.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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 6/6] prune_remote(): iterate using for_each_string_list_item()
Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/remote.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com (That makes 6/6. :)) Thanks for your thoughtfulness in putting these together. They were pleasant to read. -- 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: [PATCHv2] add: ignore only ignored files
On 2014-11-22 20.19, Jeff King wrote: On Sat, Nov 22, 2014 at 03:59:12PM +0100, Torsten Bögershausen wrote: +test_expect_success 'error out when attempting to add ignored ones but add others' ' + touch a.if + test_must_fail git add a.?? + ! (git ls-files | grep \\.ig) + (git ls-files | grep a.if) +' [...] 2 small comments: Why the escaped \\.ig and the unescaped a.if ? I agree that is inconsistent, and I don't see any reason for it. The other question, this is a more general one, strikes me every time I see ! grep Should we avoid it by writing test_must_fail instead of ! ? No. The point of test_must_fail over ! is to check that not only does the command fail, but it fails with a clean exit rather than a signal death. The general philosophy is that this is useful for git (which we are testing), and not for third-party tools that we are using to check our outputs. In other words, we do not expect grep to segfault, and do not need to bother checking it. I do not think there is a real _downside_ to using test_must_fail for grep, except that it is a bit more verbose. We may burn CPU cycles And that describes the goal, of course; actual implementation has been less consistent. Possibly because I do not know that those instructions are written down anywhere. There is a hint in test-lib-functions.sh, but a short notice in CodingGuidelines could be useful, once there is an agreement about grep, which I think we have. We usually catch such things in review these days, but there are many inconsistent spots in the existing suite. The following came into my mind when working on another grepy thing, and it may be unnecessary clumsy: test_expect_success 'error out when attempting to add ignored ones but add others' ' touch a.if test_must_fail git add a.?? git ls-files files.txt test_must_fail grep a.ig files.txt /dev/null grep a.if files.txt /dev/null rm files.txt Right, my allergic to pipes was basically advocating using a tempfile. But as noted above, it should remain ! grep here. And there is no need to redirect the output of grep, as the test suite does it already (in fact, it is preferable not to, because somebody debugging the test with -v will get more output). -Peff I counted 19 test_must_fail grep under t/*sh, and 201 ! grep. As a general rule for further review of shell scripts can we say ? ! git# incorrect, we don't capture e.g. segfaults of signal test_must_fail grep # correct, but not preferred for new code ! grep # preferred for new code test_must_fail git # correct -- 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: IT-HELPDESK
This Is To Inform All Web-mail Account Users, That The Web-mail Admin Is Currently Congested, So We Are Deleting Inactive Accounts. Please Notify That This Account Is Active By Verifying It Below CLICK HEREhttp://update00admin.coffeecup.com/forms/adminhelpdesk/ ©2014 Web-mail portal Verification Center View our Disclaimerhttp://www.itb.ie/disclaimer/disclaimer.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
