* [PATCH 4/5] ls-files: illustrate bug when recursing with relative pathspec
From: Brandon Williams @ 2017-02-24 23:50 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds
In-Reply-To: <20170224235100.52627-1-bmwill@google.com>
When using the --recurse-submodules flag with a relative pathspec which
includes "..", an error is produced inside the child process spawned for a
submodule. When creating the pathspec struct in the child, the ".." is
interpreted to mean "go up a directory" which causes an error stating that the
path ".." is outside of the repository.
While it is true that ".." is outside the scope of the submodule, it is
confusing to a user who originally invoked the command where ".." was indeed
still inside the scope of the superproject. Since the child process luanched
for the submodule has some context that it is operating underneath a
superproject, this error could be avoided.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
t/t3007-ls-files-recurse-submodules.sh | 50 ++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index a5426171d..d8ab10866 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -188,6 +188,56 @@ test_expect_success '--recurse-submodules and pathspecs' '
test_cmp expect actual
'
+test_expect_failure '--recurse-submodules and relative paths' '
+ # From top works
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ h.txt
+ sib/file
+ sub/file
+ submodule/.gitmodules
+ submodule/c
+ submodule/f.TXT
+ submodule/g.txt
+ submodule/subsub/d
+ submodule/subsub/e.txt
+ EOF
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual &&
+
+ # Relative path to top errors out
+ cat >expect <<-\EOF &&
+ ../.gitmodules
+ ../a
+ b
+ ../h.txt
+ ../sib/file
+ ../sub/file
+ ../submodule/.gitmodules
+ ../submodule/c
+ ../submodule/f.TXT
+ ../submodule/g.txt
+ ../submodule/subsub/d
+ ../submodule/subsub/e.txt
+ EOF
+ git -C b ls-files --recurse-submodules -- .. >actual &&
+ test_cmp expect actual &&
+
+ # Relative path to submodule errors out
+ cat >expect <<-\EOF &&
+ ../submodule/.gitmodules
+ ../submodule/c
+ ../submodule/f.TXT
+ ../submodule/g.txt
+ ../submodule/subsub/d
+ ../submodule/subsub/e.txt
+ EOF
+ git -C b ls-files --recurse-submodules -- ../submodule >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '--recurse-submodules does not support --error-unmatch' '
test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual &&
test_i18ngrep "does not support --error-unmatch" actual
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: SHA1 collisions found
From: Ian Jackson @ 2017-02-25 0:06 UTC (permalink / raw)
To: Junio C Hamano, Joey Hess, git
In-Reply-To: <22704.50445.435156.883001@chiark.greenend.org.uk>
Ian Jackson writes ("Re: SHA1 collisions found"):
> The idea of using the length is a neat trick, but it cannot support
> the dcurrent object name abbreviation approach unworkable.
Sorry, it's late here and my grammar seems to have disintegrated !
Ian.
^ permalink raw reply
* Re: SHA1 collisions found
From: ankostis @ 2017-02-25 0:31 UTC (permalink / raw)
To: Junio C Hamano, git
Cc: Stefan Beller, David Lang, Ian Jackson, Joey Hess, git
In-Reply-To: <xmqqh93j1g9n.fsf@gitster.mtv.corp.google.com>
On 24 February 2017 at 21:32, Junio C Hamano <gitster@pobox.com> wrote:
> ankostis <ankostis@gmail.com> writes:
>
>> Let's assume that git is retroffited to always support the "default"
>> SHA-3, but support additionally more hash-funcs.
>> If in the future SHA-3 also gets defeated, it would be highly unlikely
>> that the same math would also break e.g. Blake.
>> So certain high-profile repos might choose for extra security 2 or more hashes.
>
> I think you are conflating two unrelated things.
I believe the two distinct things you refer to below are these:
a. storing objects in filesystem and accessing them
by name (e.g. from cmdline), and
b. cross-referencing inside the objects (trees, tags, notes),
correct?
If not, then please ignore my answers, below.
> * How are these "2 or more hashes" actually used? Are you going to
> add three "parent " line to a commit with just one parent, each
> line storing the different hashes?
Yes, in all places where references are involved (tags, notes).
Based on what what the git-hackers have written so far, this might be doable.
To ensure integrity in the case of crypto-failures, all objects must
cross-reference each other with multiple hashes.
Of course this extra security would stop as soon as you reach "old"
history (unless you re-write it).
> How will such a commit object
> be named---does it have three names and do you plan to have three
> copies of .git/refs/heads/master somehow, each of which have
> SHA-1, SHA-3 and Blake, and let any one hash to identify the
> object?
Yes, based on Jason Cooper's idea, above, objects would be stored
under all names in the filesystem using hard links (although this
might not work nice on Windows).
> I suspect you are not going to do so; instead, you would use a
> very long string that is a concatenation of these three hashes as
> if it is an output from a single hash function that produces a
> long result.
>
> So I think the most natural way to do the "2 or more for extra
> security" is to allow us to use a very long hash. It does not
> help to allow an object to be referred to with any of these 2 or
> more hashes at the same time.
If hard-linking all names is doable, then most restrictions above are
gone, correct?
> * If employing 2 or more hashes by combining into one may enhance
> the security, that is wonderful. But we want to discourage
> people from inventing their own combinations left and right and
> end up fragmenting the world. If a project that begins with
> SHA-1 only naming is forked to two (or more) and each fork uses
> different hashes, merging them back will become harder than
> necessary unless you support all these hashes forks used.
Agree on discouraging people's inventions.
That is why I believe that some HASH (e.g. SHA-3) must be the blessed one.
All git >= 3.x.x must support at least this one (for naming and
cross-referencing between objects).
> Having said all that, the way to figure out the hash used in the way
> we spell the object name may not be the best place to discourage
> people from using random hashes of their choice. But I think we
> want to avoid doing something that would actively encourage
> fragmentation.
I guess the "blessed SHA-3 will discourage people using the other
names., untill the next crypto-crack.
^ permalink raw reply
* Re: [PATCH 2/5] pathspec: add PATHSPEC_FROMROOT flag
From: Stefan Beller @ 2017-02-25 0:31 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Duy Nguyen
In-Reply-To: <20170224235100.52627-3-bmwill@google.com>
On Fri, Feb 24, 2017 at 3:50 PM, Brandon Williams <bmwill@google.com> wrote:
> Add the `PATHSPEC_FROMROOT` flag to allow callers to instruct
> 'parse_pathspec()' that all provided pathspecs are relative to the root
> of the repository. This allows a caller to prevent a path that may be
> outside of the repository from erroring out during the pathspec struct
> construction.
>
> +/* For callers that know all paths are relative to the root of the repository */
> +#define PATHSPEC_FROMROOT (1<<9)
What is the calling convention for these submodule pathspecs?
IIRC, we'd pass on the super-prefix and the literal pathspec,
e.g. when there is a submodule "sub" inside the superproject,
The invocation on the submodule would be
git FOO --super-prefix="sub" <arguments> -- sub/pathspec/inside/...
and then the submodule process would need to "subtract" the superprefix
from the pathspec argument to see its actual pathspec, e.g. in gerrit:
$ GIT_TRACE=1 git grep --recurse-submodules -i test -- \
plugins/cookbook-plugin/
...
trace: run_command: '--super-prefix=plugins/cookbook-plugin/' 'grep' \
'--recurse-submodules' '-i' '-etest' '--threads=4' '--'
'plugins/cookbook-plugin/'
..
but also:
...
trace: run_command: '--super-prefix=plugins/download-commands/' 'grep' \
'--recurse-submodules' '-i' '-etest' '--threads=4' '--'
'plugins/cookbook-plugin/'
...
So if I change into a directory:
$ cd plugins
plugins$ git grep --recurse-submodules -i test -- cookbook-plugin/
plugins$ #empty?
plugins$ git grep --recurse-submodules -i test -- plugins/cookbook-plugin/
...
Usual output, so the pathspecs are absolute path to the superprojects
root? Let's try relative path:
plugins$ git grep --recurse-submodules -i test -- ../plugins/cookbook-plugin/
fatal: ../plugins/cookbook-plugin/: '../plugins/cookbook-plugin/' is
outside repository
...
Running with GIT_TRACE=1:
trace: run_command: '--super-prefix=plugins/cookbook-plugin/' 'grep' \
'--recurse-submodules' '-i' '-etest' '--threads=4' '--'
'../plugins/cookbook-plugin/'
that seems like a mismatch of pathspec and superproject prefix, the prefix ought
to be different then? Maybe also including ../ because that is the
relative path from
cwd to the superporojects root and that is where we anchor all paths?
Easy to test that out:
plugins$ GIT_TRACE=1 git --super-prefix=../ grep --recurse-submodules \
-i test -- ../plugins/cookbook-plugin/
fatal: can't use --super-prefix from a subdirectory
ok, not as easy. :/
So another test with relative path:
(in git.git)
cd t/diff-lib
t/diff-lib$ git grep freedom
COPYING:freedom to share and change it. By contrast, the GNU General Public
...
So the path displayed is relative to the cwd (and the search results as well)
In the submodule case we would expect to have the super prefix
to be computed to be relative to the cwd?
Checking the tests, this is handled correctly with this patch series. :)
But nevertheless, I think I know why I dislike this approach now:
The super prefix is handled "too dumb" IMHO, see the case
plugins$ git grep test -- cookbook-plugin/
above, that doesn't correctly figure out the correct output.
Although this might be a separate bug, but it sounds like it
is the same underlying issue.
--
for the naming: How about PATHSPEC_FROMOUTSIDE
when going with the series as is here?
(the superprefix is not resolved, so the pathspecs given are
literally pathspecs that are outside this repo and we can ignore
them?
Thanks,
Stefan
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-25 0:39 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, Git Mailing List
In-Reply-To: <20170224233929.p2yckbc6ksyox5nu@sigill.intra.peff.net>
On Fri, Feb 24, 2017 at 3:39 PM, Jeff King <peff@peff.net> wrote:
>
> One thing I worry about in a mixed-hash setting is how often the two
> will be mixed.
Honestly, I think that a primary goal for a new hash implementation
absolutely needs to be to minimize mixing.
Not for security issues, but because of combinatorics. You want to
have a model that basically reads old data, but that very aggressively
approaches "new data only" in order to avoid the situation where you
have basically the exact same tree state, just _represented_
differently.
For example, what I would suggest the rules be is something like this:
- introduce new tag2/commit2/tree2/blob2 object type tags that imply
that they were hashed using the new hash
- an old type obviously can never contain a pointer to a new type (ie
you can't have a "tree" object that contains a tree2 object or a blob2
object.
- but also make the rule that a *new* type can never contain a
pointer to an old type, with the *very* specific exception that a
commit2 can have a parent that is of type "commit".
That way everything "converges" towards the new format: the only way
you can stay on the old format is if you only have old-format objects,
and once you have a new-format object all your objects are going to be
new format - except for the history.
Obviously, if somebody stays in old format, you might end up still
getting some object duplication when you continue to merge from him,
but that tree can never merge back without converting to new-format,
so it will be a temporary situation.
So you will end up with duplicate objects, and that's not good (think
of what it does to all our full-tree "diff" optimizations, for example
- you no longer get the "these sub-trees are identical" across a
format change), but realistically you'll have a very limited time of
that kind of duplication.
I'd furthermore suggest that from a UI standpoint, we'd
- convert to 64-character hex numbers (32-byte hashes)
- (as mentioned earlier) default to a 40-character abbreviation
- make the old 40-character SHA1's just show up within the same
address space (so they'd also be encoded as 32-byte hashes, just with
the last 12 bytes zero).
- you'd see in the "object->type" whether it's a new or old-style hash.
I suspect it shouldn't be too painful to do it that way.
Linus
^ permalink raw reply
* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Linus Torvalds @ 2017-02-25 0:44 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Git Mailing List, Jeff Hostetler,
Junio C Hamano
In-Reply-To: <20170210050237.gajicliueuvk6s5d@sigill.intra.peff.net>
On Thu, Feb 9, 2017 at 9:02 PM, Jeff King <peff@peff.net> wrote:
>
> I think this is only half the story. A heavy-sha1 workload is faster,
> which is good. But one of the original reasons to prefer blk-sha1 (at
> least on Linux) is that resolving libcrypto.so symbols takes a
> non-trivial amount of time. I just timed it again, and it seems to be
> consistently 1ms slower to run "git rev-parse --git-dir" on my machine
> (from the top-level of a repo).
Yes. It's also a horrible plain to profile those things.
Avoiding openssl was a great thing, because it avoided a lot of crazy overhead.
I suspect that most of the openssl win comes from using the actual SHA
instructions on modern CPU's. Because last I looked, the hand-coded
assembly simply wasn't that much faster.
We could easily have some x86-specific library that just does "use SHA
instructions if you have them, use blk-sha1 otherwise".
Of course, if we end up using the collision checking SHA libraries
this is all moot anyway.
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-25 0:54 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, Git Mailing List
In-Reply-To: <CA+55aFw6BLjPK-F0RGd9LT7X5xosKOXOxuhmKX65ZHn09r1xow@mail.gmail.com>
On Fri, Feb 24, 2017 at 4:39 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> - you'd see in the "object->type" whether it's a new or old-style hash.
Actually, I take that back. I think it might be easier to keep
"object->type" as-is, and it would only show the current OBJ_xyz
fields. Then writing the SHA ends up deciding whether a OBJ_COMMIT
gets written as "commit" or "commit2".
With the reachability rules, you'd never have any ambiguity about which to use.
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: David Lang @ 2017-02-25 1:00 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, git
In-Reply-To: <20170224233929.p2yckbc6ksyox5nu@sigill.intra.peff.net>
On Fri, 24 Feb 2017, Jeff King wrote:
>
> So I'd much rather see strong rules like:
>
> 1. Once a repo has flag-day switched over to the new hash format[1],
> new references are _always_ done with the new hash. Even ones that
> point to pre-flag-day objects!
how do you define when a repo has "switched over" to the new format in a
distributed environment?
so you have one person working on a project that switches their version of git
to the new one that uses the new format.
But other people they interact with still use older versions of git
what happens when you have someone working on two different projects where one
has switched and the other hasn't?
what if they are forks of each other? (LEDE and OpenWRT, or just linux-kernel
and linux-kernel-stable)
> So you get a "commit-v2" object instead of a "commit", and it has a
> distinct hash identity from its "commit" counterpart. You can point
> to a classic "commit", but you do so by its new-hash.
>
> The flag-day switch would probably be a repo config flag based on
> repositoryformatversion (so old versions would just punt if they
> see it). Let's call this flag "newhash" for lack of a better term.
so how do you interact with someone who only expects the old commit instead of
the commit-v2?
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Stefan Beller @ 2017-02-25 1:15 UTC (permalink / raw)
To: David Lang
Cc: Jeff King, Junio C Hamano, Ian Jackson, Joey Hess,
git@vger.kernel.org
In-Reply-To: <nycvar.QRO.7.75.62.1702241656010.6590@qynat-yncgbc>
On Fri, Feb 24, 2017 at 5:00 PM, David Lang <david@lang.hm> wrote:
> On Fri, 24 Feb 2017, Jeff King wrote:
>
>>
>> So I'd much rather see strong rules like:
>>
>> 1. Once a repo has flag-day switched over to the new hash format[1],
>> new references are _always_ done with the new hash. Even ones that
>> point to pre-flag-day objects!
>
>
> how do you define when a repo has "switched over" to the new format in a
> distributed environment?
>
> so you have one person working on a project that switches their version of
> git to the new one that uses the new format.
>
> But other people they interact with still use older versions of git
>
> what happens when you have someone working on two different projects where
> one has switched and the other hasn't?
you get infected by the "new version requirement"
as soon as you pull? (GPL is cancer, anyone? ;)
If you are using an old version of git that doesn't understand the new version,
you're screwed.
^ permalink raw reply
* Re: SHA1 collisions found
From: Jeff King @ 2017-02-25 1:16 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, Git Mailing List
In-Reply-To: <CA+55aFw6BLjPK-F0RGd9LT7X5xosKOXOxuhmKX65ZHn09r1xow@mail.gmail.com>
On Fri, Feb 24, 2017 at 04:39:45PM -0800, Linus Torvalds wrote:
> For example, what I would suggest the rules be is something like this:
>
> - introduce new tag2/commit2/tree2/blob2 object type tags that imply
> that they were hashed using the new hash
>
> - an old type obviously can never contain a pointer to a new type (ie
> you can't have a "tree" object that contains a tree2 object or a blob2
> object.
>
> - but also make the rule that a *new* type can never contain a
> pointer to an old type, with the *very* specific exception that a
> commit2 can have a parent that is of type "commit".
Yeah, this is exactly what I had in mind. That way everybody in
"newhash" mode has no decisions to make. They follow the same rules and
it's as if sha1 never existed, except when you follow links in
historical objects.
> [in reply...]
> Actually, I take that back. I think it might be easier to keep
> "object->type" as-is, and it would only show the current OBJ_xyz
> fields. Then writing the SHA ends up deciding whether a OBJ_COMMIT
> gets written as "commit" or "commit2".
Yeah, I think there are some data structures with limited bits for the
"type" fields (e.g., the pack format). So sticking with OBJ_COMMIT might
be nice. For commits and tags, it would be nice to have an "I'm v2"
header at the start so there's no confusion about how they are meant to
be interpreted.
Trees are more difficult, as they don't have any such field. But a valid
tree does need to start with a mode, so sticking some non-numeric flag
at the front of the object would work (it breaks backwards
compatibility, but that's kind of the point).
I dunno. Maybe we do not need those markers at all, and could get by
purely on object-length, or annotating the headers in some way (like
"parent sha256:1234abcd").
It might just be nice if we could very easily identify objects as one
type or the other without having to parse them in detail.
> So you will end up with duplicate objects, and that's not good (think
> of what it does to all our full-tree "diff" optimizations, for example
> - you no longer get the "these sub-trees are identical" across a
> format change), but realistically you'll have a very limited time of
> that kind of duplication.
Yeah, cross-flag-day diffs will be more expensive. I think that's
something we have to live with. I was thinking originally that the
sha1->newhash mapping might solve that, but it only works at the blob
level. I.e., you can compare a sha1 and a newhash like:
if (!hashcmp(sha1_to_newhash(a), b))
without having to look at the contents. But it doesn't work recursively,
because the tree-pointing-to-newhash will have different content.
-Peff
^ permalink raw reply
* [PATCH 2/3] revision: exclude trees/blobs given commit
From: Jonathan Tan @ 2017-02-25 1:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, peff, peartben, benpeart
In-Reply-To: <cover.1487984670.git.jonathantanmy@google.com>
When the --objects argument is given to rev-list, an argument of the
form "^$tree" can be given to exclude all blobs and trees reachable from
that tree, but an argument of the form "^$commit" only excludes that
commit, not any blob or tree reachable from it. Make "^$commit" behave
consistent to "^$tree".
Also, formalize this behavior in unit tests. (Some of the added tests
would already pass even before this commit, but are included
nevertheless for completeness.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
revision.c | 2 ++
t/t6000-rev-list-misc.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+)
diff --git a/revision.c b/revision.c
index 5e49d9e0e..e6a62da98 100644
--- a/revision.c
+++ b/revision.c
@@ -254,6 +254,8 @@ static struct commit *handle_commit(struct rev_info *revs,
die("unable to parse commit %s", name);
if (flags & UNINTERESTING) {
mark_parents_uninteresting(commit);
+ if (revs->tree_and_blob_objects)
+ mark_tree_uninteresting(commit->tree);
revs->limited = 1;
}
if (revs->show_source && !commit->util)
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 969e4e9e5..c04a9582b 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -114,4 +114,92 @@ test_expect_success '--header shows a NUL after each commit' '
test_cmp expect actual
'
+test_expect_success 'setup tree-granularity and blob-granularity tests' '
+ echo 0 >file &&
+
+ mkdir subdir &&
+ echo 1 >subdir/file &&
+ git add file subdir/file &&
+ git commit -m one &&
+
+ echo 2 >subdir/file &&
+ git add subdir/file &&
+ git commit -m two &&
+
+ commit1=$(git rev-parse HEAD^1) &&
+ commit2=$(git rev-parse HEAD) &&
+ tree1=$(git rev-parse $commit1^{tree}) &&
+ tree2=$(git rev-parse $commit2^{tree}) &&
+ subtree1=$(git cat-file -p $tree1 | grep "subdir" | cut -c13-52) &&
+ subtree2=$(git cat-file -p $tree2 | grep "subdir" | cut -c13-52) &&
+ blob0=$(echo 0 | git hash-object --stdin) &&
+ blob1=$(echo 1 | git hash-object --stdin) &&
+ blob2=$(echo 2 | git hash-object --stdin)
+'
+
+test_expect_success 'include commit, exclude blob' '
+ git rev-list --objects $commit2 >out &&
+ grep "$blob1" out &&
+ grep "$blob2" out &&
+
+ git rev-list --objects $commit2 "^$blob2" >out &&
+ grep "$blob1" out &&
+ ! grep "$blob2" out
+'
+
+test_expect_success 'include commit, exclude tree (also excludes nested trees/blobs)' '
+ git rev-list --objects $commit2 "^$tree2" >out &&
+ grep "$tree1" out &&
+ grep "$subtree1" out &&
+ grep "$blob1" out &&
+ ! grep "$tree2" out &&
+ ! grep "$subtree2" out &&
+ ! grep "$blob2" out &&
+
+ git rev-list --objects $commit2 "^$subtree2" >out &&
+ grep "$tree1" out &&
+ grep "$subtree1" out &&
+ grep "$blob1" out &&
+ grep "$tree2" out &&
+ ! grep "$subtree2" out &&
+ ! grep "$blob2" out
+'
+
+test_expect_success 'include tree, exclude commit' '
+ git rev-list --objects "$tree1" "^$commit2" >out &&
+ ! grep "$blob0" out && # common to both
+ grep "$blob1" out && # only in tree
+ ! grep "$blob2" out # only in commit
+'
+
+test_expect_success 'include tree, exclude tree' '
+ git rev-list --objects "$tree1" "^$tree2" >out &&
+ ! grep "$blob0" out && # common to both
+ grep "$blob1" out && # only in tree1
+ ! grep "$blob2" out # only in tree2
+'
+
+test_expect_success 'include tree, exclude blob' '
+ git rev-list --objects "$tree1" "^$blob2" >out &&
+ grep "$blob0" out &&
+ grep "$blob1" out &&
+ ! grep "$blob2" out
+'
+
+test_expect_success 'include blob, exclude commit' '
+ git rev-list --objects "$blob2" "^$commit1" >out &&
+ grep "$blob2" out &&
+
+ git rev-list --objects "$blob2" "^$commit2" >out &&
+ ! grep "$blob2" out
+'
+
+test_expect_success 'include blob, exclude tree' '
+ git rev-list --objects "$blob2" "^$tree1" >out &&
+ grep "$blob2" out &&
+
+ git rev-list --objects "$blob2" "^$tree2" >out &&
+ ! grep "$blob2" out
+'
+
test_done
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 0/3] Test fetch-pack's ability to fetch arbitrary blobs
From: Jonathan Tan @ 2017-02-25 1:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, peff, peartben, benpeart
As stated in a previous e-mail [1], I was trying to think a way to allow
Git to fetch arbitrary blobs from another Git server, and it turned out
that fetch-pack already can. However, there were some bugs with blob
reachability. This patch set fixes those bugs, and verifies (with tests)
that fetch-pack can fetch reachable blobs and cannot fetch unreachable
blobs.
These patches are (I think) worthwhile on their own, but may be of
special interest to people who need Git to tolerate missing objects in
the local repo (for example, the e-mail discussion "[RFC] Add support
for downloading blobs on demand" [2]) because a way for Git to download
missing objects natively is (I think) a prerequisite to that.
[1] <20170223230358.30050-1-jonathantanmy@google.com>
[2] <20170113155253.1644-1-benpeart@microsoft.com>
Jonathan Tan (3):
revision: unify {tree,blob}_objects in rev_info
revision: exclude trees/blobs given commit
upload-pack: compute blob reachability correctly
bisect.c | 2 +-
builtin/rev-list.c | 6 ++--
list-objects.c | 4 +--
pack-bitmap-write.c | 3 +-
pack-bitmap.c | 3 +-
reachable.c | 3 +-
revision.c | 18 +++++-----
revision.h | 3 +-
t/t5500-fetch-pack.sh | 30 +++++++++++++++++
t/t6000-rev-list-misc.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
upload-pack.c | 15 +++++++++
11 files changed, 151 insertions(+), 24 deletions(-)
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply
* [PATCH 1/3] revision: unify {tree,blob}_objects in rev_info
From: Jonathan Tan @ 2017-02-25 1:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, peff, peartben, benpeart
In-Reply-To: <cover.1487984670.git.jonathantanmy@google.com>
Whenever tree_objects is set to 1 in revision.h's struct rev_info,
blob_objects is likewise set, and vice versa. Combine those two fields
into one.
Some of the existing code does not handle tree_objects being different
from blob_objects properly. For example, "handle_commit" in revision.c
recurses from an UNINTERESTING tree into its subtree if tree_objects ==
1, completely ignoring blob_objects; it probably should still recurse if
tree_objects == 0 and blob_objects == 1 (to mark the blobs), and should
behave differently depending on blob_objects (controlling the
instantiation and marking of blob objects). This commit resolves the
issue by forbidding tree_objects from being different to blob_objects.
It could be argued that in the future, Git might need to distinguish
tree_objects from blob_objects - in particular, a user might want
rev-list to print the trees but not the blobs. However, this results in
a minor performance savings at best in that objects no longer need to be
instantiated (causing memory allocations and hashtable insertions) - no
disk reads are being done for objects whether blob_objects is set or
not.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
bisect.c | 2 +-
builtin/rev-list.c | 6 +++---
list-objects.c | 4 ++--
pack-bitmap-write.c | 3 +--
pack-bitmap.c | 3 +--
reachable.c | 3 +--
revision.c | 16 ++++++----------
revision.h | 3 +--
8 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/bisect.c b/bisect.c
index 8e63c40d2..265b32905 100644
--- a/bisect.c
+++ b/bisect.c
@@ -634,7 +634,7 @@ static void bisect_common(struct rev_info *revs)
{
if (prepare_revision_walk(revs))
die("revision walk setup failed");
- if (revs->tree_objects)
+ if (revs->tree_and_blob_objects)
mark_edges_uninteresting(revs, NULL);
}
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d589..6c2651b31 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -345,7 +345,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
revs.commit_format = CMIT_FMT_RAW;
if ((!revs.commits &&
- (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
+ (!(revs.tag_objects || revs.tree_and_blob_objects) &&
!revs.pending.nr)) ||
revs.diff)
usage(rev_list_usage);
@@ -374,7 +374,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
return 0;
}
} else if (revs.max_count < 0 &&
- revs.tag_objects && revs.tree_objects && revs.blob_objects) {
+ revs.tag_objects && revs.tree_and_blob_objects) {
if (!prepare_bitmap_walk(&revs)) {
traverse_bitmap_commit_list(&show_object_fast);
return 0;
@@ -384,7 +384,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
- if (revs.tree_objects)
+ if (revs.tree_and_blob_objects)
mark_edges_uninteresting(&revs, show_edge);
if (bisect_list) {
diff --git a/list-objects.c b/list-objects.c
index f3ca6aafb..796957105 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -18,7 +18,7 @@ static void process_blob(struct rev_info *revs,
struct object *obj = &blob->object;
size_t pathlen;
- if (!revs->blob_objects)
+ if (!revs->tree_and_blob_objects)
return;
if (!obj)
die("bad blob object");
@@ -78,7 +78,7 @@ static void process_tree(struct rev_info *revs,
all_entries_interesting: entry_not_interesting;
int baselen = base->len;
- if (!revs->tree_objects)
+ if (!revs->tree_and_blob_objects)
return;
if (!obj)
die("bad tree object");
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 970559601..5080e276b 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -258,8 +258,7 @@ void bitmap_writer_build(struct packing_data *to_pack)
init_revisions(&revs, NULL);
revs.tag_objects = 1;
- revs.tree_objects = 1;
- revs.blob_objects = 1;
+ revs.tree_and_blob_objects = 1;
revs.no_walk = 0;
revs.include_check = should_include;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 39bcc1684..445a24e0d 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -951,8 +951,7 @@ void test_bitmap_walk(struct rev_info *revs)
die("Commit %s doesn't have an indexed bitmap", oid_to_hex(&root->oid));
revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
+ revs->tree_and_blob_objects = 1;
result_popcnt = bitmap_popcount(result);
diff --git a/reachable.c b/reachable.c
index d0199cace..9a3beb75f 100644
--- a/reachable.c
+++ b/reachable.c
@@ -166,8 +166,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
* in all object types, not just commits.
*/
revs->tag_objects = 1;
- revs->blob_objects = 1;
- revs->tree_objects = 1;
+ revs->tree_and_blob_objects = 1;
/* Add all refs from the index file */
add_index_objects_to_pending(revs, 0);
diff --git a/revision.c b/revision.c
index b37dbec37..5e49d9e0e 100644
--- a/revision.c
+++ b/revision.c
@@ -267,7 +267,7 @@ static struct commit *handle_commit(struct rev_info *revs,
*/
if (object->type == OBJ_TREE) {
struct tree *tree = (struct tree *)object;
- if (!revs->tree_objects)
+ if (!revs->tree_and_blob_objects)
return NULL;
if (flags & UNINTERESTING) {
mark_tree_contents_uninteresting(tree);
@@ -281,7 +281,7 @@ static struct commit *handle_commit(struct rev_info *revs,
* Blob object? You know the drill by now..
*/
if (object->type == OBJ_BLOB) {
- if (!revs->blob_objects)
+ if (!revs->tree_and_blob_objects)
return NULL;
if (flags & UNINTERESTING)
return NULL;
@@ -1817,23 +1817,19 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->limited = 1;
} else if (!strcmp(arg, "--objects")) {
revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
+ revs->tree_and_blob_objects = 1;
} else if (!strcmp(arg, "--objects-edge")) {
revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
+ revs->tree_and_blob_objects = 1;
revs->edge_hint = 1;
} else if (!strcmp(arg, "--objects-edge-aggressive")) {
revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
+ revs->tree_and_blob_objects = 1;
revs->edge_hint = 1;
revs->edge_hint_aggressive = 1;
} else if (!strcmp(arg, "--verify-objects")) {
revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
+ revs->tree_and_blob_objects = 1;
revs->verify_objects = 1;
} else if (!strcmp(arg, "--unpacked")) {
revs->unpacked = 1;
diff --git a/revision.h b/revision.h
index 9fac1a607..43cce137f 100644
--- a/revision.h
+++ b/revision.h
@@ -89,8 +89,7 @@ struct rev_info {
simplify_merges:1,
simplify_by_decoration:1,
tag_objects:1,
- tree_objects:1,
- blob_objects:1,
+ tree_and_blob_objects:1,
verify_objects:1,
edge_hint:1,
edge_hint_aggressive:1,
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 3/3] upload-pack: compute blob reachability correctly
From: Jonathan Tan @ 2017-02-25 1:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, peff, peartben, benpeart
In-Reply-To: <cover.1487984670.git.jonathantanmy@google.com>
If allowreachablesha1inwant is set, upload-pack will provide a blob to a
user, provided its hash, regardless of whether the blob is reachable or
not. Teach upload-pack to compute reachability correctly by passing the
"--objects" argument when it invokes "rev-list" if necessary.
This commit only affects the case where blob/tree hashes are provided to
upload-pack; the more typical case of only commit/tag hashes being
provided is not affected. In the case where blob/tree hashes are
provided, the reachability check is now slower (since trees need to be
read) but correct. (The user may still set allowanysha1inwant instead of
allowreachablesha1inwant to opt-out of the reachability check.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++
upload-pack.c | 15 +++++++++++++++
2 files changed, 45 insertions(+)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4a7..a4ae888ff 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,6 +547,36 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
'
+test_expect_success 'setup for tests that fetch blobs by hash' '
+ git init blobserver &&
+ test_commit -C blobserver 1 &&
+ test_commit -C blobserver 2 &&
+ test_commit -C blobserver 3 &&
+ blob1=$(echo 1 | git hash-object --stdin) &&
+ blob2=$(echo 2 | git hash-object --stdin) &&
+ blob3=$(echo 3 | git hash-object --stdin) &&
+
+ unreachable=$(echo 4 | git -C blobserver hash-object -w --stdin) &&
+ git -C blobserver cat-file -e "$unreachable"
+'
+
+test_expect_success 'fetch-pack can fetch reachable blobs by hash' '
+ test_config -C blobserver uploadpack.allowreachablesha1inwant 1 &&
+
+ git init reachabletest &&
+ git -C reachabletest fetch-pack ../blobserver "$blob1" "$blob2" &&
+ git -C reachabletest cat-file -e "$blob1" &&
+ git -C reachabletest cat-file -e "$blob2" &&
+ test_must_fail git -C reachabletest cat-file -e "$blob3"
+'
+
+test_expect_success 'fetch-pack cannot fetch unreachable blobs' '
+ test_config -C blobserver uploadpack.allowreachablesha1inwant 1 &&
+
+ git init unreachabletest &&
+ test_must_fail git -C unreachabletest fetch-pack ../blobserver "$blob1" "$unreachable"
+'
+
check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
diff --git a/upload-pack.c b/upload-pack.c
index 7597ba340..f05cc2b5e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -471,6 +471,9 @@ static int do_reachable_revlist(struct child_process *cmd,
static const char *argv[] = {
"rev-list", "--stdin", NULL,
};
+ static const char *argv_with_objects[] = {
+ "rev-list", "--objects", "--stdin", NULL,
+ };
struct object *o;
char namebuf[42]; /* ^ + SHA-1 + LF */
int i;
@@ -488,6 +491,18 @@ static int do_reachable_revlist(struct child_process *cmd,
*/
sigchain_push(SIGPIPE, SIG_IGN);
+ /*
+ * If we are testing reachability of a tree or blob, rev-list needs to
+ * operate more granularly.
+ */
+ for (i = 0; i < src->nr; i++) {
+ o = src->objects[i].item;
+ if (o->type == OBJ_TREE || o->type == OBJ_BLOB) {
+ cmd->argv = argv_with_objects;
+ break;
+ }
+ }
+
if (start_command(cmd))
goto error;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: SHA1 collisions found
From: Jeff King @ 2017-02-25 1:21 UTC (permalink / raw)
To: David Lang; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, git
In-Reply-To: <nycvar.QRO.7.75.62.1702241656010.6590@qynat-yncgbc>
On Fri, Feb 24, 2017 at 05:00:55PM -0800, David Lang wrote:
> On Fri, 24 Feb 2017, Jeff King wrote:
>
> >
> > So I'd much rather see strong rules like:
> >
> > 1. Once a repo has flag-day switched over to the new hash format[1],
> > new references are _always_ done with the new hash. Even ones that
> > point to pre-flag-day objects!
>
> how do you define when a repo has "switched over" to the new format in a
> distributed environment?
You don't. It's a decision for each local repo, but the rules push
everybody towards upgrading (because you forbid them pulling from or
pushing to people who have upgraded).
So in practice, some centralized distribution point switches, and then
it floods out from there.
> so you have one person working on a project that switches their version of
> git to the new one that uses the new format.
That shouldn't happen when they switch. It should happen when they
decide to move their local clone to the new format. So let's assume they
upgrade _and_ decide to switch.
> But other people they interact with still use older versions of git
Those people get forced to upgrade if they want to continue interacting.
> what happens when you have someone working on two different projects where
> one has switched and the other hasn't?
See above. You only flip the flag on for one of the projects.
> what if they are forks of each other? (LEDE and OpenWRT, or just
> linux-kernel and linux-kernel-stable)
Once one flips, the other one needs to flip to, or can't interact with
them. I know that's harsh, and is likely to create headaches. But in the
long run, I think once everything has converged the resulting system is
less insane.
For that reason I _wouldn't_ recommend projects like the kernel flip the
flag immediately. Ideally we write the code and the new versions
permeate the community. Then somebody (per-project) decides that it's
time for the community to start switching.
> > The flag-day switch would probably be a repo config flag based on
> > repositoryformatversion (so old versions would just punt if they
> > see it). Let's call this flag "newhash" for lack of a better term.
>
> so how do you interact with someone who only expects the old commit instead
> of the commit-v2?
You ask them to upgrade.
-Peff
^ permalink raw reply
* [PATCH] submodule init: warn about falling back to a local path
From: Stefan Beller @ 2017-02-25 1:31 UTC (permalink / raw)
To: j6t; +Cc: git, philipoakley, gitster, sop, Stefan Beller
In-Reply-To: <CAGZ79kbzhHaV4-cVvqwodwXSBstRfH1FrOh=iYMvU6cqYUcUng@mail.gmail.com>
When a submodule is initialized, the config variable 'submodule.<name>.url'
is set depending on the value of the same variable in the .gitmodules
file. When the URL indicates to be relative, then the url is computed
relative to its default remote. The default remote cannot be determined
accurately in all cases, such that it falls back to 'origin'.
The 'origin' remote may not exist, though. In that case we give up looking
for a suitable remote and we'll just assume it to be a local relative path.
This can be confusing to users as there is a lot of guessing involved,
which is not obvious to the user.
So in the corner case of assuming a local autoritative truth, warn the
user to lessen the confusion.
This behavior was introduced in 4d6893200 (submodule add: allow relative
repository path even when no url is set, 2011-06-06), which shared the
code with submodule-init and then ported to C in 3604242f080a (submodule:
port init from shell to C, 2016-04-15).
In case of submodule-add, this behavior makes sense in some use cases[1],
however for submodule-init there does not seem to be an immediate obvious
use case to fall back to a local submodule. However there might be, so
warn instead of die here.
While adding the warning, also clarify the behavior of relative URLs in
the documentation.
[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private
"store a secret locally in a submodule, with no intention to publish it"
Reported-by: Shawn Pearce <spearce@spearce.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
> Perhaps s/ all submodules/,&/?
done
> I read this as "copying..., resolving relative to the default remote
> (if exists)."
reworded with shorter sentences:
Initialize the submodules recorded in the index (which were
added and committed elsewhere) by setting `submodule.$name.url`
in .git/config. It uses the same setting from .gitmodules as
a template. If the URL is relative, it will be resolved using
the default remote. If there is no default remote, the current
repository will be assumed to be upstream.
...
next paragraph
Documentation/git-submodule.txt | 38 ++++++++++++++++++++++++--------------
builtin/submodule--helper.c | 8 +++-----
2 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8acc72ebb8..e05d0cddef 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.
+
<repository> is the URL of the new submodule's origin repository.
This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
+or ../), the location relative to the superproject's default remote
repository (Please note that to specify a repository 'foo.git'
which is located right next to a superproject 'bar.git', you'll
have to use '../foo.git' instead of './foo.git' - as one might expect
when following the rules for relative URLs - because the evaluation
of relative URLs in Git is identical to that of relative directories).
-If the superproject doesn't have an origin configured
++
+The default remote is the remote of the remote tracking branch
+of the current branch. If no such remote tracking branch exists or
+the HEAD is detached, "origin" is assumed to be the default remote.
+If the superproject doesn't have a default remote configured
the superproject is its own authoritative upstream and the current
working directory is used instead.
+
@@ -118,18 +122,24 @@ too (and can also report changes to a submodule's work tree).
init [--] [<path>...]::
Initialize the submodules recorded in the index (which were
- added and committed elsewhere) by copying submodule
- names and urls from .gitmodules to .git/config.
- Optional <path> arguments limit which submodules will be initialized.
- It will also copy the value of `submodule.$name.update` into
- .git/config.
- The key used in .git/config is `submodule.$name.url`.
- This command does not alter existing information in .git/config.
- You can then customize the submodule clone URLs in .git/config
- for your local setup and proceed to `git submodule update`;
- you can also just use `git submodule update --init` without
- the explicit 'init' step if you do not intend to customize
- any submodule locations.
+ added and committed elsewhere) by setting `submodule.$name.url`
+ in .git/config. It uses the same setting from .gitmodules as
+ a template. If the URL is relative, it will be resolved using
+ the default remote. If there is no default remote, the current
+ repository will be assumed to be upstream.
++
+Optional <path> arguments limit which submodules will be initialized.
+If no path is specified, all submodules are initialized.
++
+When present, it will also copy the value of `submodule.$name.update`.
+This command does not alter existing information in .git/config.
+You can then customize the submodule clone URLs in .git/config
+for your local setup and proceed to `git submodule update`;
+you can also just use `git submodule update --init` without
+the explicit 'init' step if you do not intend to customize
+any submodule locations.
++
+See the add subcommand for the defintion of default remote.
deinit [-f|--force] (--all|[--] <path>...)::
Unregister the given submodules, i.e. remove the whole
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e3..15a5430c00 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
strbuf_addf(&remotesb, "remote.%s.url", remote);
free(remote);
- if (git_config_get_string(remotesb.buf, &remoteurl))
- /*
- * The repository is its own
- * authoritative upstream
- */
+ if (git_config_get_string(remotesb.buf, &remoteurl)) {
+ warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
remoteurl = xgetcwd();
+ }
relurl = relative_url(remoteurl, url, NULL);
strbuf_release(&remotesb);
free(remoteurl);
--
2.12.0.rc1.16.ge4278d41a0.dirty
^ permalink raw reply related
* Re: SHA1 collisions found
From: David Lang @ 2017-02-25 1:39 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, git
In-Reply-To: <20170225012100.ivfdlwspsqd7bkhf@sigill.intra.peff.net>
On Fri, 24 Feb 2017, Jeff King wrote:
>> what if they are forks of each other? (LEDE and OpenWRT, or just
>> linux-kernel and linux-kernel-stable)
>
> Once one flips, the other one needs to flip to, or can't interact with
> them. I know that's harsh, and is likely to create headaches. But in the
> long run, I think once everything has converged the resulting system is
> less insane.
>
> For that reason I _wouldn't_ recommend projects like the kernel flip the
> flag immediately. Ideally we write the code and the new versions
> permeate the community. Then somebody (per-project) decides that it's
> time for the community to start switching.
can you 'un-flip' the flag? or if you have someone who is a developer flip their
repo (because they heard that sha1 is unsafe, and they want to be safe), they
can't contribute to the kernel. We don't want to have them loose all their work,
so how can they convert their local repo back to somthing that's compatible?
how would submodules work if one module flips and another (or the parent)
doesn't?
OpenWRT/LEDE have their core repo, and they pull from many other (unrelated)
projects into that repo (and then have 'feeds', which is sort-of-like-submodules
to pull in other software that's maintained completely independently)
Microsoft has made lots of money with people being forced to upgrade Word
because one person got a new version and everyone else needed to upgrade to be
compatible. There's a LOT of pain during that process. Is that really the best
way to go?
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Jeff King @ 2017-02-25 1:47 UTC (permalink / raw)
To: David Lang; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, git
In-Reply-To: <nycvar.QRO.7.75.62.1702241733250.6590@qynat-yncgbc>
On Fri, Feb 24, 2017 at 05:39:43PM -0800, David Lang wrote:
> On Fri, 24 Feb 2017, Jeff King wrote:
>
> > > what if they are forks of each other? (LEDE and OpenWRT, or just
> > > linux-kernel and linux-kernel-stable)
> >
> > Once one flips, the other one needs to flip to, or can't interact with
> > them. I know that's harsh, and is likely to create headaches. But in the
> > long run, I think once everything has converged the resulting system is
> > less insane.
> >
> > For that reason I _wouldn't_ recommend projects like the kernel flip the
> > flag immediately. Ideally we write the code and the new versions
> > permeate the community. Then somebody (per-project) decides that it's
> > time for the community to start switching.
>
> can you 'un-flip' the flag? or if you have someone who is a developer flip
> their repo (because they heard that sha1 is unsafe, and they want to be
> safe), they can't contribute to the kernel. We don't want to have them loose
> all their work, so how can they convert their local repo back to somthing
> that's compatible?
I don't think it would be too hard to write an un-flipper (it's
basically just rewriting the newhash bit of history using sha1, and
converting your refs back to point at the sha1s).
> how would submodules work if one module flips and another (or the parent)
> doesn't?
That's a good question. It's possible that another exception should be
carved out for referring to a gitlink via sha1 (we _could_ say "no,
point to a newhash version of the submodule", but I think that creates a
lot of hardship for not much gain).
> OpenWRT/LEDE have their core repo, and they pull from many other (unrelated)
> projects into that repo (and then have 'feeds', which is
> sort-of-like-submodules to pull in other software that's maintained
> completely independently)
I think with submodules this should probably still work. If they are
pulling in with a subtree-ish strategy, then they'd convert the incoming
trees to the newhash format as part of that.
> Microsoft has made lots of money with people being forced to upgrade Word
> because one person got a new version and everyone else needed to upgrade to
> be compatible. There's a LOT of pain during that process. Is that really the
> best way to go?
I think there's going to be a lot of pain regardless. Any attempt to
mitigate that pain and work seamlessly across old and new versions of
git is going cause _ongoing_ pain as people quietly rewrite the same
content back and forth with different hashes. The viral-convergence
strategy is painful once (when you're forced to upgrade), but after that
just works.
If you want to work on a dual-hash strategy, be my guest. I can't
promise I'll be able to find horrific corner cases in it, but I
certainly can't even try to do so until there is a concrete proposal. :)
-Peff
^ permalink raw reply
* Re: SHA1 collisions found
From: David Lang @ 2017-02-25 1:56 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ian Jackson, Joey Hess, git
In-Reply-To: <20170225014747.f36j2ctlszpebpsy@sigill.intra.peff.net>
On Fri, 24 Feb 2017, Jeff King wrote:
>> OpenWRT/LEDE have their core repo, and they pull from many other (unrelated)
>> projects into that repo (and then have 'feeds', which is
>> sort-of-like-submodules to pull in other software that's maintained
>> completely independently)
>
> I think with submodules this should probably still work. If they are
> pulling in with a subtree-ish strategy, then they'd convert the incoming
> trees to the newhash format as part of that.
as I understand things, they have two categories of things
1. Feeds, which are completely independent, separate maintainers
2. core, which gets pulled into one repo, I don't know if they use submodules in
the process. I know that what downstream users see is a single repo.
I understand and agree with the idea of trying to converge rapidly. I'm just
looking at cases where this may be hard (or where there may be holdouts for
whatever reason)
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Jacob Keller @ 2017-02-25 2:26 UTC (permalink / raw)
To: Jeff King
Cc: David Lang, Junio C Hamano, Ian Jackson, Joey Hess,
Git mailing list
In-Reply-To: <20170225012100.ivfdlwspsqd7bkhf@sigill.intra.peff.net>
On Fri, Feb 24, 2017 at 5:21 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 24, 2017 at 05:00:55PM -0800, David Lang wrote:
>
>> On Fri, 24 Feb 2017, Jeff King wrote:
>>
>> >
>> > So I'd much rather see strong rules like:
>> >
>> > 1. Once a repo has flag-day switched over to the new hash format[1],
>> > new references are _always_ done with the new hash. Even ones that
>> > point to pre-flag-day objects!
>>
>> how do you define when a repo has "switched over" to the new format in a
>> distributed environment?
>
> You don't. It's a decision for each local repo, but the rules push
> everybody towards upgrading (because you forbid them pulling from or
> pushing to people who have upgraded).
>
> So in practice, some centralized distribution point switches, and then
> it floods out from there.
This seems like the most reasonable strategy so far. I think that
trying to allow long term co-existence is a huge pain that discourages
switching, when we actually want to encourage everyone to switch
someone has switched.
I don't think it's sane to try and allow simultaneous use of both
hashes, since that creates a lot of headaches and discourages
transition somewhat.
Thanks,
Jake
^ permalink raw reply
* Re: SHA1 collisions found
From: Jacob Keller @ 2017-02-25 2:28 UTC (permalink / raw)
To: David Lang
Cc: Jeff King, Junio C Hamano, Ian Jackson, Joey Hess,
Git mailing list
In-Reply-To: <nycvar.QRO.7.75.62.1702241733250.6590@qynat-yncgbc>
On Fri, Feb 24, 2017 at 5:39 PM, David Lang <david@lang.hm> wrote:
> On Fri, 24 Feb 2017, Jeff King wrote:
>
>>> what if they are forks of each other? (LEDE and OpenWRT, or just
>>> linux-kernel and linux-kernel-stable)
>>
>>
>> Once one flips, the other one needs to flip to, or can't interact with
>> them. I know that's harsh, and is likely to create headaches. But in the
>> long run, I think once everything has converged the resulting system is
>> less insane.
>>
>> For that reason I _wouldn't_ recommend projects like the kernel flip the
>> flag immediately. Ideally we write the code and the new versions
>> permeate the community. Then somebody (per-project) decides that it's
>> time for the community to start switching.
>
>
> can you 'un-flip' the flag? or if you have someone who is a developer flip
> their repo (because they heard that sha1 is unsafe, and they want to be
> safe), they can't contribute to the kernel. We don't want to have them loose
> all their work, so how can they convert their local repo back to somthing
> that's compatible?
I'd think one of the first things we want is a way to flip *and*
unflip by re-writing history ala git-filter-branch style. (So if you
wanted, you could also flip all your old history).
One unrelated thought I had. When an old client sees the new stuff, it
will probably fail in a lot of weird ways. I wonder what we can do so
that if we in the future have to switch to an even newer hash, how can
we make it so that the old versions give a more clean error
experience? Ideally so that it lessens the pain of transition somewhat
in the future if/when it has to happen again?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH v2 1/4] delete_ref: accept a reflog message argument
From: Kyle Meyer @ 2017-02-25 3:05 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Jeff King, Git Mailing List
In-Reply-To: <CACsJy8BJgL1i85nZ9CpAXCKaG+PQQkjRKLFPWzbsMF8WN8TEjQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> You'll probably want to update the comment block above if msg can be
> NULL. We have _very_ good documentation in this file, let's keep it
> uptodate.
Looking at how other functions in refs.h document their "msg" or
"logmsg" parameter, none seem to mention it explicitly. update_ref
refers to ref_transaction_update, and
ref_transaction_{update,create,delete,verify} refer to ref.h's
"Reference transaction updates" comment.
delete_ref's docstring already mentions that "flag" is passed to
ref_transaction_delete, so perhaps it should mention "msg" here as
well.
-- >8 --
Subject: [PATCH] delete_ref: mention "msg" parameter in docstring
delete_ref() was recently extended to take a "msg" argument.
---
refs.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/refs.h b/refs.h
index e529f4c3a..988750218 100644
--- a/refs.h
+++ b/refs.h
@@ -274,7 +274,8 @@ int reflog_exists(const char *refname);
* verify that the current value of the reference is old_sha1 before
* deleting it. If old_sha1 is NULL, delete the reference if it
* exists, regardless of its old value. It is an error for old_sha1 to
- * be NULL_SHA1. flags is passed through to ref_transaction_delete().
+ * be NULL_SHA1. msg and flags are passed through to
+ * ref_transaction_delete().
*/
int delete_ref(const char *msg, const char *refname,
const unsigned char *old_sha1, unsigned int flags);
--
2.11.1
^ permalink raw reply related
* Re: SHA1 collisions found
From: grarpamp @ 2017-02-25 5:39 UTC (permalink / raw)
To: git
In-Reply-To: <CA+P7+xqyaWHBxug0BPdCgDVJBLtsvUxbgfgy1uJcoGf3q6xMQg@mail.gmail.com>
Repos should address keeping / 'fixing' broken sha-1 as needed.
They also really need to create new native modes so users can
initialize and use repos with (sha-3 / sha-256 / whatever) going forward.
Backward compatibility with sha-1 or 'fixed sha-1' will be fine. Clients
can 'taste' and 'test' repos for which hash mode to use, or add it to
their configs. Make things flexible, modular, configurable, updateable.
What little point is there in 'fixing / caveating' their use of broken sha-1,
without also doing strong (sha-3 / optionals) in the first place, defaulting
new init's to whichever strong hash looks good, and letting natural
migration to that happen on its own through the default process.
Introducing new hash modes also gives good oppurtunity to incorporate
other generally 'incompatabile with the old' changes to benefit the future.
One might argue against mixed mode, after all, export and import,
as with any other repo migration, is generally possible. And mixed
mode tends to prolong the actual endeavour to move to something
better in the init itself. Native and new makes you update to follow.
A lot of question / wrong ramble here, but the point should be
consistant... move, natively, even if only for sake of death of old
broken hashes. And attacks only get worse. Thought food is all.
^ permalink raw reply
* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-25 6:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jeff King, Ian Jackson, Joey Hess, Git Mailing List
In-Reply-To: <CA+55aFw6BLjPK-F0RGd9LT7X5xosKOXOxuhmKX65ZHn09r1xow@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> For example, what I would suggest the rules be is something like this:
>
> - introduce new tag2/commit2/tree2/blob2 object type tags that imply
> that they were hashed using the new hash
>
> - an old type obviously can never contain a pointer to a new type (ie
> you can't have a "tree" object that contains a tree2 object or a blob2
> object.
>
> - but also make the rule that a *new* type can never contain a
> pointer to an old type, with the *very* specific exception that a
> commit2 can have a parent that is of type "commit".
OK, I think that is what Peff was suggesting in his message, and I
do not have problem with such a transition plan. Or the *very*
specific exception could be that a reference to "commit" can use old
name (which would allow binding a submodule before transition to a
new project).
We probably do not need "blob2" object as they do not embed any
pointer to another thing. A loose blob with old name can be made
available on the filesystem also under new name without much "heavy"
transition, and an in-pack blob can be pointed at with _two_ entries
in the updated pack index file under old and new names, both for the
base (just deflated) representation and also ofs-delta. A ref-delta
based on another blob with old name may need a bit of special
handling, but the deltification would not be visible at the "struct object"
layer, so probably not such a big deal.
We may also be able to get away without "commit2" and "tag2" as
their pointers can be widened and parse_{commit,tag}_object() should
be able to deal with objects with new names transparently. "tree2"
may be a bit tricky, though, but offhand it seems to me that nothing
is insurmountable.
> That way everything "converges" towards the new format: the only way
> you can stay on the old format is if you only have old-format objects,
> and once you have a new-format object all your objects are going to be
> new format - except for the history.
Yes.
> So you will end up with duplicate objects, and that's not good (think
> of what it does to all our full-tree "diff" optimizations, for example
> - you no longer get the "these sub-trees are identical" across a
> format change), but realistically you'll have a very limited time of
> that kind of duplication.
>
> I'd furthermore suggest that from a UI standpoint, we'd
>
> - convert to 64-character hex numbers (32-byte hashes)
>
> - (as mentioned earlier) default to a 40-character abbreviation
>
> - make the old 40-character SHA1's just show up within the same
> address space (so they'd also be encoded as 32-byte hashes, just with
> the last 12 bytes zero).
Yes to all of the above.
> - you'd see in the "object->type" whether it's a new or old-style hash.
I am not sure if this is needed. We may need to abstract tree_entry walker
a little bit as a preparatory step, but I suspect that the hash (and
more importantly the internal format) can be kept as an internal
knowledge to the object layer (i.e. {commit,tree,tag}.c).
So,... thanks for straightening me out. I was thinking we would
need mixed mode support for smoother transition, but it now seems to
me that the approach to stratify the history into old and new is
workable.
^ permalink raw reply
* [PATCH 0/6 v5] allow "-" as a shorthand for "previous branch"
From: Siddharth Kannan @ 2017-02-25 7:24 UTC (permalink / raw)
To: git
Cc: gitster, Matthieu.Moy, pranit.bauva, peff, pclouds, sandals,
Siddharth Kannan
An updated version of the patch [1]. Discussion here[1] has been taken into
account. The test for "-@{yesterday}" is there inside the log-shorthand test,
it is commented out for now.
I have removed the redundant pieces of code in merge.c and revert.c as mentioned
by Matthieu in [2]. As analysed by Junio[3], the same type of code inside
checkout.c and worktree.c can not be removed because the appropriate functions
inside revision.c are not called in their codepaths.
Thanks for your review of the previous versions, Junio and Matthieu!
[1]: 1487258054-32292-1-git-send-email-kannan.siddharth12@gmail.com
[2]: vpqbmu768on.fsf@anie.imag.fr
[3]: xmqq1sv1euob.fsf@gitster.mtv.corp.google.com
Siddharth Kannan (6):
revision.c: do not update argv with unknown option
revision.c: swap if/else blocks
revision.c: args starting with "-" might be a revision
sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"
merge.c: delegate handling of "-" shorthand to revision.c:get_sha1
revert.c: delegate handling of "-" shorthand to setup_revisions
builtin/merge.c | 2 -
builtin/revert.c | 2 -
revision.c | 15 +++---
sha1_name.c | 5 ++
t/t3035-merge-hyphen-shorthand.sh | 33 ++++++++++++
t/t3514-revert-shorthand.sh | 25 +++++++++
t/t4214-log-shorthand.sh | 106 ++++++++++++++++++++++++++++++++++++++
7 files changed, 178 insertions(+), 10 deletions(-)
create mode 100755 t/t3035-merge-hyphen-shorthand.sh
create mode 100755 t/t3514-revert-shorthand.sh
create mode 100755 t/t4214-log-shorthand.sh
--
2.1.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox