On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
[off-list ref] wrote:
When an object has already been exported (and thus is in the marks) it
is flagged as SHOWN, so it will not be exported again, even if this time
it's exported through a different ref.
We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.
Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags & SHOWN), the user still wants the ref
updated.
So:
% git branch test master
% git fast-export $mark_flags master
% git fast-export $mark_flags test
Would export 'test' properly.
Additionally, this fixes issues with remote helpers; now they can push
refs wich objects have already been exported.
Won't this also export child (or maybe parent) branches that weren't
mentioned? For example:
$ git branch one
$ echo foo > content
$ git commit -m two
$ git fast-export one
$ git fast-export two
I suspect that one of those will export both one and two. If not, this
seems like a great solution to the fast-export problem.
--
Cheers,
Sverre Rabbelier
From: Felipe Contreras <hidden> Date: 2016-06-15 22:55:09
On Tue, Oct 30, 2012 at 7:12 PM, Sverre Rabbelier [off-list ref] wrote:
On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
[off-list ref] wrote:
quoted
When an object has already been exported (and thus is in the marks) it
is flagged as SHOWN, so it will not be exported again, even if this time
it's exported through a different ref.
We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.
Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags & SHOWN), the user still wants the ref
updated.
So:
% git branch test master
% git fast-export $mark_flags master
% git fast-export $mark_flags test
Would export 'test' properly.
Additionally, this fixes issues with remote helpers; now they can push
refs wich objects have already been exported.
Won't this also export child (or maybe parent) branches that weren't
mentioned? For example:
$ git branch one
$ echo foo > content
$ git commit -m two
$ git fast-export one
$ git fast-export two
I suspect that one of those will export both one and two. If not, this
seems like a great solution to the fast-export problem.
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
And if you are talking about the ref for the reset at the end, it has
to be both in the list of refs selected by the user (initially in
&revs.pending), either marked or the object already referenced by
another ref in the list selected by the user (e.g. fast-export one
two, where one^{commit} == two^{commit}, and not marked as
UNINTERESTING (e.g. ^two).
Cheers.
--
Felipe Contreras
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
[off-list ref] wrote:
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
Because the marking is per-commit, not per-ref, right? Perhaps you
could add a simple test case to make sure it works as expected?
Something along the lines of the scenario I described in my previous
email?
--
Cheers,
Sverre Rabbelier
From: Felipe Contreras <hidden> Date: 2016-06-15 22:55:09
On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier [off-list ref] wrote:
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
[off-list ref] wrote:
quoted
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
Because the marking is per-commit, not per-ref, right?
Oh, you meant using marks?
It doesn't matter anyway, because get_tags_and_duplicates() would get
'one' on the first run, and 'two' on the second.
If you meant something like this:
% git fast-export $marks_args one
% git fast-export $marks_args one two
Then yeah, 'one' will be updated once again in the second command, but
there's nothing fatal about it, and your patch series had the same
result.
Perhaps you
could add a simple test case to make sure it works as expected?
Something along the lines of the scenario I described in my previous
email?
I'm not sure what that test should be doing.
Cheers.
--
Felipe Contreras
On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
[off-list ref] wrote:
On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier [off-list ref] wrote:
quoted
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
[off-list ref] wrote:
quoted
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
Because the marking is per-commit, not per-ref, right?
Oh, you meant using marks?
No, I meant the 'SHOWN' flag, doesn't it get added per commit, not per
ref? That is, commit->object.flags & SHOWN refers to the object
underlying the ref. So I suspect this scenario doesn't pass the tests:
git init &&
echo first > content &&
git add content &&
git commit -m "first" &&
git branch first &&
echo two > content &&
git commit -m "second" &&
git branch second &&
git fast-export first > actual &&
test_cmp actual expected_first &&
git fast-export second > actual &&
test_cmp actual expected_second
With expected_first being something like:
<fast-export stream with the first commit>
<reset command to set first to the right commit>
And expected_second being something like
<fast export stream with the first and second command>
<reset command to set first and second to their respective branches>
--
Cheers,
Sverre Rabbelier
From: Felipe Contreras <hidden> Date: 2016-06-15 22:55:09
On Tue, Oct 30, 2012 at 10:59 PM, Sverre Rabbelier [off-list ref] wrote:
On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
[off-list ref] wrote:
quoted
On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier [off-list ref] wrote:
quoted
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
[off-list ref] wrote:
quoted
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
Because the marking is per-commit, not per-ref, right?
Oh, you meant using marks?
No, I meant the 'SHOWN' flag, doesn't it get added per commit, not per
ref? That is, commit->object.flags & SHOWN refers to the object
underlying the ref. So I suspect this scenario doesn't pass the tests:
Without marks you cannot have the SHOWN mark at that point; we haven't
traversed the commits.
git init &&
echo first > content &&
git add content &&
git commit -m "first" &&
git branch first &&
echo two > content &&
git commit -m "second" &&
git branch second &&
git fast-export first > actual &&
test_cmp actual expected_first &&
git fast-export second > actual &&
test_cmp actual expected_second
With expected_first being something like:
<fast-export stream with the first commit>
<reset command to set first to the right commit>
Why would a 'reset' command be expected if the 'first' branch is
already pointing to the 'first' commit?
And expected_second being something like
<fast export stream with the first and second command>
<reset command to set first and second to their respective branches>
Ditto, plus, why would 'git fast-export second' do anything regarding
'first'? It wasn't specified in the committish; it's not relevant.
Before an after my patch the output is the same:
% git fast-export first:
reset refs/heads/first
commit refs/heads/first
% git fast-export second:
reset refs/heads/second
commit refs/heads/second
commit refs/heads/second
Which is expected and correct; the branch already points to the right
commit, no need for an extra reset.
Cheers.
--
Felipe Contreras
From: Felipe Contreras <hidden> Date: 2016-06-15 22:55:09
On Tue, Oct 30, 2012 at 11:35 PM, Sverre Rabbelier [off-list ref] wrote:
On Tue, Oct 30, 2012 at 3:18 PM, Felipe Contreras
[off-list ref] wrote:
quoted
Which is expected and correct; the branch already points to the right
commit, no need for an extra reset.
I think you're correct. Thanks for confirming.
Thanks for reviewing. If you are still not convinced, I could pull the
patches from msysgit and simplify them, I'm sure the end result would
be pretty similar, if not exactly the same as this patch (plus other
orthogonal changes). I saw some patches that were not part of the
patch series you sent before, so maybe that's why you expected certain
behavior that wasn't actually there in that particular patch series.
But hopefully that's not needed.
Cheers.
--
Felipe Contreras