* Re: change symlink
From: shawn wilson @ 2012-10-30 21:42 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m2fw4vod81.fsf@igel.home>
On Tue, Oct 30, 2012 at 9:35 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> shawn wilson <ag4ve.us@gmail.com> writes:
>
>> but should t2 be reported as 'deleted'?
>
> Sure, that's what you did.
>
if i do the same to a file (same repo):
touch test2
git add test2
git commit test2 -m "test2"
rm test
ln -s test2 test
git status
# On branch master
# Changes not staged for commit:
# (use "git add/rm <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# deleted: t2
# typechange: test
#
no changes added to commit (use "git add" and/or "git commit -a")
why is this different?
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 21:41 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sverre Rabbelier, >, Jeff King, Junio C Hamano,
Johannes Schindelin, Elijah Newren
In-Reply-To: <20121030213827.GM15167@elie.Belkin>
On Tue, Oct 30, 2012 at 10:38 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> % 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,
>
> That's probably worth a mention in the commit message and tests
> (test_expect_failure), to save future readers from some confusion.
It is mentioned in the commit message.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 21:40 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s3LP65XOYFg-tBe_rzT1+gXp=714C-u14mkwxY26r4b=g@mail.gmail.com>
On Tue, Oct 30, 2012 at 8:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>> On Tue, Oct 30, 2012 at 7:47 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> and in the examples listed in the patch
>>> description the changed behavior does not look like an improvement.
>>
>> I disagree.
>>
>> % git log master ^master
>>
>> What do you expect? Nothing.
>
> Yep.
>
>> % git fast-export master ^master
>>
>> What do you expect? Nothing.
So you think what we have now is the correct behavior:
% git fast-export master ^master
reset refs/heads/master
from :0
That of course would crash fast-import. But hey, it's your opinion.
Would be interesting to see if other people think the above is correct.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Jonathan Nieder @ 2012-10-30 21:38 UTC (permalink / raw)
To: Felipe Contreras
Cc: Sverre Rabbelier, >, Jeff King, Junio C Hamano,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s2QwdZKqJq0BZ5HOtZYiCMxCxycui9EmxxfL+Sa6M_6+g@mail.gmail.com>
Felipe Contreras wrote:
> % 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,
That's probably worth a mention in the commit message and tests
(test_expect_failure), to save future readers from some confusion.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 21:35 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAGdFq_jJwZMLq=3co13hs7gas6y9kZRTKwcT+CP=n6-24Uv5Og@mail.gmail.com>
On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
> <felipe.contreras@gmail.com> 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?
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
^ permalink raw reply
* Re: change symlink
From: Andreas Schwab @ 2012-10-30 21:35 UTC (permalink / raw)
To: shawn wilson; +Cc: git
In-Reply-To: <CAH_OBidWxkhG3o4C4OPP4OxyQQfw_fF_h4C9KR9AnoOZ27=9TQ@mail.gmail.com>
shawn wilson <ag4ve.us@gmail.com> writes:
> but should t2 be reported as 'deleted'?
Sure, that's what you did.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] Teach rm to remove submodules when given with a trailing '/'
From: Jens Lehmann @ 2012-10-30 21:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List, Jeff King
In-Reply-To: <508E2C05.9010109@viscovery.net>
Am 29.10.2012 08:11, schrieb Johannes Sixt:
> Am 10/29/2012 0:28, schrieb Jens Lehmann:
>> + /* Remove trailing '/' from directories to find submodules in the index */
>> + for (i = 0; i < argc; i++) {
>> + size_t pathlen = strlen(argv[i]);
>> + if (pathlen && is_directory(argv[i]) && (argv[i][pathlen - 1] == '/'))
>> + argv[i] = xmemdupz(argv[i], pathlen - 1);
>> + }
>> +
>> pathspec = get_pathspec(prefix, argv);
>> refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
>
> That's wrong: Either move the check below get_pathspec() (which normalizes
> backslashes to forward-slashes on Windows) or use is_dir_sep().
Thanks for bringing this up.
> But isn't it somewhat dangerous to check pathspec for existance in the
> worktree without interpreting them? Think of magic pathspec syntax (that
> we do not have yet, but which may materialize sometime in the future).
I have to admit I'm not aware of magic pathspec syntax. Do you happen to
have any pointers where I could look at code doing similar things right?
^ permalink raw reply
* Re: [PATCH 2/2] completion: simplify __gitcomp test helper
From: SZEDER Gábor @ 2012-10-30 21:27 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1350869941-22485-3-git-send-email-felipe.contreras@gmail.com>
On Mon, Oct 22, 2012 at 03:39:01AM +0200, Felipe Contreras wrote:
> By using print_comp as suggested by SZEDER Gábor.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> t/t9902-completion.sh | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 1c6952a..2e7fc06 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -74,15 +74,12 @@ newline=$'\n'
This $newline variable was only used to set IFS to a newline inside SQ
blocks. AFAICS after this change there are no such places left,
because print_comp() takes care of IFS, so $newline is not necessary
anymore.
> test_gitcomp ()
> {
> + local -a COMPREPLY &&
> sed -e 's/Z$//' > expected &&
> - (
> - local -a COMPREPLY &&
> - cur="$1" &&
> - shift &&
> - __gitcomp "$@" &&
> - IFS="$newline" &&
> - echo "${COMPREPLY[*]}" > out
> - ) &&
> + cur="$1" &&
> + shift &&
> + __gitcomp "$@" &&
> + print_comp &&
> test_cmp expected out
> }
>
> --
> 1.8.0
>
^ permalink raw reply
* Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable
From: Jens Lehmann @ 2012-10-30 21:26 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nPJ-8UvyYN800-Tj+mLQtv-vF3SJScb1O8EEiY238Z6w@mail.gmail.com>
Am 29.10.2012 11:30, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> Am 02.10.2012 21:44, schrieb Jens Lehmann:
>>> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:
>>>> Introduce a diff.submoduleFormat configuration variable corresponding
>>>> to the '--submodule' command-line option of 'git diff'.
>>>
>>> Nice. Maybe a better name would be "diff.submodule", as this sets the
>>> default for the "--submodule" option of diff?
>>>
>>> And I think you should also test in t4041 that "--submodule=short"
>>> overrides the config setting.
>>
>> We also need tests which show that setting that config to "log" does
>> not break one of the many users of "git diff" ("stash", "rebase" and
>> "format-patch" come to mind, most probably I missed some others). I
>> suspect we'll have to add "--submodule=short" options to some call
>> sites to keep them working with submodule changes.
>
> Um, why would "stash", "rebase" or "format-patch" be affected by this
> setting? They don't operate on submodules at all. To be sure, I ran
> all the tests with the following diff and nothing broke.
They do operate on the submodule commits too (while they don't touch
submodule work trees) and IIRC rebase applies diffs, so that could
break when the output of diff changes due to the new config option.
But it looks like your test did prove that nothing goes wrong there,
I assume they they use plumbing diff commands which aren't affected
by the new option.
^ permalink raw reply
* Re: change symlink
From: shawn wilson @ 2012-10-30 21:24 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m2mwz3odys.fsf@igel.home>
On Tue, Oct 30, 2012 at 9:19 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> shawn wilson <ag4ve.us@gmail.com> writes:
>> % git status
>> # On branch master
>> # Changes not staged for commit:
>> # (use "git add/rm <file>..." to update what will be committed)
>> # (use "git checkout -- <file>..." to discard changes in working directory)
>> #
>> # deleted: t2
>> #
>> no changes added to commit (use "git add" and/or "git commit -a")
>
> I'd expected t2/one/test be reported as untracked.
>
that's fine - i can do 'git list-files --others' but should t2 be
reported as 'deleted'?
^ permalink raw reply
* Re: change symlink
From: Andreas Schwab @ 2012-10-30 21:19 UTC (permalink / raw)
To: shawn wilson; +Cc: git
In-Reply-To: <CAH_OBie-irmpBrJG6KB3W8bgYjQdyVYiUR-SvJPnx1FXUya0uA@mail.gmail.com>
shawn wilson <ag4ve.us@gmail.com> writes:
> i'm curious why this is being reported as deleted in status and diff
> and not modified? this was tested on a build of the master branch of
> the current git repo (1.8.0).
>
> mkdir t cd t; git --init
>
> touch test
> git add test
> git commit test -m "test"
>
> ln -s test t2
> git add t2
> git commit t2 -m "symlink"
>
> rm t2
> mkdir -p t2/one
> ln -s test t2/one/test
git add t2/one/test
> this then shows up as:
>
> % git status
> # On branch master
> # Changes not staged for commit:
> # (use "git add/rm <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # deleted: t2
> #
> no changes added to commit (use "git add" and/or "git commit -a")
I'd expected t2/one/test be reported as untracked.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Sverre Rabbelier @ 2012-10-30 21:17 UTC (permalink / raw)
To: Felipe Contreras
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s3MHrG_XeZEodnxemrW-V18+NHnFvi7koyx9mH8XuHc6w@mail.gmail.com>
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
<felipe.contreras@gmail.com> 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
^ permalink raw reply
* Re: Can't understand the behaviour of git-diff --submodule
From: Jens Lehmann @ 2012-10-30 21:17 UTC (permalink / raw)
To: Francis Moreau; +Cc: git
In-Reply-To: <508C6823.5060800@web.de>
Am 28.10.2012 01:02, schrieb Jens Lehmann:
> Am 26.10.2012 22:43, schrieb Francis Moreau:
>> On Fri, Oct 26, 2012 at 10:05 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> [...]
>>>
>>> That is weird, "git diff --submodule" should show that too. Is there
>>> anything unusual about your setup? (The only explanation I can come
>>> up with after checking the code is that your submodule has neither a
>>> .git directory nor a gitfile or the objects directory in there doesn't
>>> contain these commits)
>>
>> Oh now you're asking, I think the submodule has been added by using
>> the --reference option of git-submodule-add.
>>
>> $ cd configs
>> $ cat .git
>> gitdir: ../.git/modules/configs
>
> Thanks, I suspect the --reference option makes the difference here,
> I'll check that as soon as I find some time.
Since 1.7.11 and 1.7.10.3 git does handle submodules with alternates
(which is what --reference uses) correctly. What version are you
seeing this problem with?
^ permalink raw reply
* change symlink
From: shawn wilson @ 2012-10-30 21:07 UTC (permalink / raw)
To: git
i'm curious why this is being reported as deleted in status and diff
and not modified? this was tested on a build of the master branch of
the current git repo (1.8.0).
mkdir t cd t; git --init
touch test
git add test
git commit test -m "test"
ln -s test t2
git add t2
git commit t2 -m "symlink"
rm t2
mkdir -p t2/one
ln -s test t2/one/test
this then shows up as:
% git status
# On branch master
# Changes not staged for commit:
# (use "git add/rm <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# deleted: t2
#
no changes added to commit (use "git add" and/or "git commit -a")
% git --no-pager diff
diff --git a/t2 b/t2
deleted file mode 120000
index 30d74d2..0000000
--- a/t2
+++ /dev/null
@@ -1 +0,0 @@
-test
\\ No newline at end of file
^ permalink raw reply related
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 20:15 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
Daniel Barkalow, Michael J Gruber
In-Reply-To: <alpine.DEB.1.00.1210302027410.7256@s15462909.onlinehome-server.info>
Hi,
On Tue, Oct 30, 2012 at 8:33 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Tue, 30 Oct 2012, Felipe Contreras wrote:
>
>> But you mentioned something about cooperation, and I've yet to see how
>> is it that you are planning to cooperate. If you say you don't have time
>> to spend on this, I don't see why I should worry about testing this
>> series of patches.
>
> It has been mentioned before that the communication style including all
> these snarky and nasty comments is not helpful.
Snarky and nasty comments? How about this?
---
> As to the functionality you seek: git-remote-hg found in
> git://github.com/msysgit/git works. It has the following advantages
> over every other solution, including the one proposed in this thread:
>
> - it works
>
> - no really, it works
>
> - it supports pushes, too
>
> - it matured over a long time
>
> - there are tests
>
> - whenever we fixed bugs, we also added tests for the bug fixes
>
> - it is rock solid
>
> - it is in constant use
>
> Without push support, remote-hg is useless to me. Without regression
> tests proving that it is rock solid, I will not use remote-hg. And I
> will not indulge in efforts to duplicate work.
---
How many times does somebody has to say "it works" before it becomes a
snarky comment?
Or this?
---
> FTR, the reason that it's crashing is because you're lying. You're
> saying you already have master (by means of ^master), but you don't.
---
Or this?
---
> It seems unlikely to me that this never worked, surely no reviewer
> would accept a patch that doesn't actually implement the feature?
> What's the history here?
---
So what did I say?
> But you mentioned something about cooperation,
That's a fact.
Johannes:
---
> > It would be better to work together, but to me the code-styles are way
> > too different, the difference between night and day.
>
> Aha. Well, okay, it was an offer to collaborate.
---
> and I've yet to see how is it that you are planning to cooperate.
This is also a fact. You haven't provided a branch, you haven't reviewed
my implementation, you haven't tried it. You mentioned something about
testing the performance, but then retracted from it.
So, if you were planning to collaborate, now it would be a good time to
mention how.
> If you say you don't have time to spend on this, I don't see why I
> should worry about testing this series of patches.
I'm just clarifying how I'm planning to spend my time, specifically if
you are not going to collaborate.
What is snarky and nasty about any of these comments? I'm simply asking
you if you are going to collaborate and how, because I don't see it,
and what I'm going to do.
You think that's snarkier than the comments above? Well, I disagree.
But I don't blame you when you are snarky, nor do I think I should.
> It is hardly the first time that your mails have been insulting, as
> can be researched easily from in the public mailing list archives.
Those who want to be insulted would get insulted. I asked
a simple question "are you going to collaborate?", if you find that
offensive, that's your right.
> In light of the indignation when advised to keep the tone down a little,
> it is probable that the mails were never put through the "would I be
> insulted or hurt if I was the recipient of this?" test, as in "do you want
> me to throw away my work?" when you literally asked us to throw away our
> work.
How did I ask you to throw away your work? I have asked multiple times
now for you to provide a branch so that we can take a look and try it.
I don't know of a better way to throw away code than to refuse to
provide it, which is what you have been doing. So, if anybody can be
blamed of trying to throw away code, it shouldn't be me.
I know you will find the previous statement offensive, but it happens to
be true. It is sad that you will concentrate on the statement, rather
than the fact, and instead of providing the branch (which will help
to avoid throwing away the code), and thus nullify the statement, you
choose to be offended and complain about how offended you are.
Well, I'm offended at how much you refuse to collaborate, and at how
much disdain you throw at my code, but I'm not going to complain about
me being offended; people get offended about all sorts of things. Why
is why there's no law against offending people.
Instead, I choose to do something positive about it and improve my code
with your criticism (e.g. lack of tests), even if that criticism is rude
and unwarranted. But that seems to mean nothing to you.
> So unlike others, I do not ask you to change your tone, nor your
> willingness to work with others. Instead, I prefer to do other things
> instead.
I guess that answers the question; you are not going to collaborate. Got
it.
I will not ask you again for a branch with the remote-hg code.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Johannes Schindelin @ 2012-10-30 19:41 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jonathan Nieder, Jeff King, Junio C Hamano, Sverre Rabbelier,
Elijah Newren, git
In-Reply-To: <CAMP44s1W4mwK+cNwBqu2S0=Aw04XX9KBan8w4ghyzqbODdmiLQ@mail.gmail.com>
Hi Felipe,
On Tue, 30 Oct 2012, Felipe Contreras wrote:
> On Tue, Oct 30, 2012 at 8:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Felipe Contreras wrote:
> >> On Tue, Oct 30, 2012 at 7:47 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> >
> >>> and in the examples listed in the patch
> >>> description the changed behavior does not look like an improvement.
> >>
> >> I disagree.
> >>
> >> % git log master ^master
> >>
> >> What do you expect? Nothing.
> >
> > Yep.
> >
> >> % git fast-export master ^master
> >>
> >> What do you expect? Nothing.
> >
> > Nope.
>
> That's _your_ opinion. I would like to see what others think.
If you wanted to prove that you can work with others without offending
them, I think that failed.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 19:33 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
Daniel Barkalow, Michael J Gruber
In-Reply-To: <CAMP44s3CEGqUav-ijnzm7osD70LsjRLyOEeV3bF-LWYTCEPCSQ@mail.gmail.com>
Hi Felipe,
On Tue, 30 Oct 2012, Felipe Contreras wrote:
> But you mentioned something about cooperation, and I've yet to see how
> is it that you are planning to cooperate. If you say you don't have time
> to spend on this, I don't see why I should worry about testing this
> series of patches.
It has been mentioned before that the communication style including all
these snarky and nasty comments is not helpful. It is hardly the first
time that your mails have been insulting, as can be researched easily from
in the public mailing list archives.
In light of the indignation when advised to keep the tone down a little,
it is probable that the mails were never put through the "would I be
insulted or hurt if I was the recipient of this?" test, as in "do you want
me to throw away my work?" when you literally asked us to throw away our
work.
So unlike others, I do not ask you to change your tone, nor your
willingness to work with others. Instead, I prefer to do other things
instead.
Hth,
Johannes
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 19:17 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <20121030185914.GI15167@elie.Belkin>
(again to the mailing list)
On Tue, Oct 30, 2012 at 7:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> They have been marked as UNINTERESTING for a reason, lets respect that.
That doesn't say anything.
> and in the examples listed in the patch
> description the changed behavior does not look like an improvement.
I disagree.
% git log master ^master
What do you expect? Nothing.
% git fast-export master ^master
What do you expect? Nothing.
> Worse, the description lists a few examples but gives no convincing
> explanation to reassure about the lack of bad behavior for examples
> not listed.
What examples not listed?
--
Felipe Contreras
^ permalink raw reply
* Re: git push tags
From: Chris Rorvick @ 2012-10-30 19:11 UTC (permalink / raw)
To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9CC9wjeyggejkVjKgY2HGAFw70hJo-S0S-W-p4gnd2zug@mail.gmail.com>
On Tue, Oct 30, 2012 at 1:34 PM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hi Cris,
>
> I think a key in the config file of the remote repo is better than an
> option on git-push for what concerns security: it allows the owner of
> the remote repo to enforce the policy not to overwrite tags, which
> would not be possible if any user that has push access can --force
> tags.
>
> -Angelo
Hi Angelo,
Security is orthogonal to what this patch is attempting to resolve.
As Kacper pointed out, you can never be sure you're not going to
clobber an existing tag in the remote repo. This patch attempts to
give git-push better (i.e., less surprising) semantics for tags. In
other words, it's should will prevent mistakes, not provide any sort
of security.
So I don't think a config option is better or worse, it's just trying
to solve a different problem.
Thanks,
Chris
^ permalink raw reply
* [PATCH v3 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>
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.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/fast-export.c | 11 ++++++++---
t/t5800-remote-helpers.sh | 11 +++++++++++
t/t9350-fast-export.sh | 14 ++++++++++++++
3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7fb6fe1..663a93d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct object_array *pending,
typename(e->item->type));
continue;
}
- if (commit->util) {
- /* more than one name for the same object */
+
+ /*
+ * This ref will not be updated through a commit, lets make
+ * sure it gets properly upddated eventually.
+ */
+ if (commit->util || commit->object.flags & SHOWN) {
if (!(commit->object.flags & UNINTERESTING))
string_list_append(extra_refs, full_name)->util = commit;
- } else
+ }
+ if (!commit->util)
commit->util = full_name;
}
}
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..69a145a 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new refspec' '
compare_refs clone HEAD server refs/heads/new-refspec
'
+test_expect_success 'push ref with existing object' '
+ (cd localclone &&
+ git branch point-to-master master &&
+ git push origin point-to-master
+ ) &&
+
+ (cd server &&
+ git show-ref refs/heads/point-to-master
+ )
+'
+
test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 6ea8f6f..a4178e3 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -446,4 +446,18 @@ test_expect_success 'proper extra refs handling' '
test_cmp expected actual
'
+cat > expected << EOF
+reset refs/heads/master
+from :13
+
+EOF
+
+test_expect_success 'refs are updated even if no commits need to be exported' '
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks master > /dev/null &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks master > actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.0
^ permalink raw reply related
* [PATCH v3 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>
They have been marked as UNINTERESTING for a reason, lets respect that.
Currently the first ref is handled properly, but not the rest, so:
% git fast-export master ^master
Would currently throw a reset for master (2nd ref), which is not what we
want.
% git fast-export master ^foo ^bar ^roo
% git fast-export master salsa..tacos
Even if all these refs point to the same object; foo, bar, roo, salsa,
and tacos would all get a reset.
This is most certainly not what we want. After this patch, nothing gets
exported, because nothing was selected (everything is UNINTERESTING).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/fast-export.c | 7 ++++---
t/t9350-fast-export.sh | 6 ++++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 065f324..7fb6fe1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array *pending,
typename(e->item->type));
continue;
}
- if (commit->util)
+ if (commit->util) {
/* more than one name for the same object */
- string_list_append(extra_refs, full_name)->util = commit;
- else
+ if (!(commit->object.flags & UNINTERESTING))
+ string_list_append(extra_refs, full_name)->util = commit;
+ } else
commit->util = full_name;
}
}
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 49bdb44..6ea8f6f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
)
'
+test_expect_success 'proper extra refs handling' '
+ git fast-export master ^master master..master > actual &&
+ echo -n > expected &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.0
^ permalink raw reply related
* [PATCH v3 2/4] fast-export: fix comparisson in tests
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>
First the expected, then the actual, otherwise the diff would be the
opposite of what we want.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9350-fast-export.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3e821f9..49bdb44 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
(
cd limit-by-paths &&
git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
@@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
(
cd limit-by-paths &&
git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
@@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
(
cd limit-by-paths &&
git fast-export master~2..master~1 > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
--
1.8.0
^ permalink raw reply related
* [PATCH v3 1/4] fast-export: trivial cleanup
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>
Setting commit to commit is a no-op. It might have been there to avoid a
compiler warning, but if so, it was the compiler to blame.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/fast-export.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..065f324 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,
for (i = 0; i < pending->nr; i++) {
struct object_array_entry *e = pending->objects + i;
unsigned char sha1[20];
- struct commit *commit = commit;
+ struct commit *commit;
char *full_name;
if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
--
1.8.0
^ permalink raw reply related
* [PATCH v3 0/4] fast-export: general fixes
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
Johannes Schindelin, Elijah Newren, Felipe Contreras
Hi,
Note: sorry for the noise, the first try (v2) was silently eaten by the mailing
list handler.
First patches are general cleanups and fixes, the last patch fixes a real issue
that affects remote helpers.
Changes since v2:
* Actually send it to the ml
Changes since v1:
* Improved commit messages
* Use /dev/null in tests
* Add test for remote helpers
Felipe Contreras (4):
fast-export: trivial cleanup
fast-export: fix comparisson in tests
fast-export: don't handle uninteresting refs
fast-export: make sure refs are updated properly
builtin/fast-export.c | 16 +++++++++++-----
t/t5800-remote-helpers.sh | 11 +++++++++++
t/t9350-fast-export.sh | 26 +++++++++++++++++++++++---
3 files changed, 45 insertions(+), 8 deletions(-)
--
1.8.0
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 18:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> They have been marked as UNINTERESTING for a reason, lets respect that.
This patch looks unsafe, and in the examples listed in the patch
description the changed behavior does not look like an improvement.
Worse, the description lists a few examples but gives no convincing
explanation to reassure about the lack of bad behavior for examples
not listed.
Perhaps this patch has a prerequisite and has come out of order.
Hope that helps,
Jonathan
Patch left unsnipped so we can get a copy in the list archive.
> Currently the first ref is handled properly, but not the rest, so:
>
> % git fast-export master ^master
>
> Would currently throw a reset for master (2nd ref), which is not what we
> want.
>
> % git fast-export master ^foo ^bar ^roo
> % git fast-export master salsa..tacos
>
> Even if all these refs point to the same object; foo, bar, roo, salsa,
> and tacos would all get a reset.
>
> This is most certainly not what we want. After this patch, nothing gets
> exported, because nothing was selected (everything is UNINTERESTING).
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> builtin/fast-export.c | 7 ++++---
> t/t9350-fast-export.sh | 6 ++++++
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 065f324..7fb6fe1 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array *pending,
> typename(e->item->type));
> continue;
> }
> - if (commit->util)
> + if (commit->util) {
> /* more than one name for the same object */
> - string_list_append(extra_refs, full_name)->util = commit;
> - else
> + if (!(commit->object.flags & UNINTERESTING))
> + string_list_append(extra_refs, full_name)->util = commit;
> + } else
> commit->util = full_name;
> }
> }
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 49bdb44..6ea8f6f 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
> )
> '
>
> +test_expect_success 'proper extra refs handling' '
> + git fast-export master ^master master..master > actual &&
> + echo -n > expected &&
> + test_cmp expected actual
> +'
> +
> test_done
> --
> 1.8.0
^ 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