* Re: Getting started contributing.
From: Junio C Hamano @ 2013-02-03 19:46 UTC (permalink / raw)
To: Duy Nguyen; +Cc: adamfraser, git
In-Reply-To: <CACsJy8AYOAwLKufQ34brk1agyFAX9xjgAE9_LAcRx=RGxcEZzg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Sun, Feb 3, 2013 at 2:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> On the other hand, there probably still are many loose ends.
>
> A few other things
>
> - Mark more strings for translation (not as easy as it sounds, some
> strings can't be translated)
True, but not a good advice for somebody new, exactly for the reason
you stated, i.e. some strings must not be translated.
> - Color more in the output where it makes sense
Eeek.
> - Stop/Warn the user from updating HEAD (e.g. checkout another
> branch) while in the middle of a rebase (some makes sense, most is by
> mistake)
Perhaps, but again probably not for somebody new who hasn't mastered
various workflows and understood why it may make sense to allow it.
^ permalink raw reply
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: Junio C Hamano @ 2013-02-03 19:58 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <6995fd5e4d9cb3320ab80c983f1b25ae8a399284.1359901732.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> When compiling combine-diff.c, clang 3.2 says:
>
> combine-diff.c:1006:19: warning: adding 'int' to a string does not
> append to the string [-Wstring-plus-int]
> prefix = COLONS + offset;
> ~~~~~~~^~~~~~~~
> combine-diff.c:1006:19: note: use array indexing to silence this warning
> prefix = COLONS + offset;
> ^
> & [ ]
>
> Suppress this by making the suggested change.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
This was not lost in the noise.
I thought that this wasn't a serious patch, but your attempt to
demonstrate to others why patches trying to squelch clang warnings
are not necessarily a good thing to do.
Who is that compiler trying to help with such a warning message?
After all, we are writing in C, and clang is supposed to be a C
compiler. And adding integer to a pointer to (const) char is a
straight-forward way to look at the trailing part of a given string.
> - prefix = COLONS + offset;
> + prefix = &COLONS[offset];
In other words, both are perfectly valid C. Why should we make it
less readable to avoid a stupid compiler warning?
^ permalink raw reply
* Re: [PATCH] Handle path completion and colon for tcsh script
From: Manlio Perillo @ 2013-02-03 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marc Khouzam, git@vger.kernel.org
In-Reply-To: <7vboc2bhpj.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 02/02/2013 21:10, Junio C Hamano ha scritto:
> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>
>> Recent enhancements to git-completion.bash provide
>> intelligent path completion for git commands. Such
>> completions do not add the '/' at the end of directories
>> for recent versions of bash.
>> ...
>> Here is the update for tcsh completion which is needed to handle
>> the cool new path completion feature just pushed to 'next'.
>>
> [...]
> But I have to wonder if this is sweeping a problem under the rug.
> Shouldn't the completion for bash users end completed directory name
> with '/', even if we didn't have to worry about tcsh?
>
The problem is that when using the "new"
`compopt -o filenames` command, Bash assumes COMPREPLY contains a list
of filenames, and when it detects a directory name, it adds a slash.
The problem is, if the directory name *already* has a slash, Bash adds
another slash!
I don't know if this can be considered a bug or a feature.
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlEOwaQACgkQscQJ24LbaUSjwgCfbgb1id5DcNG0Q75FWwgNPCqb
qkUAnAmMzCahB745/BWeDJTHbJFXucxs
=vf+P
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Jonathan Nieder @ 2013-02-03 20:12 UTC (permalink / raw)
To: Anand Kumria; +Cc: Paul Mackerras, Junio C Hamano, git
In-Reply-To: <CAM1C4G=mKzfgrfO-n_yXjyDkZ9vA7H6kQEY5Yja-5C-YRRLmyw@mail.gmail.com>
Hi Anand,
Anand Kumria wrote:
> I've not been able to find the canonical location of your gitk repository.
Here's how I find it:
$ git clone git://repo.or.cz/git.git
[...]
$ cd git
$ git log -1 --oneline -- gitk-git
ec3ae6ec Merge git://ozlabs.org/~paulus/gitk
$ cd ..
$ git clone git://ozlabs.org/~paulus/gitk.git
Patches, including documentation patches, go to git@vger.kernel.org,
cc-ing Paul Mackerras.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Junio C Hamano @ 2013-02-03 20:23 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Anand Kumria, Paul Mackerras, git
In-Reply-To: <20130203201217.GB3221@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Anand Kumria wrote:
>
>> I've not been able to find the canonical location of your gitk repository.
>
> Here's how I find it:
>
> $ git clone git://repo.or.cz/git.git
> [...]
> $ cd git
> $ git log -1 --oneline -- gitk-git
> ec3ae6ec Merge git://ozlabs.org/~paulus/gitk
> $ cd ..
> $ git clone git://ozlabs.org/~paulus/gitk.git
>
> Patches, including documentation patches, go to git@vger.kernel.org,
> cc-ing Paul Mackerras.
Or look at "A note from the maintainer" posted here every few
months.
^ permalink raw reply
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: John Keeping @ 2013-02-03 20:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <7vwqup890o.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 11:58:15AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > When compiling combine-diff.c, clang 3.2 says:
> >
> > combine-diff.c:1006:19: warning: adding 'int' to a string does not
> > append to the string [-Wstring-plus-int]
> > prefix = COLONS + offset;
> > ~~~~~~~^~~~~~~~
> > combine-diff.c:1006:19: note: use array indexing to silence this warning
> > prefix = COLONS + offset;
> > ^
> > & [ ]
> >
> > Suppress this by making the suggested change.
> >
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> > ---
>
> This was not lost in the noise.
>
> I thought that this wasn't a serious patch, but your attempt to
> demonstrate to others why patches trying to squelch clang warnings
> are not necessarily a good thing to do.
>
> Who is that compiler trying to help with such a warning message?
> After all, we are writing in C, and clang is supposed to be a C
> compiler. And adding integer to a pointer to (const) char is a
> straight-forward way to look at the trailing part of a given string.
A quick search turned up the original thread where this feature was
added to Clang [1]. It seems that it does find genuine bugs where
people try to log values by doing:
log("failed to handle error: " + errno);
[1] http://thread.gmane.org/gmane.comp.compilers.clang.scm/47203
> > - prefix = COLONS + offset;
> > + prefix = &COLONS[offset];
>
> In other words, both are perfectly valid C. Why should we make it
> less readable to avoid a stupid compiler warning?
Are you happy to change COLONS to a const char[] instead of a #define?
That also suppresses the warning.
Since Git is warning-free on GCC and so close to being warning-free on
recent Clang I think it is worthwhile to fix the remaining two issues
which do seem to be intentional diagnostics rather than Clang bugs.
John
^ permalink raw reply
* Re: [PATCH] Handle path completion and colon for tcsh script
From: Junio C Hamano @ 2013-02-03 20:43 UTC (permalink / raw)
To: Manlio Perillo; +Cc: Marc Khouzam, git@vger.kernel.org
In-Reply-To: <510EC1A4.4000207@gmail.com>
Manlio Perillo <manlio.perillo@gmail.com> writes:
> The problem is that when using the "new"
> `compopt -o filenames` command, Bash assumes COMPREPLY contains a list
> of filenames, and when it detects a directory name, it adds a slash.
>
> The problem is, if the directory name *already* has a slash, Bash adds
> another slash!
So bash users do see the trailing slash because bash adds one to
what we compute and return, which we do strip the trailing slash
exactly because we know bash will add one. Because tcsh completion
uses what we compute directly, without bash massaging our output to
add the trailing slash, it needs some magic.
OK, that makes sense. It was this part from the originally proposed
log message:
>> ... Such completions do not add the '/' at the end of directories
>> for recent versions of bash. However, the '/' is needed by tcsh,
>> ...
with a large gap between the two sentences that fooled me, and the
explanation in your message helped to fill the gap to understand the
situation better.
Perhaps
... for recent versions of bash, which will then add the
trailing slash for paths that are directory to the result of
our completion. The completion for tcsh however uses the
result of our completion directly, so it either needs to add
the necessary slash itself, or needs to ask us to keep the
trailiing slash. This patch does the latter.
or something?
^ permalink raw reply
* Re: [PATCH 3/3] builtin/apply: tighten (dis)similarity index parsing
From: Junio C Hamano @ 2013-02-03 20:50 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <2cac21192f79f9fbb5822417775954eba29064fa.1359901732.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 6c11e8b..4745e75 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1041,15 +1041,17 @@ static int gitdiff_renamedst(const char *line, struct patch *patch)
>
> static int gitdiff_similarity(const char *line, struct patch *patch)
> {
> - if ((patch->score = strtoul(line, NULL, 10)) == ULONG_MAX)
> - patch->score = 0;
> + unsigned long val = strtoul(line, NULL, 10);
> + if (val <= 100)
> + patch->score = val;
> return 0;
> }
>
> static int gitdiff_dissimilarity(const char *line, struct patch *patch)
> {
> - if ((patch->score = strtoul(line, NULL, 10)) == ULONG_MAX)
> - patch->score = 0;
> + unsigned long val = strtoul(line, NULL, 10);
> + if (val <= 100)
> + patch->score = val;
> return 0;
> }
This makes sort of sense; .score is used only for display and not
for making any decision, so as long as you know it is initialized to
zero when the call to this function is made, it should be OK.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: Junio C Hamano @ 2013-02-03 21:07 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <20130203203150.GU1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> A quick search turned up the original thread where this feature was
> added to Clang [1]. It seems that it does find genuine bugs where
> people try to log values by doing:
>
> log("failed to handle error: " + errno);
To be perfectly honest, anybody who writes such a code should be
sent back to school before trying to touch out code ever again ;-).
It is not even valid Python, Perl nor Java, I would think.
> Are you happy to change COLONS to a const char[] instead of a #define?
Happy? Not really.
It could be a good change for entirely different reason. We will
save space if we ever need to use it in multiple places. But the
entire "COLONS + offset" thing was a hack we did, knowing that it
will break when we end up showing a muiti-way diff for more than 32
blobs.
If we were to be touching that area of code, I'd rather see a change
to make it more robust against such a corner case. If it results in
squelching misguided clang warnings against programmers who should
not be writing in C, that is a nice side effect, but I loathe to see
any change whose primary purpose is to squelch pointless warnings.
combine-diff.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index bb1cc96..7f6187f 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -982,14 +982,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
free(sline);
}
-#define COLONS "::::::::::::::::::::::::::::::::"
-
static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev)
{
struct diff_options *opt = &rev->diffopt;
- int i, offset;
- const char *prefix;
- int line_termination, inter_name_termination;
+ int line_termination, inter_name_termination, i;
line_termination = opt->line_termination;
inter_name_termination = '\t';
@@ -1000,17 +996,14 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
show_log(rev);
if (opt->output_format & DIFF_FORMAT_RAW) {
- offset = strlen(COLONS) - num_parent;
- if (offset < 0)
- offset = 0;
- prefix = COLONS + offset;
+ /* As many colons as there are parents */
+ for (i = 0; i < num_parent; i++)
+ putchar(':');
/* Show the modes */
- for (i = 0; i < num_parent; i++) {
- printf("%s%06o", prefix, p->parent[i].mode);
- prefix = " ";
- }
- printf("%s%06o", prefix, p->mode);
+ for (i = 0; i < num_parent; i++)
+ printf("%06o ", p->parent[i].mode);
+ printf("%06o", p->mode);
/* Show sha1's */
for (i = 0; i < num_parent; i++)
^ permalink raw reply related
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Matthieu Moy @ 2013-02-03 21:23 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Niedier
In-Reply-To: <1359870520-22644-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
> cp .git/BISECT_START saved &&
> test_must_fail git bisect start $HASH4 foo -- &&
> git branch > branch.output &&
> - test_i18ngrep "* (no branch)" branch.output > /dev/null &&
> + test_i18ngrep "* (bisecting other)" branch.output > /dev/null &&
I'd have spelled it (no branch, bisecting other) to make it clear that
we're on detached HEAD, and avoid confusing old-timers. But maybe your
version is enough, I'm not sure.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Junio C Hamano @ 2013-02-03 21:58 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Nguyễn Thái Ngọc Duy, git, Jonathan Niedier
In-Reply-To: <vpqpq0hnlb1.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> --- a/t/t6030-bisect-porcelain.sh
>> +++ b/t/t6030-bisect-porcelain.sh
>> @@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
>> cp .git/BISECT_START saved &&
>> test_must_fail git bisect start $HASH4 foo -- &&
>> git branch > branch.output &&
>> - test_i18ngrep "* (no branch)" branch.output > /dev/null &&
>> + test_i18ngrep "* (bisecting other)" branch.output > /dev/null &&
>
> I'd have spelled it (no branch, bisecting other) to make it clear that
> we're on detached HEAD, and avoid confusing old-timers. But maybe your
> version is enough, I'm not sure.
Yeah, I do not think "bisecting other" alone makes much sense.
What does "other" refer to when you start your bisection at a
detached head? I personally think "other" has _any_ value in that
message, because "(no branch, bisecting)" gives the same amount of
information, especially because "other" does not say which branch it
refers to at all.
^ permalink raw reply
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Junio C Hamano @ 2013-02-03 22:00 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Nguyễn Thái Ngọc Duy, git, Jonathan Niedier
In-Reply-To: <7v1ucx83fz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Yeah, I do not think "bisecting other" alone makes much sense.
>
> What does "other" refer to when you start your bisection at a
> detached head? I personally think "other" has _any_ value in that
s/_any_/_no_/; obviously ;-)
> message, because "(no branch, bisecting)" gives the same amount of
> information, especially because "other" does not say which branch it
> refers to at all.
^ permalink raw reply
* [New Feature] git-submodule-move - Easily move submodules
From: TJ @ 2013-02-03 22:36 UTC (permalink / raw)
To: git
I've recently had need to re-arrange more than ten submodules within a project and discovered there is apparently no easy way to do it.
Using some suggestions I found on Stack Overflow I eventually figured out the steps required. Because the steps can be
complex I thought it would be handy to have a tool to automate the functionality.
I have put together a reasonably bullet-proof shell script "git-submodule-move" that does the job pretty well. I've put it through quite a bit of testing and trusted it with my own project and it has
performed well.
I've published to github so others can use and improve it.
https://github.com/iam-TJ/git-submodule-move
^ permalink raw reply
* Re: [PATCH] git p4: chdir resolves symlinks only for relative paths
From: Pete Wyckoff @ 2013-02-03 23:08 UTC (permalink / raw)
To: Miklós Fazekas; +Cc: git, Gary Gibbons
In-Reply-To: <CAAMmcSSEzs3+vZDO=FDMV9c2rp-8HTdMuPeeQCkok6y7sRDYJw@mail.gmail.com>
mfazekas@szemafor.com wrote on Tue, 29 Jan 2013 09:37 +0100:
> If a p4 client is configured to /p/foo which is a symlink
> to /vol/bar/projects/foo, then resolving symlink, which
> is done by git-p4's chdir will confuse p4: "Path
> /vol/bar/projects/foo/... is not under client root /p/foo"
> While AltRoots in p4 client specification can be used as a
> workaround on p4 side, git-p4 should not resolve symlinks
> in client paths.
> chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve
> relative paths, but as a side effect it resolves symlinks
> too. Now it checks if the dir is relative before resolving.
>
> Signed-off-by: Miklós Fazekas <mfazekas@szemafor.com>
> ---
> git-p4.py | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 2da5649..5d74649 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -64,7 +64,10 @@ def chdir(dir):
> # not using the shell, we have to set it ourselves. This path could
> # be relative, so go there first, then figure out where we ended up.
> os.chdir(dir)
> - os.environ['PWD'] = os.getcwd()
> + if os.path.isabs(dir):
> + os.environ['PWD'] = dir
> + else:
> + os.environ['PWD'] = os.getcwd()
>
> def die(msg):
> if verbose:
Thanks, this is indeed a bug and I have reproduced it with a test
case. Your patch works, but I think it would be better to
separate the callers of chdir(): those that know they are
cd-ing to a path from a p4 client, and everybody else. The former
should not use os.getcwd(), as you show.
I'll whip something up soon, unless you beat me to it.
-- Pete
^ permalink raw reply
* [RFC] Should "log --cc" imply "log --cc -p"?
From: Junio C Hamano @ 2013-02-03 23:10 UTC (permalink / raw)
To: git
I think a natural way to ask reviewing the recent merges while
showing tricky ones would be to say:
$ git log --first-parent --cc master..pu
But this does not to show what I expect to see, which is an output
of:
$ git log --first-parent --cc -p master..pu
This is only a minor irritation, but I think it might make sense to
make it notice the --cc in the former and turn -p on automatically.
The same for
$ git log --cc next~3..next
which may make sense to turn into "git log -p --cc next~3..next".
When deciding if the above makes sense, there are a few things to
know to be true as prerequisites for the discussion:
* Neither of these
$ git log --first-parent -p master..pu
$ git log -p master..pu
shows any patches, and it is not a bug. No patches are shown for
merges unless -m is given, and when -m is given, we give pairwise
2-way diffs for the number of parents.
* We recently tweaked this:
$ git log --first-parent -m -p master..pu
to omit diffs with second and later parents, as that is what the
user wishes with --first-parent.
* The "--cc" option, when comparing two trees (i.e. showing a
non-merge commit), is designed to show a normal patch. In other
words, you can view "--cc" as a modifier when you request a patch
output format with "-p". For "git show", "--cc -p" is turned on
by default, and giving "-m" explicity (i.e. "git show -m") you
can turn it off and have it do "-m -p" instead.
^ permalink raw reply
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: John Keeping @ 2013-02-03 23:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <7v8v7585sr.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 01:07:48PM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > A quick search turned up the original thread where this feature was
> > added to Clang [1]. It seems that it does find genuine bugs where
> > people try to log values by doing:
> >
> > log("failed to handle error: " + errno);
>
> To be perfectly honest, anybody who writes such a code should be
> sent back to school before trying to touch out code ever again ;-).
Yeah, I can't see that getting through review here :-).
> It is not even valid Python, Perl nor Java, I would think.
It is valid Java, although I can't think of any other languages that let
you do that.
> > Are you happy to change COLONS to a const char[] instead of a #define?
>
> Happy? Not really.
>
> It could be a good change for entirely different reason. We will
> save space if we ever need to use it in multiple places. But the
> entire "COLONS + offset" thing was a hack we did, knowing that it
> will break when we end up showing a muiti-way diff for more than 32
> blobs.
>
> If we were to be touching that area of code, I'd rather see a change
> to make it more robust against such a corner case. If it results in
> squelching misguided clang warnings against programmers who should
> not be writing in C, that is a nice side effect, but I loathe to see
> any change whose primary purpose is to squelch pointless warnings.
This seems like a sensible change.
I generally like to get rid of the pointless warnings so that the useful
ones can't hide in the noise. Perhaps "CFLAGS += -Wno-string-plus-int"
would be better for this particular warning, but when there's only one
bit of code that triggers it, tweaking that seemed simpler.
> combine-diff.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index bb1cc96..7f6187f 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -982,14 +982,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
> free(sline);
> }
>
> -#define COLONS "::::::::::::::::::::::::::::::::"
> -
> static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev)
> {
> struct diff_options *opt = &rev->diffopt;
> - int i, offset;
> - const char *prefix;
> - int line_termination, inter_name_termination;
> + int line_termination, inter_name_termination, i;
>
> line_termination = opt->line_termination;
> inter_name_termination = '\t';
> @@ -1000,17 +996,14 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
> show_log(rev);
>
> if (opt->output_format & DIFF_FORMAT_RAW) {
> - offset = strlen(COLONS) - num_parent;
> - if (offset < 0)
> - offset = 0;
> - prefix = COLONS + offset;
> + /* As many colons as there are parents */
> + for (i = 0; i < num_parent; i++)
> + putchar(':');
>
> /* Show the modes */
> - for (i = 0; i < num_parent; i++) {
> - printf("%s%06o", prefix, p->parent[i].mode);
> - prefix = " ";
> - }
> - printf("%s%06o", prefix, p->mode);
> + for (i = 0; i < num_parent; i++)
> + printf("%06o ", p->parent[i].mode);
> + printf("%06o", p->mode);
>
> /* Show sha1's */
> for (i = 0; i < num_parent; i++)
^ permalink raw reply
* Re: Feature request: Allow extracting revisions into directories
From: Konstantin Khomoutov @ 2013-02-03 23:26 UTC (permalink / raw)
To: Robert Clausecker; +Cc: git
In-Reply-To: <1359901085.24730.11.camel@t520>
On Sun, Feb 03, 2013 at 03:18:05PM +0100, Robert Clausecker wrote:
> git currently has the archive command that allows to save an arbitrary
> revision into a tar or zip file. Sometimes it is useful to not save this
> revision into an archive but to directly put all files into an arbitrary
> directory. Currently this seems to be not possible to archive directly;
> the only way I found to do it is to run git archive and then directly
> unpack the archive into a directory.
>
> git --git-dir REPO archive REVISION | tar x
>
> It would be nice to have a command or simply a switch to git archive
> that allows the user to put the files of REVISION into a directory
> instead of making an archive.
You could use plumbing commands combined with a throwaway custom index
file and a separate work tree which will receive the tree at REVISION:
export GIT_WORK_TREE=/path/to/dest/directory
export GIT_DIR=/path/to/repo/.git
export GIT_INDEX_FILE="$GIT_WORK_TREE/.index"
git read-tree REVISION
git checkout-index -a
rm -f "$GIT_INDEX_FILE"
^ permalink raw reply
* Re: Moving (renaming) submodules, recipe/script
From: W. Trevor King @ 2013-02-03 23:38 UTC (permalink / raw)
To: TJ; +Cc: Git, Jens Lehmann, Jonathan Nieder, Peter Collingbourne
In-Reply-To: <20130107003603.GA25698@odin.tremily.us>
[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]
On Sun, Feb 03, 2013 at 10:36:17PM +0000, TJ wrote:
> I've recently had need to re-arrange more than ten submodules within
> a project and discovered there is apparently no easy way to do it.
I ran into a similar problem last month, and wrote a similar script
[1] ;). There are a few other related threads you might be interested
in:
On Sun, Jan 06, 2013 at 07:36:03PM -0500, W. Trevor King wrote:
> Today I had to move my first submodule, and I discovered that Git's
> support for this is pretty limited. There have been a few patch
> series attempting to address this [1,2], but none of them seems to
> have pushed through into master (although I can't put my finger on a
> reason for why). There are also some SO postings discussing this
> [3,4]. It would be nice if `git mv` worked out of the box on
> …
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/88720
> [2]: http://thread.gmane.org/gmane.comp.version-control.git/143250
> [4]: http://stackoverflow.com/questions/4323558/moving-submodules-with-git
> [3]: http://stackoverflow.com/questions/4604486/how-do-i-move-an-existing-git-submodule-within-a-git-repository
The long-term solution is probably Jens' branch:
On Mon, Jan 07, 2013 at 07:59:53AM +0100, Jens Lehmann wrote:
> I´m currently working on teaching mv to move submodules and intend
> to send those patches to the list after finishing submodule deinit.
> Please see
> https://github.com/jlehmann/git-submod-enhancements/commits/mv-submodules
> for the current state of this series.
Cheers,
Trevor
[1]: http://thread.gmane.org/gmane.comp.version-control.git/212861
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: Junio C Hamano @ 2013-02-04 0:24 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <20130203231549.GV1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
>> If we were to be touching that area of code, I'd rather see a change
>> to make it more robust against such a corner case. If it results in
>> squelching misguided clang warnings against programmers who should
>> not be writing in C, that is a nice side effect, but I loathe to see
>> any change whose primary purpose is to squelch pointless warnings.
>
> This seems like a sensible change.
>
> I generally like to get rid of the pointless warnings so that the useful
> ones can't hide in the noise. Perhaps "CFLAGS += -Wno-string-plus-int"
> would be better for this particular warning, but when there's only one
> bit of code that triggers it, tweaking that seemed simpler.
Thanks for a sanity check. Ideally it should also have test cases
to show "git diff --cc --raw blob1 blob2...blob$n" for n=4 and n=40
(or any two values clearly below and above the old hardcoded limit)
behave sensibly, exposing the old breakage, which I'll leave as a
LHF (low-hanging-fruit). Hint, hint...
-- >8 --
Subject: [PATCH] combine-diff: lift 32-way limit of combined diff
The "raw" format of combine-diff output is supposed to have as many
colons as there are parents at the beginning, then blob modes for
these parents, and then object names for these parents.
We weren't however prepared to handle a more than 32-way merge and
did not show the correct number of colons in such a case.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
combine-diff.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index bb1cc96..7f6187f 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -982,14 +982,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
free(sline);
}
-#define COLONS "::::::::::::::::::::::::::::::::"
-
static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev)
{
struct diff_options *opt = &rev->diffopt;
- int i, offset;
- const char *prefix;
- int line_termination, inter_name_termination;
+ int line_termination, inter_name_termination, i;
line_termination = opt->line_termination;
inter_name_termination = '\t';
@@ -1000,17 +996,14 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
show_log(rev);
if (opt->output_format & DIFF_FORMAT_RAW) {
- offset = strlen(COLONS) - num_parent;
- if (offset < 0)
- offset = 0;
- prefix = COLONS + offset;
+ /* As many colons as there are parents */
+ for (i = 0; i < num_parent; i++)
+ putchar(':');
/* Show the modes */
- for (i = 0; i < num_parent; i++) {
- printf("%s%06o", prefix, p->parent[i].mode);
- prefix = " ";
- }
- printf("%s%06o", prefix, p->mode);
+ for (i = 0; i < num_parent; i++)
+ printf("%06o ", p->parent[i].mode);
+ printf("%06o", p->mode);
/* Show sha1's */
for (i = 0; i < num_parent; i++)
--
1.8.1.2.628.geb8a6d5
^ permalink raw reply related
* Re: Feature request: Allow extracting revisions into directories
From: Sitaram Chamarty @ 2013-02-04 0:42 UTC (permalink / raw)
To: Robert Clausecker; +Cc: git
In-Reply-To: <1359915086.24730.19.camel@t520>
On 02/03/2013 11:41 PM, Robert Clausecker wrote:
>
> Am Sonntag, den 03.02.2013, 21:55 +0530 schrieb Sitaram Chamarty:
>> Could you help me understand why piping it to tar (actually 'tar -C
>> /dest/dir -x') is not sufficient to achieve what you want?
>
> Piping the output of git archive into tar is of course a possible
> solution; I just don't like the fact that you need to pipe the output
> into a separate program to do something that should be possible with a
> simple switch and not an extra command. It feels unintuitive and like a
> workaround to make an archive just to unpack it on-the-fly. Also, adding
> such a command (or at least documenting the way to do this using a pipe
> to tar somewhere in the man pages) is a small and simple change that
> improves usability.
I realise it appears to be the fashion these days to get away from the
Unix philosophy of having different tools do different things and
combining them as needed.
Ignoring the option-heavy GNU, and looking at the more traditional BSD
tar manpage [1], I notice the following flags that could still be
potentially needed by someone running 'git archive': '-t' (instead of
'-x'), '-C dir', '--exclude/include', '-k', '-m', '--numeric-owner', -o,
-P, -p, -q, -s, -T, -U, -v, -w, and -X.
And I'm ignoring the esoteric ones like "--chroot" and "-S" (sparse mode).
How many of these options would you like included in git? And if you
say "I don't need any of those; I just need '-x'", that's not relevant.
Someone else may need any or all of those flags, and if you accept "-x"
you have to accept some of the others too.
Also, I often want to deploy to a different host, and I might do that
like so:
git archive ... | ssh host tar -C /deploy/dir -x
Why not put that ssh functionality into git also?
What about computing a checksum and putting out a "sha1sums.txt" file?
People do that also. How about a flag for that?
Where does this end?
[1]: http://www.unix.com/man-page/FreeBSD/1/tar/
^ permalink raw reply
* Re: Re: Re: Re: Re: Segmentation fault with latest git (070c57df)
From: Jongman Heo @ 2013-02-04 2:18 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, Junio C Hamano, Thomas Rast, git, Antoine Pelisse
Jonathan Nieder wrote:
> Jongman Heo wrote:
>
>>> But it doesn't stimulate any prerequisites in make, which is weird.
>>> What's in builtin/.depend/fetch.o.d?
>[...]
>> please see below~.
>>
>> $ cat builtin/.depend/fetch.o.d
>> fetch.o: builtin/fetch.c cache.h git-compat-util.h compat/bswap.h \
>
>That's the problem. See the following thread:
>
> http://thread.gmane.org/gmane.comp.version-control.git/185625/focus=185680
>
>Currently when COMPUTE_HEADER_DEPENDENCIES=auto git tests for
>dependency generation support by checking the output and exit status
>from the following command:
>
>$(CC) $(ALL_CFLAGS) -c -MF /dev/null -MMD -MP \
>-x c /dev/null -o /dev/null 2>&1
>
>Perhaps this can be improved? Even something as simple as a ccache
>version test could presumably help a lot.
>
>Hope that helps,
>Jonathan
Hi,
Unfortunately, the patch didn't help to me.
Anyway, ccache is the culprit (I'm using ccache 2.4 version).
If I disable ccache using CCACHE_DISABLE=1, then the issue doesn't happen. Thanks.
Best regards,
Jongman Heo.
^ permalink raw reply
* Re: [PATCH] send-email: ignore files ending with ~
From: Alexandre Courbot @ 2013-02-04 2:43 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Junio C Hamano, git
In-Reply-To: <CALWbr2wCEHkVU86Dkzq1+mRx605h4LBo-5e8ySAwv=T0pMw62A@mail.gmail.com>
On Mon, Feb 4, 2013 at 2:16 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
> On Sun, Feb 3, 2013 at 3:55 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> It certainly happened to a lot of people already: you carefully prepare
>> your set of patches, export them using format-patch --cover-letter,
>> write your cover letter, and send the set like this:
>>
>> $ git send-email --to=somerenowneddeveloper --to=myfutureemployer
>> --cc=thismailinglistiwanttoimpress 00*
>
> Why don't you use 00*.patch ? That seems dubious to me to ignore files
> specified on the command line.
For the same reason I do not list all the patches individually on the
command line: laziness.
The goal of this patch is in no way to argue that using send-email
like this is a recommandable way. It just adds a safeguard on a
use-case that is probably not so uncommon.
Alex.
^ permalink raw reply
* RE: [PATCH] Handle path completion and colon for tcsh script
From: Marc Khouzam @ 2013-02-04 2:50 UTC (permalink / raw)
To: Junio C Hamano, Manlio Perillo; +Cc: git@vger.kernel.org
In-Reply-To: <7vhalt86wj.fsf@alter.siamese.dyndns.org>
> > The problem is, if the directory name *already* has a slash, Bash adds
> > another slash!
>
> So bash users do see the trailing slash because bash adds one to
> what we compute and return, which we do strip the trailing slash
> exactly because we know bash will add one.
The problem is slightly more obscure than that, and I wonder if it
should be documented somewhere in the bash script?
Manlio explained in a previous
exchange with me, that bash will properly deal with an existing
trailing slash when doing the completion on the command-line, but
will screw it up by adding a second slash when dealing with multiple
possible completions and printing those for the user to choose from.
For example:
$ git status
# On branch tcsh_next
# Untracked files:
# (use "git add <file>..." to include in what will be committed)
#
# fish/
# fishing/
nothing added to commit but untracked files present (use "git add" to track)
$ git add fish<tab>
fish// fishing// <-------- notice the double slashes
$ git add fishi<tab>
# will complete the command line properly to the below, with a single slash.
$ git add fishing/
> Because tcsh completion
> uses what we compute directly, without bash massaging our output to
> add the trailing slash, it needs some magic.
Yes, that is right.
> OK, that makes sense. It was this part from the originally proposed
> log message:
>
> >> ... Such completions do not add the '/' at the end of directories
> >> for recent versions of bash. However, the '/' is needed by tcsh,
> >> ...
>
> with a large gap between the two sentences that fooled me, and the
> explanation in your message helped to fill the gap to understand the
> situation better.
Sorry about the lack of details.
I'm see more and more the importance of these commit messages :)
> Perhaps
>
> ... for recent versions of bash, which will then add the
> trailing slash for paths that are directory to the result of
> our completion. The completion for tcsh however uses the
> result of our completion directly, so it either needs to add
> the necessary slash itself, or needs to ask us to keep the
> trailiing slash. This patch does the latter.
>
> or something?
How about the following, which gives a little more detail about
the solution for tcsh? I think it is worth putting the below extra
details because I feel a mistake could easily be made about this
trailing slash issue, which I had gotten wrong for my own version
of the script for a couple of weeks, before figuring out the mistake.
Handle path completion and colon for tcsh script
Recent enhancements to git-completion.bash provide
intelligent path completion for git commands. Such
completions do not provide the '/' at the end of directories
for recent versions of bash; instead, bash itself will add the
trailing slash to directories to the result provided by
git-completion.bash. However, the completion for tcsh uses
the result of the bash completion script directly, so it either
needs to add the necessary slash itself, or needs to ask the
bash script to keep the trailing slash. Adding the slash itself
is difficult because we cannot easily tell if an entry of the
output of the bash script is a directory or something else.
For example, assuming there is a directory named 'commit'
in the current directory, then, when completing
git add commit<tab>
we would need to add a slash, but for
git help commit<tab>
we should not.
Figuring out such differences would require adding much
intelligence to the tcsh completion script. Instead, it is
simpler to ask the bash script to keep the trailing slash.
This patch does this.
Also, tcsh does not handle the colon as a completion
separator so we remove it from the list of separators.
Signed-off-by: Marc Khouzam <marc.khouzam@ericsson.com>
Thanks
Marc
^ permalink raw reply
* Re: [PATCH] Handle path completion and colon for tcsh script
From: Junio C Hamano @ 2013-02-04 3:02 UTC (permalink / raw)
To: Marc Khouzam; +Cc: Manlio Perillo, git@vger.kernel.org
In-Reply-To: <E59706EF8DB1D147B15BECA3322E4BDC09B5A4@eusaamb103.ericsson.se>
Thanks for a detailed explanation. The two examples illustrating
different interpretation of the same word were really good.
Will replace and requeue.
^ permalink raw reply
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Greg KH @ 2013-02-04 0:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rene Scharfe, git, Konstantin Ryabitsev
In-Reply-To: <7vr4l1gqv8.fsf@alter.siamese.dyndns.org>
On Thu, Jan 31, 2013 at 10:16:27AM -0800, Junio C Hamano wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
>
> > On Thu, Jan 31, 2013 at 09:32:12AM -0800, Junio C Hamano wrote:
> >
> >> How about fixing kup to teach the "let's cheat and let the other end
> >> run 'git archive', if the resulting archive and GPG signature
> >> locally created does match, we do not have to transfer the tarball
> >> itself" trick a fall-back mode that says "but if the signature does
> >> not match, then transfer the bulk used to create the signature to
> >> the remote anyway". This fallback can and should of course be
> >> useful for the compressed patch transfer.
> >
> > Ugh, uploading a 431Mb file, over a flaky wireless connection (I end up
> > doing lots of kernel releases while traveling), would be a horrible
> > change. I'd rather just keep using the same older version of git that
> > kernel.org is running instead.
>
> Then how about fixing kup to try both versions of Git? There will
> be people who run different versions of Git anyway, and kup should
> not be preventing Git from helping people on other platforms, or
> improving its output in general.
I think the combinations of different versions of git that would have to
be installed on kernel.org to handle stuff like this as things change
over time, wouldn't be worth it.
The number of people this affects right now is only one (me), given that
the offending file is not in Linus's tree right now, so he doesn't have
issues with uploading new releases.
So I'll just work to ensure I have the same version of git in place if I
ever run into this problem again. Or just break down and do
full-compressed tarballs instead, if I'm in a place where I have a good
network connection.
thanks,
greg k-h
^ 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