* Re: [PATCH] convert: mark a file-local symbol static
From: Ramsay Jones @ 2016-10-17 21:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Johannes Schindelin, Lars Schneider, GIT Mailing-list
In-Reply-To: <xmqqmvi2u4c0.fsf@gitster.mtv.corp.google.com>
On 17/10/16 21:07, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
>> Heh, I actually have the following in my config.mak already:
>>
>> extra-clean: clean
>> find . -iname '*.o' -exec rm {} \;
>>
>> But for some reason I _always_ type 'make clean' and then, to top
>> it off, I _always_ type the 'find' command by hand (I have no idea
>> why) :-D
>
> "git clean -x" anybody?
I don't use 'git clean' because, on the very few occasions that I have
tried to use it, it always deletes _far_ more than I thought it would
or should. (particularly config.mak). Hmm, "git clean -X -- '*.o'"
_might_ do what I want, but 'find' is so much easier ... :-D
ATB,
Ramsay Jones
^ permalink raw reply
* Problems with "git svn clone"
From: K Richard Pixley @ 2016-10-17 21:03 UTC (permalink / raw)
To: git
I'm trying to create a git-svn repository with:
git svn clone --username=pixleyr --stdlayout --branches branches --branches branches2 svn://mumble.com/mumble/mumble
but the command dies about 11seconds in with:
A src/kernel/ppc/2.4.26-pre5-moto-pq3-2004_06_04-0/drivers/usb/serial/belkin_sa.h
A src/kernel/ppc/2.4.26-pre5-moto-pq3-2004_06_04-0/drivers/usb/serial/keyspan_usa28_fw.h
error: git-svn died of signal 11
This seems awfully early and blatant for a core dump. What can I do to
get this running?
Initially discovered on git-2.7.4, (ubuntu-16.04), but also reproduced
on freshly built top of tree git-2.10.1.445.g3cdd5d1.
--rich
________________________________
- CONFIDENTIAL-
This email and any files transmitted with it are confidential, and may also be legally privileged. If you are not the intended recipient, you may not review, use, copy, or distribute this message. If you receive this email in error, please notify the sender immediately by reply email and then delete this email.
^ permalink raw reply
* Re: [PATCH] cocci: avoid self-references in object_id transformations
From: Junio C Hamano @ 2016-10-17 21:11 UTC (permalink / raw)
To: René Scharfe; +Cc: brian m. carlson, Git List
In-Reply-To: <014ef44e-9dd8-40b3-a3ec-b483f938ee02@web.de>
René Scharfe <l.s.r@web.de> writes:
> I get these instead with 6513eabcbcbcfa684d4bb2d57f61c662b870b5ca on
> Debian testing with its "spatch version 1.0.4 with Python support and
> with PCRE support", which look legit:
They do look good. So I'd stop worrying about it and see how
painful to update my copy of spatch would be.
Thanks.
^ permalink raw reply
* Re: [PATCH] convert: mark a file-local symbol static
From: Stefan Beller @ 2016-10-17 20:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ramsay Jones, Jeff King, Johannes Schindelin, Lars Schneider,
GIT Mailing-list
In-Reply-To: <xmqqmvi2u4c0.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 17, 2016 at 1:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
>> Heh, I actually have the following in my config.mak already:
>>
>> extra-clean: clean
>> find . -iname '*.o' -exec rm {} \;
>>
>> But for some reason I _always_ type 'make clean' and then, to top
>> it off, I _always_ type the 'find' command by hand (I have no idea
>> why) :-D
>
> "git clean -x" anybody?
I only want to cleanup compiled stuff and such, not the precious
unversioned text files I have laying around here. So I guess
git clean -x -e:(attr:!binary)
would suffice, though. ;)
^ permalink raw reply
* Re: [PATCH] convert: mark a file-local symbol static
From: René Scharfe @ 2016-10-17 20:48 UTC (permalink / raw)
To: Junio C Hamano, Ramsay Jones
Cc: Jeff King, Johannes Schindelin, Lars Schneider, GIT Mailing-list
In-Reply-To: <xmqqmvi2u4c0.fsf@gitster.mtv.corp.google.com>
Am 17.10.2016 um 22:07 schrieb Junio C Hamano:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
>> Heh, I actually have the following in my config.mak already:
>>
>> extra-clean: clean
>> find . -iname '*.o' -exec rm {} \;
>>
>> But for some reason I _always_ type 'make clean' and then, to top
>> it off, I _always_ type the 'find' command by hand (I have no idea
>> why) :-D
>
> "git clean -x" anybody?
This removes config.mak as well.
René
^ permalink raw reply
* Re: [GIT PULL] l10n updates for 2.10.0 maint branch
From: Junio C Hamano @ 2016-10-17 20:29 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Ralf Thielow, Dimitriy Ryazantcev
In-Reply-To: <CANYiYbEip5fu9gz24ts_xcPBfPRLMOBrhjfF9gjs=bZym1aizg@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Hi Junio,
>
> Please pull the following l10n updates for Git 2.10 to the maint branch.
>
> The following changes since commit 9a4b694c539fead26833c2104c1a93d3a2b4c50a:
>
> l10n: zh_CN: review for git v2.10.0 l10n (2016-09-11 21:34:23 +0800)
Thanks.
> Dimitriy Ryazantcev (1):
> l10n: ru.po: update Russian translation
>
> Jiang Xin (1):
> Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru
>
> Ralf Thielow (2):
> l10n: de.po: fix translation of autostash
> l10n: de.po: translate 260 new messages
>
> po/de.po | 5182 +++++++++++++++++++++++++++++++++-----------------------------
> po/ru.po | 52 +-
> 2 files changed, 2815 insertions(+), 2419 deletions(-)
>
> --
> Jiang Xin
^ permalink raw reply
* Re: [PATCH] convert: mark a file-local symbol static
From: Junio C Hamano @ 2016-10-17 20:07 UTC (permalink / raw)
To: Ramsay Jones
Cc: Jeff King, Johannes Schindelin, Lars Schneider, GIT Mailing-list
In-Reply-To: <16298e68-c0f9-b7ff-d4b5-0d4b79306377@ramsayjones.plus.com>
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> Heh, I actually have the following in my config.mak already:
>
> extra-clean: clean
> find . -iname '*.o' -exec rm {} \;
>
> But for some reason I _always_ type 'make clean' and then, to top
> it off, I _always_ type the 'find' command by hand (I have no idea
> why) :-D
"git clean -x" anybody?
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Junio C Hamano @ 2016-10-17 20:07 UTC (permalink / raw)
To: Johannes Sixt
Cc: Johannes Schindelin, Stefan Beller, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <2aa41392-a7ed-cd48-2737-bd852757ab35@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>> "../." taken relative to "$(pwd)/." must be "$(pwd)/."
>> "../." taken relative to "$PWD/." must be "$(pwd)/."
>>
>> test, because of the limitation of your bash, cannot have the latter
>> half of the pair, so you'd need to comment it out with in-code
>> explanation, perhaps?
>
> No, we do not have to test both cases. Git, the program, never sees
> Unixy input. It cannot distinguish the two cases.
Thanks.
^ permalink raw reply
* Re: [PATCH] cocci: avoid self-references in object_id transformations
From: René Scharfe @ 2016-10-17 20:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: brian m. carlson, Git List
In-Reply-To: <xmqqlgxmvof2.fsf@gitster.mtv.corp.google.com>
Am 17.10.2016 um 20:08 schrieb Junio C Hamano:
> ... oops. Totally unrelated to this patch, but I see these in
> strbuf.cocci.patch (this is at the tip of 'pu'), which are total
> nonsense. Perhaps I am running a way-stale spatch? It claims to be
> "spatch version 1.0.0-rc19 with Python support and with PCRE support"
>
> --- date.c
> +++ /tmp/cocci-output-21568-bd3448-date.c
> @@ -179,7 +179,7 @@ const char *show_date(unsigned long time
>
> if (mode->type == DATE_UNIX) {
> strbuf_reset(&timebuf);
> - strbuf_addf(&timebuf, "%lu", time);
> + strbuf_addstr(&timebuf, time);
> return timebuf.buf;
> }
>
> --- log-tree.c
> +++ /tmp/cocci-output-21608-b02087-log-tree.c
> @@ -400,7 +400,7 @@ void log_write_email_headers(struct rev_
> extra_headers = subject_buffer;
>
> if (opt->numbered_files)
> - strbuf_addf(&filename, "%d", opt->nr);
> + strbuf_addstr(&filename, opt->nr);
> else
> fmt_output_commit(&filename, commit, opt);
> snprintf(buffer, sizeof(buffer) - 1,
I get these instead with 6513eabcbcbcfa684d4bb2d57f61c662b870b5ca on
Debian testing with its "spatch version 1.0.4 with Python support and
with PCRE support", which look legit:
--- sequencer.c
+++ /tmp/cocci-output-40365-db7a71-sequencer.c
@@ -159,7 +159,7 @@ int sequencer_remove_state(struct replay
free(opts->xopts[i]);
free(opts->xopts);
- strbuf_addf(&dir, "%s", get_dir(opts));
+ strbuf_addstr(&dir, get_dir(opts));
remove_dir_recursively(&dir, 0);
strbuf_release(&dir);
--- builtin/branch.c
+++ /tmp/cocci-output-40858-a86d1a-branch.c
@@ -316,7 +316,7 @@ static char *build_format(struct ref_fil
if (filter->verbose) {
strbuf_addf(&local, "%%(align:%d,left)%%(refname:strip=2)%%(end)", maxwidth);
- strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addstr(&local, branch_get_color(BRANCH_COLOR_RESET));
strbuf_addf(&local, " %%(objectname:short=7) ");
if (filter->verbose > 1)
^ permalink raw reply
* Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
From: Ævar Arnfjörð Bjarmason @ 2016-10-17 19:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narębski, Git Mailing List
In-Reply-To: <xmqqbmyiykzz.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 17, 2016 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> As far as I can tell the only outstanding "change this" is your
>> s/SHA1/SHA-1/ in <xmqq37k9jm86.fsf@gitster.mtv.corp.google.com>, do
>> you want to fix that up or should I submit another series?
>
> I think I did that already myself while queuing. Could you fetch
> what I queued on 'pu' to double check?
Thanks, looked at it, looks good to me!
> I think the diff between what was posted and what is queued (I just
> checked) looks like this:
>
> -gitweb: Link to 7-char+ SHA1s, not only 8-char+
> +gitweb: link to 7-char+ SHA-1s, not only 8-char+
>
> Change the minimum length of an abbreviated object identifier in the
> commit message gitweb tries to turn into link from 8 hexchars to 7.
>
> @@ -5,16 +12,18 @@
> SHA-1 in commit log message links to "object" view", 2006-12-10), but
> the default abbreviation length is 7, and has been for a long time.
>
> -It's still possible to reference SHA1s down to 4 characters in length,
> +It's still possible to reference SHA-1s down to 4 characters in length,
> see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
> git actually produce that, so I doubt anyone is putting that into log
> -messages in practice, but people definitely do put 7 character SHA1s
> +messages in practice, but people definitely do put 7 character SHA-1s
> into log messages.
>
> I think it's fairly dubious to link to things matching [0-9a-fA-F]
> here as opposed to just [0-9a-f], that dates back to the initial
> version of gitweb from 161332a ("first working version",
> -2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
> +2005-08-07). Git will accept all-caps SHA-1s, but didn't ever produce
> them as far as I can tell.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> +Acked-by: Jakub Narębski <jnareb@gmail.com>
> +Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Johannes Sixt @ 2016-10-17 19:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Stefan Beller, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <xmqqfunvxxgu.fsf@gitster.mtv.corp.google.com>
Am 17.10.2016 um 09:10 schrieb Junio C Hamano:
> And I agree from that point of view that having to spell both sides
> as $(pwd) would mean you are not testing that "Unixy input to
> Windowsy output" expectation, but at the same time, I think you
> would want "Windowsy input to Windowsy output" combination also does
> produce correct result, which is not tested in the three tests shown
> above. IOW, probably you would want to test both (at least on any
> platform where $PWD and $(pwd) textually disagree) for all these
> [*1*], and the pair
>
> "../." taken relative to "$(pwd)/." must be "$(pwd)/."
> "../." taken relative to "$PWD/." must be "$(pwd)/."
>
> test, because of the limitation of your bash, cannot have the latter
> half of the pair, so you'd need to comment it out with in-code
> explanation, perhaps?
No, we do not have to test both cases. Git, the program, never sees
Unixy input. It cannot distinguish the two cases. If we did both tests,
we would just test that *if* the front-end of git is an MSYS program
(such as bash), *then* that MSYS front-end has converted the Unixy path
to a Windows-y path in such a way that the end-result is also as
expected. It's similar to a test that grep produces expected output.
I think that we could reduce the confusion by converting all $PWD to
$(pwd) in these test cases. I don't remember why I suggested to use $PWD
for one of the arguments of the test cases (the second must be $(pwd)),
but the most likely reason is only that we save a process.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Junio C Hamano @ 2016-10-17 19:16 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <CAGZ79kZvRf0gHA7tBh1veS9YzAWZ0j0SMGL3c9QQOrERMjmuyQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> Where at the end-user facing level does this trailing "/." surface
>> and how does the difference appear to them? I think that is the
>> crucial question.
>>
>> Unless there is some convincing argument why "." is not special
>> (i.e. counter-argument to the above "bus vs sub" and ". vs sub"
>> example), I would think "existing users with /." does not matter.
>> If they are "relying" on the behaviour, I would think it is not
>> because they find that behaviour intuitive, but only because they
>> learned to live with it. IOW, treating all of A/B/C the same way
>> would appear to them a strict bugfix, I would think.
>
> I see, so we should adapt the windows style and chop off '/.'
> to make A,B,C all the same, because internally we never produced
> C AFAICT.
>
> These came in via hand edited .gitmodules files.
Can you elaborate a bit more on this?
Without seeing "The user added X/. instead of the usual X because
s/he wanted to see Y happen. If s/he had X there, Z would have
happened instead of Y" and why being able to expect Y to happen is a
good thing (compared to Z), we may fail to notice why the more
"intuitive" (at least to me) "these three must result in the same
outcome: path/to/dir, path/to/dir/, or path/to/dir/." does not serve
a legitimate use case.
^ permalink raw reply
* Re: [PATCH v4 00/25] Prepare the sequencer for the upcoming rebase -i patches
From: Junio C Hamano @ 2016-10-17 19:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This patch series marks the '4' in the countdown to speed up rebase -i
> by implementing large parts in C (read: there will be three more patch
> series after that before the full benefit hits git.git: sequencer-i,
> rebase--helper and rebase-i-extra). It is based on the `libify-sequencer`
> patch series I submitted earlier.
The difference between the end results of v3 and v4 looked OK
(except the 08/25 "strip LF" change that is unneeded) to me, so I'll
skip the early part of the series from my review that correspond to
the ones in v3 that I've already reviewed and found good, and
continue from the later ones.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 05/25] sequencer: eventually release memory allocated for the option values
From: Junio C Hamano @ 2016-10-17 19:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <b771bfbc88734b74acda0c377446e984ca81fd9c.1476450940.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
> like a one-shot command when it reads its configuration: memory is
> allocated and released only when the command exits.
>
> This is kind of okay for git-cherry-pick, which *is* a one-shot
> command. All the work to make the sequencer its work horse was
> done to allow using the functionality as a library function, though,
> including proper clean-up after use.
>
> To remedy that, we now take custody of the option values in question,
> requiring those values to be malloc()ed or strdup()ed
That is the approach this patch takes, so "eventually release" in
the title is no longer accurate, I would think.
> Sadly, the current approach makes the code uglier, as we now have to
> take care to strdup() the values passed via the command-line.
I obviously disagree with that statement and the _entrust was too
ugly to live, but it is obviously subjective, and it boils down to
who has a better taste. Let's not go there.
> +
> + /* These option values will be free()d */
> + opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
> + opts->strategy = xstrdup_or_null(opts->strategy);
xstrdup-or-null does make things cleaner.
> +static int git_config_string_dup(char **dest,
> + const char *var, const char *value)
> +{
> + if (!value)
> + return config_error_nonbool(var);
> + free(*dest);
> + *dest = xstrdup(value);
> + return 0;
> +}
So does this.
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Stefan Beller @ 2016-10-17 18:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <xmqqh98avnhh.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 17, 2016 at 11:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> In any case, I find it more disturbing that we somehow ended up with
>>> a system where these three things are expected to behave differently:
>>>
>>> A - path/to/dir
>>> B - path/to/dir/
>>> C - path/to/dir/.
>>>
>>> Is that something we can fix?
>>
>> Well A, B are the same.
>> C is "obviously" different, when it comes to counting slashes for relative
>> path/url purposes, in the way that there are characters after the last slash
>> and just by coincidence '.' refers to the directory itself, C behaving like
>> 'path/to/dir/sub' seems right to me.
>
> It doesn't look right to me at all. If you were contrasting
>
> cd path/to/dir/sub && cd ..
> cd path/to/dir/bus && cd ..
>
> then I would understand, but why should these two
>
> cd path/to/dir/. && cd ..
> cd path/to/dir/sub && cd ..
>
> behave the same?
>
>> So how do you imagine this fix going forward?
>> * Breaking existing users with /. at the end? by treating it the same as A,B
>> * Do some check based on time/version of Git and cover the old data?
>> * Forbid /. at the end from now on?
>
> Where at the end-user facing level does this trailing "/." surface
> and how does the difference appear to them? I think that is the
> crucial question.
>
> Unless there is some convincing argument why "." is not special
> (i.e. counter-argument to the above "bus vs sub" and ". vs sub"
> example), I would think "existing users with /." does not matter.
> If they are "relying" on the behaviour, I would think it is not
> because they find that behaviour intuitive, but only because they
> learned to live with it. IOW, treating all of A/B/C the same way
> would appear to them a strict bugfix, I would think.
I see, so we should adapt the windows style and chop off '/.'
to make A,B,C all the same, because internally we never produced
C AFAICT.
These came in via hand edited .gitmodules files.
>
> It is totally a different matter if OUR code that consumes the
> output from the submodule-helper --resolve-relative" internally is
> confused and relies on "../. relative to path/to/dir/. is the same
> as ../. relative to path/to/dir/sub" for whatever reason. Without
> fixing that, I would not surprised if fixing things to treat A/B/C
> the same way would surface differences in the end-user observable
> behaviour in a negative way.
>
^ permalink raw reply
* Re: [PATCH v11 00/14] Git filter protocol
From: Junio C Hamano @ 2016-10-17 18:47 UTC (permalink / raw)
To: larsxschneider; +Cc: git, jnareb, peff, ramsay, tboegi
In-Reply-To: <20161016232038.84951-1-larsxschneider@gmail.com>
larsxschneider@gmail.com writes:
> The goal of this series is to avoid launching a new clean/smudge filter
> process for each file that is filtered.
>
> A short summary about v1 to v5 can be found here:
> https://git.github.io/rev_news/2016/08/17/edition-18/
>
> This series is also published on web:
> https://github.com/larsxschneider/git/pull/15
>
> Patches 1 and 2 are cleanups and not strictly necessary for the series.
> Patches 3 to 12 are required preparation. Patch 13 is the main patch.
> Patch 14 adds an example how to use the Git filter protocol in contrib.
Will replace. If you ever need tor reroll 13, please squash the
following in (which I already did locally so there is no need to
resend only to correct it).
Thanks.
convert.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/convert.c b/convert.c
index 9d2aa68df9..bc242276ff 100644
--- a/convert.c
+++ b/convert.c
@@ -535,7 +535,8 @@ static int packet_write_list(int fd, const char *line, ...)
return packet_flush_gently(fd);
}
-static void read_multi_file_filter_status(int fd, struct strbuf *status) {
+static void read_multi_file_filter_status(int fd, struct strbuf *status)
+{
struct strbuf **pair;
char *line;
for (;;) {
--
2.10.1-613-g6ad57fc60c
^ permalink raw reply related
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Junio C Hamano @ 2016-10-17 18:28 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <CAGZ79kYW2qK1GKxoKy_mkVkjjqEUzkh5aPSzDEfRd6U=PYDdzw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> In any case, I find it more disturbing that we somehow ended up with
>> a system where these three things are expected to behave differently:
>>
>> A - path/to/dir
>> B - path/to/dir/
>> C - path/to/dir/.
>>
>> Is that something we can fix?
>
> Well A, B are the same.
> C is "obviously" different, when it comes to counting slashes for relative
> path/url purposes, in the way that there are characters after the last slash
> and just by coincidence '.' refers to the directory itself, C behaving like
> 'path/to/dir/sub' seems right to me.
It doesn't look right to me at all. If you were contrasting
cd path/to/dir/sub && cd ..
cd path/to/dir/bus && cd ..
then I would understand, but why should these two
cd path/to/dir/. && cd ..
cd path/to/dir/sub && cd ..
behave the same?
> So how do you imagine this fix going forward?
> * Breaking existing users with /. at the end? by treating it the same as A,B
> * Do some check based on time/version of Git and cover the old data?
> * Forbid /. at the end from now on?
Where at the end-user facing level does this trailing "/." surface
and how does the difference appear to them? I think that is the
crucial question.
Unless there is some convincing argument why "." is not special
(i.e. counter-argument to the above "bus vs sub" and ". vs sub"
example), I would think "existing users with /." does not matter.
If they are "relying" on the behaviour, I would think it is not
because they find that behaviour intuitive, but only because they
learned to live with it. IOW, treating all of A/B/C the same way
would appear to them a strict bugfix, I would think.
It is totally a different matter if OUR code that consumes the
output from the submodule-helper --resolve-relative" internally is
confused and relies on "../. relative to path/to/dir/. is the same
as ../. relative to path/to/dir/sub" for whatever reason. Without
fixing that, I would not surprised if fixing things to treat A/B/C
the same way would surface differences in the end-user observable
behaviour in a negative way.
^ permalink raw reply
* Re: [PATCH] cocci: avoid self-references in object_id transformations
From: Junio C Hamano @ 2016-10-17 18:08 UTC (permalink / raw)
To: René Scharfe; +Cc: brian m. carlson, Git List
In-Reply-To: <20161015134503.u3aznujploqee2le@vauxhall.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On Sat, Oct 15, 2016 at 10:25:34AM +0200, René Scharfe wrote:
>> The object_id functions oid_to_hex, oid_to_hex_r, oidclr, oidcmp, and
>> oidcpy are defined as wrappers of their legacy counterparts sha1_to_hex,
>> sha1_to_hex_r, hashclr, hashcmp, and hashcpy, respectively. Make sure
>> that the Coccinelle transformations for converting legacy function calls
>> are not applied to these wrappers themselves, which would result in
>> tautological declarations.
>
> Ah, yes, this is a good idea. I've had to hack around this, but this is
> much better than having to fix it up by hand.
Yes, seeing an empty *.cocci.patch files after running coccicheck is
a great feeling, but without something like this patch, we can never
reach that goal ;-)
Thanks.
... oops. Totally unrelated to this patch, but I see these in
strbuf.cocci.patch (this is at the tip of 'pu'), which are total
nonsense. Perhaps I am running a way-stale spatch? It claims to be
"spatch version 1.0.0-rc19 with Python support and with PCRE support"
--- date.c
+++ /tmp/cocci-output-21568-bd3448-date.c
@@ -179,7 +179,7 @@ const char *show_date(unsigned long time
if (mode->type == DATE_UNIX) {
strbuf_reset(&timebuf);
- strbuf_addf(&timebuf, "%lu", time);
+ strbuf_addstr(&timebuf, time);
return timebuf.buf;
}
--- log-tree.c
+++ /tmp/cocci-output-21608-b02087-log-tree.c
@@ -400,7 +400,7 @@ void log_write_email_headers(struct rev_
extra_headers = subject_buffer;
if (opt->numbered_files)
- strbuf_addf(&filename, "%d", opt->nr);
+ strbuf_addstr(&filename, opt->nr);
else
fmt_output_commit(&filename, commit, opt);
snprintf(buffer, sizeof(buffer) - 1,
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Stefan Beller @ 2016-10-17 17:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker
In-Reply-To: <xmqqfunvxxgu.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 17, 2016 at 12:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>> > expecting success:
>>> > actual=$(git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' '../.') &&
>>> > test "$actual" = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
>>> >
>>> > +++ git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' ../.
>>> > ++ actual=C:/git-sdk-64/usr/src/git/wip/t/.
>>> > ++ test C:/git-sdk-64/usr/src/git/wip/t/. = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
>
> This may well be total misunderstanding on my side, but is the
> expectation of this test even correct? If it wants to take "../."
> relative to "$LEAD/t/trash utils/.", should't it go one level up
> with ".." to $LEAD/t and then stay there with ".", expecting
> "$LEAD/t" which is what the above is giving us?
>
> IOW, the above makes me wonder why having one of these as the base
>
> A - path/to/dir
> B - path/to/dir/
> C - path/to/dir/.
>
> to resolve the relative "../." give different results.
Because the shell script originally did "just"
relative="../."
if path/to/dir/ ends with slash, chop it off.
while $relative starts with "../";
do
chop off starting '../' of relative
chop of last '/' and following from "path/to/dir/."
done
(Linux:)
As B was made to A first, only C differs as a result, because
you had one more '/' in there.
(Windows:)
However Windows also detects '/.' (C) and makes it an A,
(in C only, because shell code was not treated Windows-sy)
which is where the incompatibility between Windows and other
platforms arises.
So we have a couple of choices (for Git)now:
* go back to using shell only for submodule things as that doesn't
have the regression and it alos plays nicely with Git for Windows.
* use C for the submodule code in Git and revert the regression fix
"because consistency across platforms trumps
consistency over time"
* use C for the submodule code in Git and keep the regression fix
"because consistency over time in Git proper is more important
than playing nicely with Git for Windows"
* would it be possible to revert this to shell on Windows only?
> Whether bash
> on Windows removes the dot at the end of C to turn it into B, as
> long as A and B give us the same result we wouldn't be hitting the
> problem, no?
Well in Git proper A,B are the same and C is different.
(B was fixed as a regression)
In Windows C is like B, which was different without the regression
fix, but now it is the same as A, too.
>
>>> > test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
>>> > test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
>>> > -test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
>>> > +test_submodule_relative_url "(null)" "$(pwd)/." "../." "$(pwd)/."
>
>>> > The reasons this is ugly: we specifically test for *Unixy* paths when we
>>> > use $PWD, as opposed to *Windowsy* paths when using $(pwd).
>
> Just to ensure I am following this correctly, two tests that come
> before the one you are touching above have $PWD on the input side
> and $(pwd) on the expectation side. That is what you mean by the
> next paragraph, right? They want to make sure that you honor the
> Unixy user input on Windows and still produce Windowsy result, that
> is.
>
>>> > We do this to
>>> > ensure a certain level of confidence that running things such as
>>> >
>>> > git clone --recurse-submodules /z/project/.
>>> >
>>> > work. And now that does not work anymore.
>
> And I agree from that point of view that having to spell both sides
> as $(pwd) would mean you are not testing that "Unixy input to
> Windowsy output" expectation, but at the same time, I think you
> would want "Windowsy input to Windowsy output" combination also does
> produce correct result, which is not tested in the three tests shown
> above. IOW, probably you would want to test both (at least on any
> platform where $PWD and $(pwd) textually disagree) for all these
> [*1*], and the pair
>
> "../." taken relative to "$(pwd)/." must be "$(pwd)/."
> "../." taken relative to "$PWD/." must be "$(pwd)/."
>
> test, because of the limitation of your bash, cannot have the latter
> half of the pair, so you'd need to comment it out with in-code
> explanation, perhaps? IOW something along the lines of...
>
> -- >8 -- snip -- >8 --
>
> test_submodule_relative_url "(null)" "$(pwd)/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
> test_submodule_relative_url "(null)" "$(pwd)/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
> test_submodule_relative_url "(null)" "$(pwd)/." "../." "$(pwd)/."
>
> if test_have_prereq MINGW
> then
>
> test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
> test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
> # This does not work correctly because Win-Bash strips . at the end
> # "of $PWD/."
> # test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
>
> fi
>
> -- >8 -- snip -- >8 --
>
> In any case, I find it more disturbing that we somehow ended up with
> a system where these three things are expected to behave differently:
>
> A - path/to/dir
> B - path/to/dir/
> C - path/to/dir/.
>
> Is that something we can fix?
Well A, B are the same.
C is "obviously" different, when it comes to counting slashes for relative
path/url purposes, in the way that there are characters after the last slash
and just by coincidence '.' refers to the directory itself, C behaving like
'path/to/dir/sub' seems right to me.
So how do you imagine this fix going forward?
* Breaking existing users with /. at the end? by treating it the same as A,B
* Do some check based on time/version of Git and cover the old data?
* Forbid /. at the end from now on?
>
>
> [Footnote]
>
> *1* It is tempting to update the above test sequence using
> a helper like:
>
> tsru () {
> test_submodule_relative_url "(null)" "$(pwd)/$1" "$2" "$(pwd)/$3"
> if test_have_prereq MINGW
> then
> test_submodule_relative_url "(null)" "$PWD/$1" "$2" "$(pwd)/$3"
> fi
> }
>
> then write the above three tests like so:
>
> tsru subsuper_update_r ../subsubsuper_update_r subsubsuper_update_r
> tsru super_update_r2 ../subsuper_update_r subsuper_update_r
> tsru . ../. .
>
> but you would want to disable the MINGW half for only the third
> test, we cannot quite do that.
^ permalink raw reply
* Re: Merge conflicts in .gitattributes can cause trouble
From: Junio C Hamano @ 2016-10-17 17:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Lars Schneider, git, Jeff King, me
In-Reply-To: <alpine.DEB.2.20.1610171803400.197091@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I would vote for:
>
> 4. We keep letting Git read in the *current* version of .gitattributes
> *before* the merge, and apply those attributes while performing the
> merge.
Even though this needs a major surgery to the way the attr subsystem
reads from these files, I think it is conceptually the cleanest.
^ permalink raw reply
* Re: [RFC] Case insensitive Git attributes
From: Junio C Hamano @ 2016-10-17 17:50 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Johannes Schindelin, Stefan Beller, Lars Schneider, git
In-Reply-To: <CACsJy8ASnvryrrWvr4ACWL_9oeKgE_4SUqcRHGf+Y6kcaeDUhg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> I agree. Which is why I wrote "we probably want something in the same
> spirit but limited to .gitattributes and .gitignore only". In other
> words we could have core.someName that makes .gitattributes and
> .gitignore patterns case-insensitive (or core-sensitive). If it's
> present, it overrides core.ignoreCase. If it's not present,
> core.ignoreCase decides. I'm just not sure if the new config should
> cover everything involving filename's case in git. That's too big to
> fit in my head.
Once I stopped thinking about this as "filename's case", it does fit
my head ;-)
I view the proposed knob as making patterns in .gitattributes and
.gitignore case insensitive, iow, it is a lazy and useful short-hand
for (mentally) editing "*.c attr" to "*.[cC] attr" without touching
these files.
And I agree that the knob that is missing in today's Git should
default to whatever core.ignoreCase's value is, iow, on case
insensitive filesystem, attr and ignore may match case insensitively
in today's Git, but when the knob is introduced, it should allow
forcing case sensitive match there by setting it to false, just like
the knob is proposed to be used in the oppositite direction to force
case insensitive match regardless of the case insensitiveness of the
underlying filesystem.
^ permalink raw reply
* Re: [RFC] test-lib: detect common misuse of test_expect_failure
From: Junio C Hamano @ 2016-10-17 17:36 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161014235718.2rbejrzs5axhopt7@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I like the general idea, but I'm not sure how this would interact with
> the tests in t0000 that test the test suite.
I tried but gave up adding a new test for this to t0000 ;-)
>> test_expect_failure () {
>> + if test "$test_in_progress" = 1
>> + then
>> + error "bug in the test script: did you mean test_must_fail instead of test_expect_failure?"
>> + fi
>
> This follows existing practice for things like the &&-lint-checker, and
> bails out on the whole test script.
Yes, you guessed correctly where the above came from.
> That sometimes makes it hard to find
> the problematic test, especially if you're running via something like
> "prove", because it doesn't make valid TAP output.
Yeah, true.
> It might be nicer if we just said "this test is malformed, and therefore
> fails", and then you get all the usual niceties for recording and
> finding the failed test.
>
> I don't think it would be robust enough to try to propagate the error up
> to the outer test_expect_success block (and anyway, you'd also want to
> know about it in a test_expect_failure block; it's a bug in the test,
> not a known breakage). But perhaps error() could dump some TAP-like
> output with a "virtual" failed test.
>
> Something like:
> ...
> which lets "make prove" collect the broken test number.
>
> It would perhaps need to cover the case when $test_count is "0"
> separately. I dunno. It would be nicer still if we could continue
> running other tests in the script, but I think it's impossible to
> robustly jump back to the outer script.
>
> These kinds of "bug in the test suite" are presumably rare enough that
> the niceties don't matter that much, but I trigger the &&-checker
> reasonably frequently (that and test_line_count, because I can never
> remember the correct invocation).
>
> Anyway. That's all orthogonal to your patch. I just wondered if we could
> do better, but AFAICT the right way to do better is to hook into
> error(), which means your patch would not have to care exactly how it
> fails.
Yeah, the change to error() may be a good thing to do, but it has
quite a many callers in t/*lib*.sh and definitely deserves to be a
separate patch, not tied to this single test.
^ permalink raw reply
* Re: [PATCH] fetch: use "quick" has_sha1_file for tag following
From: Junio C Hamano @ 2016-10-17 17:30 UTC (permalink / raw)
To: Jeff King
Cc: Vegard Nossum, git, Quentin Casasnovas, Shawn Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20161014185953.k4b5xwihlgvxurjc@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Still an impressive speedup as a percentage, but negligible in absolute
> terms. But that's on a local filesystem on a Linux machine. I'd worry
> much more about a system with a slow readdir(), e.g., due to NFS.
> Somebody's real-world NFS case[1] was what prompted us to do 0eeb077
> (index-pack: avoid excessive re-reading of pack directory, 2015-06-09).
Yes.
> It looks like I _did_ look into optimizing this into a single stat()
> call in the thread at [1]. I completely forgot about that. I did find
> there that naively using stat_validity() on a directory is racy, though
> I wonder if we could do something clever with gettimeofday() instead.
It feels funny to hear an idea to compare fs timestamp with gettimeofday
immedately after hearing the word NFS, though ;-).
>> I agree that the fallout from the inaccuracy of "quick" approach is
>> probably acceptable and the next "fetch" will correct it anyway, so
>> let's do the "quick but inaccurate" for now and perhaps cook it in
>> 'next' for a bit longer than other topics?
>
> I doubt that cooking in 'next' for longer will turn up anything useful.
> The case we care about is the race between a repack and a fetch. We
> lived with the "quick" version of has_sha1_file() everywhere for 8
> years.
A very convincing argument. I stand corrected.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 16/25] sequencer: support amending commits
From: Junio C Hamano @ 2016-10-17 17:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <ec146dc2f3a3698bd433bbec4d7ecbab762c1bfb.1476120229.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This teaches the run_git_commit() function to take an argument that will
> allow us to implement "todo" commands that need to amend the commit
> messages ("fixup", "squash" and "reword").
Likewise to 15/25, i.e. Good, though the growth by these two steps
starts to make me wonder if these three options should be crammed
into an unsigned "flags" bitword.
I see you have v4, so I'll ignore the remainder of this stale round
and start reading that updated one instead.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index b621f4b..403a4f0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -481,7 +481,7 @@ static char **read_author_script(void)
> * author metadata.
> */
> static int run_git_commit(const char *defmsg, struct replay_opts *opts,
> - int allow_empty, int edit)
> + int allow_empty, int edit, int amend)
> {
> char **env = NULL;
> struct argv_array array;
> @@ -510,6 +510,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
> argv_array_push(&array, "commit");
> argv_array_push(&array, "-n");
>
> + if (amend)
> + argv_array_push(&array, "--amend");
> if (opts->gpg_sign)
> argv_array_pushf(&array, "-S%s", opts->gpg_sign);
> if (opts->signoff)
> @@ -785,7 +787,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> }
> if (!opts->no_commit)
> res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
> - opts, allow, opts->edit);
> + opts, allow, opts->edit, 0);
>
> leave:
> free_message(commit, &msg);
^ permalink raw reply
* Re: [PATCH] convert: mark a file-local symbol static
From: Ramsay Jones @ 2016-10-17 17:21 UTC (permalink / raw)
To: Jeff King, Johannes Schindelin
Cc: Lars Schneider, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20161017093754.wah5d6cg4qgtw7ln@sigill.intra.peff.net>
On 17/10/16 10:37, Jeff King wrote:
> On Mon, Oct 17, 2016 at 11:04:19AM +0200, Johannes Schindelin wrote:
>
>>> Gross. I would not be opposed to a Makefile rule that outputs the
>>> correct set of OBJECTS so this (or other) scripts could build on it.
>>
>> You could also use the method I use in Git for Windows to "extend" the
>> Makefile:
>>
>> -- snipsnap --
>> cat >dummy.mak <<EOF
>> include Makefile
>>
>> blub: $(OBJECTS)
>> do-something-with $^
>> EOF
>>
>> make -f dummy.mak blub
>
> Hacky but clever. I like it.
>
> In the particular case of git, I think I've cheated similarly before by
> putting things in config.mak, though of course an arbitrary script can't
> assume it can overwrite that file.
Heh, I actually have the following in my config.mak already:
extra-clean: clean
find . -iname '*.o' -exec rm {} \;
But for some reason I _always_ type 'make clean' and then, to top
it off, I _always_ type the 'find' command by hand (I have no idea
why) :-D
ATB,
Ramsay Jones
^ 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