* Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
From: Jonathan Nieder @ 2012-10-31 0:37 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351623987-21012-5-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> --- 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;
Here's an explanation of why the above makes sense to me.
get_tags_and_duplicates() gets called after the marks import and
before the revision walk. It walks through the revs from the
commandline and for each one:
- peels it to a refname, and then to a commit
- stores the refname so fast-export knows what arg to pass to
the "commit" command during the revision walk
- if it already had a refname stored, instead adds the
(refname, commit) pair to the extra_refs list, so fast-export
knows to add a "reset" command later.
If the commit already has the SHOWN flag set because it was pointed to
by a mark, it is not going to come up in the revision walk, so it will
not be mentioned in the output stream unless it is added to
extra_refs. That's what this patch does.
Incidentally, the change from "else" to "if (!commit->util)" is
unnecessary because if a commit is already SHOWN then it will not be
encountered in the revision walk so commit->util does not need to be
set.
If the commit does not have the SHOWN or UNINTERESTING flag set but it
is going to get the UNINTERESTING flag set during the walk because of
a negative commit listed on the command line, this patch won't help.
Jonathan
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Jonathan Nieder @ 2012-10-31 0:11 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351623987-21012-5-git-send-email-felipe.contreras@gmail.com>
(cc-ing the git list)
Felipe Contreras 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
Yes, makes perfect sense.
For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
[...]
> --- 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
> + )
Style: if you indent like this, the test becomes clearer:
(
cd localclone &&
git branch point-to-master master &&
git push origin point-to-master
) &&
(
cd server &&
git rev-parse --verify refs/heads/point-to-master
)
[...]
> +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 &&
The redirect just makes the test log with "-v" less informative, so
I'd drop it.
> + git fast-export --import-marks=tmp-marks \
> + --export-marks=tmp-marks master > actual &&
> + test_cmp expected actual
Redirections in git shell scripts are generally spelled as
"do_something >actual", without a space between the operator and
filename.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 23:55 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s3ArAQXH+-EbH4MHYaV6fTAWdwGzBdZwzn_qtCABHyonQ@mail.gmail.com>
Felipe Contreras wrote:
> On Tue, Oct 30, 2012 at 11:07 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nope. I just don't want regressions, and found a patch description
>> that did nothing to explain to the reader how it avoids regressions
>> more than a little disturbing.
>
> I see, so you don't have any specific case where this could cause
> regressions, you are just saying it _might_ (like all patches).
Yes, exactly. The commit log needs a description of the current
behavior, the intent behind the current code, the change the patch
makes, and the motivation behind that change, like all patches.
Despite the nice examples, it doesn't currently have that.
The patch description just raises more questions for the reader. From
the description, one might imagine that this patch causes
git fast-export <mark args> master
not to emit anything when another branch that has already been
exported is ahead of "master". If I understand correctly (though
I haven't tested), this patch does cause
git fast-export ^next master
not to emit anything when next is ahead of "master". That doesn't
seem like progress.
I haven't reviewed the later patches in the series; maybe they fix
these things. But in the long term it is much easier to understand
and maintain a patch series that does not introduce regressions in the
first place, and the context one might use to convincingly explain
that a patch is not introducing a regression turns out to be essential
for many other purposes as well.
Jonathan
^ permalink raw reply
* Re: Links broken in ref docs.
From: Mike Norman @ 2012-10-30 23:30 UTC (permalink / raw)
To: Holger Hellmuth (IKS); +Cc: Kevin, Scott Chacon, Andrew Ardill, git
In-Reply-To: <508FAE23.4080704@ira.uka.de>
I just checked and the issue seems to be fixed! Clicked around on a
bunch of previously broken links and they work!
On Tue, Oct 30, 2012 at 3:38 AM, Holger Hellmuth (IKS)
<hellmuth@ira.uka.de> wrote:
> Am 30.10.2012 09:07, schrieb Mike Norman:
>
>> Not seen any recently. I'm guessing the dev is in the path of
>> hurricane Sandy? (Not sarcasm, btw.)
>
>
> Do you still see failures? I checked out the website just now and it seemed
> to work flawlessly (at least the links I tried, could not find any Sharing
> or Updating section). New design since I last visited, more end-user polish.
>
>
^ permalink raw reply
* Re: [PATCH v5 1/3] completion: add new __gitcompadd helper
From: SZEDER Gábor @ 2012-10-30 23:18 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Matthieu Moy
In-Reply-To: <CAMP44s3guAhFe1ACrAzC9T05fCMAejU6Wn+GtYfSpmfJ8EX-xg@mail.gmail.com>
On Mon, Oct 22, 2012 at 02:41:21AM +0200, Felipe Contreras wrote:
> On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> > On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:
>
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index d743e56..01325de 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
> >> fi
> >> fi
> >>
> >> +__gitcompadd ()
> >> +{
> >> + COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
> >> +}
> >> +
> >> # Generates completion reply with compgen, appending a space to possible
> >> # completion words, if necessary.
> >> # It accepts 1 to 4 arguments:
> >> @@ -238,13 +243,11 @@ __gitcomp ()
> >>
> >> case "$cur_" in
> >> --*=)
> >> - COMPREPLY=()
> >> + __gitcompadd
> >> ;;
> >> *)
> >> local IFS=$'\n'
> >> - COMPREPLY=($(compgen -P "${2-}" \
> >> - -W "$(__gitcomp_1 "${1-}" "${4-}")" \
> >> - -- "$cur_"))
> >> + __gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
> >> ;;
> >> esac
> >> }
> >> @@ -261,7 +264,7 @@ __gitcomp ()
> >> __gitcomp_nl ()
> >> {
> >> local IFS=$'\n'
> >> - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
> >> + __gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
> >> }
> >
> > I feel hesitant about this change. One of the ways I'm exploring to
> > fix the issues with shell metacharacters and expansion in compgen is
> > to actually replace compgen. We already iterate over all possible
> > completion words in __gitcomp_1(), so it doesn't make much of a
> > difference to do the filtering for the current word while we are at
> > it. However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen
> > ...)), and tha basic idea of never touching COMPREPLY directly make
> > this basically impossible.
>
> How is it impossible? You can still replace compgen, all you have to
> do is modify __gitcompadd and replace that code with whatever custom
> code you want. You can change the arguments and everything. The only
> limitation is that it should be the only place where COMPREPLY is
> modified, and all is good. Well, it doesn't have to be only _one_
> place, but the less functions that do this, the better.
That's exactly the problem: there isn't, there can't be one single
"whatever custom code I want".
The compgen() in __gitcomp() will be replaced by an enhanced version
of the loop in __gitcomp_1(), while in __gitcomp_nl() it will be
replaced by a little awk scriptlet. And then there is the oddball
$(git ls-tree |sed magic) in __git_complete_revlist_file(), where
possible completion words are filenames possibly containing newlines,
therefore requiring yet another approach.
^ permalink raw reply
* Re: [PATCH v6 2/3] completion: add new __gitcompadd helper
From: Felipe Contreras @ 2012-10-30 23:05 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Junio C Hamano, Jeff King, Matthieu Moy
In-Reply-To: <20121030225839.GO12052@goldbirke>
On Tue, Oct 30, 2012 at 11:58 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Mon, Oct 22, 2012 at 03:45:41AM +0200, Felipe Contreras wrote:
>> The idea is to never touch the COMPREPLY variable directly.
>>
>> This allows other completion systems override __gitcompadd, and do
>> something different instead.
>>
>> Also, this allows the simplification of the completion tests (separate
>> patch).
>
> This doesn't apply anymore, does it? The mentioned simplification is
> done in the other series.
Yeah, but you mentioned you didn't like all the COMPREPLY=() changes
and it might be time to get rid of them.
So this series supersedes that one.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 1/2] completion: refactor __gitcomp related tests
From: Felipe Contreras @ 2012-10-30 23:02 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <20121030224558.GN12052@goldbirke>
On Tue, Oct 30, 2012 at 11:45 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:
>> Lots of duplicated code!
>>
>> No functional changes.
>
> I'm not sure.
> I'm all for removing duplicated application code, but I'm usually more
> conservative when it comes to test code. The more logic, the more
> possibility for bugs in tests. So tests should be dead simple, even
> if that means some duplicated test code or the lack of convenience
> functions.
> While this might be considered just a matter of personal preference, ...
Fair enough, but my preference is different: test code should be the
same as normal code; easy to maintain.
>> +test_gitcomp ()
>> +{
>> + sed -e 's/Z$//' > expected &&
>> (
>> local -a COMPREPLY &&
>> - cur="--re" &&
>> - __gitcomp "--dry-run --reuse-message= --reedit-message=
>> - --reset-author" &&
>> + cur="$1" &&
>> + shift &&
>> + __gitcomp "$@" &&
>
> ... I was really puzzled by how __gitcomp() gets its arguments here,
> and had to think for a while to figure out why it's not broken.
You mean because "$@" is treated in a special way? Without that
behavior I'm not sure many things would be possible :)
Anyway, I don't think we should make our life more difficult while
maintaining test code... that's my opinion.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v6 2/3] completion: add new __gitcompadd helper
From: SZEDER Gábor @ 2012-10-30 22:58 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Jeff King, Matthieu Moy
In-Reply-To: <1350870342-22653-3-git-send-email-felipe.contreras@gmail.com>
On Mon, Oct 22, 2012 at 03:45:41AM +0200, Felipe Contreras wrote:
> The idea is to never touch the COMPREPLY variable directly.
>
> This allows other completion systems override __gitcompadd, and do
> something different instead.
>
> Also, this allows the simplification of the completion tests (separate
> patch).
This doesn't apply anymore, does it? The mentioned simplification is
done in the other series.
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 22:56 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAGdFq_iiGpYW-txPaa6mZrxg3mYdOX-Ez9uLF-rB5bAjZd5rWg@mail.gmail.com>
On Tue, Oct 30, 2012 at 11:35 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 3:18 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> 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
^ permalink raw reply
* Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
From: Albert Krawczyk @ 2012-10-30 22:50 UTC (permalink / raw)
To: kusmabite; +Cc: karsten.blees, git, msysgit
In-Reply-To: <CABPQNSb6QzxxJpoackhzRz19UWwpxjJ6MPEeptTU9Z0bEaAB+A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]
> Wow, awesome results :)
They get even better when used with scripted commands like git svn rebase:
| cold | warm | hot
----------------+-------+------+-----
msysgit-v1.8.0 | 7m29s | 36s | 33s
this patch | 28s | 14s | 14
https://github.com/pro-logic/git/commit/32c03dd8#commitcomment-2070415
Karsten's code is amazing. Now if only I could figure out a way to spread
the love through the rest of the source :)
Albert
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
[-- Attachment #2: Type: text/html, Size: 1546 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] completion: refactor __gitcomp related tests
From: SZEDER Gábor @ 2012-10-30 22:45 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1350869941-22485-2-git-send-email-felipe.contreras@gmail.com>
On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:
> Lots of duplicated code!
>
> No functional changes.
I'm not sure.
I'm all for removing duplicated application code, but I'm usually more
conservative when it comes to test code. The more logic, the more
possibility for bugs in tests. So tests should be dead simple, even
if that means some duplicated test code or the lack of convenience
functions.
While this might be considered just a matter of personal preference, ...
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> t/t9902-completion.sh | 72 ++++++++++++++++-----------------------------------
> 1 file changed, 23 insertions(+), 49 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index cbd0fb6..1c6952a 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -72,87 +72,61 @@ test_completion_long ()
>
> newline=$'\n'
>
> -test_expect_success '__gitcomp - trailing space - options' '
> - sed -e "s/Z$//" >expected <<-\EOF &&
> - --reuse-message=Z
> - --reedit-message=Z
> - --reset-author Z
> - EOF
> +test_gitcomp ()
> +{
> + sed -e 's/Z$//' > expected &&
> (
> local -a COMPREPLY &&
> - cur="--re" &&
> - __gitcomp "--dry-run --reuse-message= --reedit-message=
> - --reset-author" &&
> + cur="$1" &&
> + shift &&
> + __gitcomp "$@" &&
... I was really puzzled by how __gitcomp() gets its arguments here,
and had to think for a while to figure out why it's not broken.
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Sverre Rabbelier @ 2012-10-30 22:35 UTC (permalink / raw)
To: Felipe Contreras
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s2KNmr7zAvFo2gOR8G=YaoBWiGPCjPY47x00eev6MOAFw@mail.gmail.com>
On Tue, Oct 30, 2012 at 3:18 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 22:22 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <20121030220717.GO15167@elie.Belkin>
On Tue, Oct 30, 2012 at 11:07 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> Well, that's what we have now, and you want to preserve this "feature"
>> (aka bug), right?
>
> Nope. I just don't want regressions, and found a patch description
> that did nothing to explain to the reader how it avoids regressions
> more than a little disturbing.
I see, so you don't have any specific case where this could cause
regressions, you are just saying it _might_ (like all patches).
> I also think the proposed behavior is wrong.
That's a different matter, lets see what others think.
--
Felipe Contreras
^ permalink raw reply
* Re: change symlink
From: shawn wilson @ 2012-10-30 22:22 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m2390vobnh.fsf@igel.home>
On Tue, Oct 30, 2012 at 10:09 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> shawn wilson <ag4ve.us@gmail.com> writes:
>
>> and once it's added, status says:
>> # renamed: t2 -> t2/one/test
>>
>> that's not exactly true, but...
>
> What's wrong with it? Both files have the same contents, which is the
> link target for symlinks.
>
ok, you're right about that (another test where i changed where the
symlink pointed):
# deleted: test
# new file: test/one/t3
however (what got me started wondering about this and a point i forgot
about) - t2/one/test doesn't show up under 'untracked files' in in
status that scenario. shouldn't it?
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 22:18 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAGdFq_h3L-1rPvb=dSYeXqEea+f+g2kRHp7aAjaU-AxjZHB7dQ@mail.gmail.com>
On Tue, Oct 30, 2012 at 10:59 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> 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?
>
> 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
^ permalink raw reply
* Re: [PATCH] test-lib: avoid full path to store test results
From: Elia Pinto @ 2012-10-30 22:17 UTC (permalink / raw)
To: Jonathan Nieder, Felipe Contreras, git, Junio C Hamano, Jeff King,
Ævar Arnfjörð, Johannes Sixt
In-Reply-To: <20121030070152.GA13324@elie.Belkin>
Thanks. I know that posix support these usages, but exists some
traditional shell that not support it. These are described in the
autoconf manual, last time i have checked. As the construct ; export
var = x should be portable, but it is not. If this is important these
days i don't know.
Best
2012/10/30, Jonathan Nieder <jrnieder@gmail.com>:
> Elia Pinto wrote:
>
>> The shell word splitting done in base is a bashism, iow not portable.
>
> No, ${varname##glob} is in POSIX and we already use it here and there.
> See Documentation/CodingGuidelines:
>
> - We use ${parameter#word} and its [#%] siblings, and their
> doubled "longest matching" form.
>
> Thanks for looking the patch over.
> Jonathan
>
--
Inviato dal mio dispositivo mobile
^ permalink raw reply
* Re: change symlink
From: Andreas Schwab @ 2012-10-30 22:09 UTC (permalink / raw)
To: shawn wilson; +Cc: git
In-Reply-To: <CAH_OBif=Zr-3GO3CE4D6O3wJJZysB6=vvmA37K6ujTHTr-un7w@mail.gmail.com>
shawn wilson <ag4ve.us@gmail.com> writes:
> and once it's added, status says:
> # renamed: t2 -> t2/one/test
>
> that's not exactly true, but...
What's wrong with it? Both files have the same contents, which is the
link target for symlinks.
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 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 22:07 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s1b+E8a0kdSgREbGzRTFy+nCw4VcjHadd3soQAXRkNzZw@mail.gmail.com>
Felipe Contreras wrote:
> Well, that's what we have now, and you want to preserve this "feature"
> (aka bug), right?
Nope. I just don't want regressions, and found a patch description
that did nothing to explain to the reader how it avoids regressions
more than a little disturbing.
I also think the proposed behavior is wrong.
I don't think there's much benefit to be gained from continuing to
discuss this. Consider it a single data point: I would be deeply
worried if this patch were applied without at least a clearer
description of the change in behavior. Maybe I'm the only one!
Hope that helps,
Jonathan
^ permalink raw reply
* Re: change symlink
From: shawn wilson @ 2012-10-30 22:03 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m27gq7ochp.fsf@igel.home>
On Tue, Oct 30, 2012 at 9:50 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> shawn wilson <ag4ve.us@gmail.com> writes:
>
>> why is this different?
>
> You didn't tell git about t2/one/test. You need to add it first to make
> it known.
>
and once it's added, status says:
# renamed: t2 -> t2/one/test
that's not exactly true, but...
i guess the point is to only track files? if this is the case, i still
disagree with what status is reporting but i suppose it's fine (from a
bug point of view).
^ permalink raw reply
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 22:01 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <20121030214531.GN15167@elie.Belkin>
On Tue, Oct 30, 2012 at 10:45 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> So you think what we have now is the correct behavior:
>>
>> % git fast-export master ^master
>> reset refs/heads/master
>> from :0
>
> No, I don't think that, either.
Well, that's what we have now, and you want to preserve this "feature"
(aka bug), right?
And I still haven't why this is "unsafe", and what are those "examples
not listed".
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Sverre Rabbelier @ 2012-10-30 21:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s2QwdZKqJq0BZ5HOtZYiCMxCxycui9EmxxfL+Sa6M_6+g@mail.gmail.com>
On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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?
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
^ permalink raw reply
* Re: change symlink
From: Andreas Schwab @ 2012-10-30 21:50 UTC (permalink / raw)
To: shawn wilson; +Cc: git
In-Reply-To: <CAH_OBifch3uuXYHQ1R9vS6xFu8LuY3mUfiPsHcs3F=HMvnBzyg@mail.gmail.com>
shawn wilson <ag4ve.us@gmail.com> writes:
> why is this different?
You didn't tell git about t2/one/test. You need to add it first to make
it known.
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 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 21:45 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAMP44s1tFhh3Xqe9tqoDAdtwnGc=kFT6OmAreeP1nbTstweaQQ@mail.gmail.com>
Felipe Contreras wrote:
> So you think what we have now is the correct behavior:
>
> % git fast-export master ^master
> reset refs/heads/master
> from :0
No, I don't think that, either.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 2/2] completion: simplify __gitcomp test helper
From: Felipe Contreras @ 2012-10-30 21:43 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <20121030212725.GA15709@goldbirke>
On Tue, Oct 30, 2012 at 10:27 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 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.
Right, I thought I did that =/
--
Felipe Contreras
^ permalink raw reply
* 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
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