* 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
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Greg KH @ 2013-02-04 0:48 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Junio C Hamano, Rene Scharfe, git
In-Reply-To: <510AAF4F.6060201@kernel.org>
On Thu, Jan 31, 2013 at 12:52:15PM -0500, Konstantin Ryabitsev wrote:
> On 31/01/13 12:41 PM, Greg KH wrote:
> > 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.
>
> Well, we do accept compressed archives, so you would be uploading about
> 80MB instead of 431MB, but that would still be a problem for anyone
> releasing large tarballs over unreliable connections. I know you
> routinely do 2-3 releases at once, so that would still mean uploading
> 120-180MB.
That would mean I can't do kernel releases while on ferry rides, which
is probably a good thing in the end :)
> I don't have immediate statistics on how many people release using "kup
> --tar", but I know that at least you and Linus rely on that exclusively.
What causes you to upgrade the version of git on the server? Are you
relying on packages for a distro, or is this "hand installed" by
yourself? As long as I stay in lock-step with your updates, all should
be fine.
Oh, maybe we can report back to the user, the version of git that is
being used on the server, if the checksums don't match, so that I know
to at least see if my version is different from yours?
thanks,
greg k-h
^ 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:42 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git, Konstantin Ryabitsev
In-Reply-To: <510AB910.5050504@lsrfire.ath.cx>
On Thu, Jan 31, 2013 at 07:33:52PM +0100, René Scharfe wrote:
> Am 31.01.2013 18:28, schrieb Greg KH:
> > I tracked this down to commit 22f0dcd9634a818a0c83f23ea1a48f2d620c0546
> > (archive-tar: split long paths more carefully). The diff of a hex dump
> > of the tar archives shows the following difference:
> >
> > --- old_git_archive 2013-01-31 17:31:24.466343388 +0100
> > +++ new_git_archive 2013-01-31 17:32:21.509674417 +0100
> > @@ -19239998,8 +19239998,8 @@
> > 125943d0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125943e0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125943f0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > -12594400:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > -12594410:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > +12594400:7765 7374 6272 6964 6765 2d6f 6d61 7033 westbridge-omap3
> > +12594410:2d70 6e61 6e64 2d68 616c 2f00 0000 0000 -pnand-hal/.....
> > 12594420:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 12594430:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 12594440:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > @@ -19240025,8 +19240025,8 @@
> > 12594580:2f61 7374 6f72 6961 2f61 7263 682f 6172 /astoria/arch/ar
> > 12594590:6d2f 706c 6174 2d6f 6d61 702f 696e 636c m/plat-omap/incl
> > 125945a0:7564 652f 6d61 6368 2f77 6573 7462 7269 ude/mach/westbri
> > -125945b0:6467 652f 7765 7374 6272 6964 6765 2d6f dge/westbridge-o
> > -125945c0:6d61 7033 2d70 6e61 6e64 2d68 616c 0000 map3-pnand-hal..
> > +125945b0:6467 6500 0000 0000 0000 0000 0000 0000 dge.............
> > +125945c0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945d0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945e0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945f0:0000 0000 0000 0000 0000 0000 0000 0000 ................
>
> This is the only directory in the repository whose path is long enough to
> make a difference with the patch, 105 characters in total:
>
> drivers/staging/westbridge/astoria/arch/arm/plat-omap/include/mach/westbridge/westbridge-omap3-pnand-hal/
>
> Five characters less and you wouldn't notice a thing. It contains
> "westbridge" thrice, so I think it's cheating just to reach that
> length, though. ;-)
Yeah, that file path was bad, which is just one reason why it was
deleted from the kernel in newer versions :)
> > Interestingly, the output of uncompressing the tar archives is
> > identical, so the data is correct, but the binary isn't.
>
> The path is split differently between two header fields, that's all.
Ok, thanks for the explaination, I didn't realize that.
> > Now keeping binary compatibility of tar archive files isn't really a big
> > deal, but, the commit to git that causes this seems a bit odd, is it
> > really needed? Or can we just fix the version of tar with NetBSD
> > instead? :)
>
> Apart from Junio's suggestion, I can't think of a practical solution.
>
> You could downgrade your git to a version before the fix. A downside is
> that you won't be able to extract the archive on NetBSD without getting
> an error message (but the contents would be intact, except perhaps for
> permission bits of the directory above).
>
> You could upgrade the kernel.org version of git, but that might cause the
> same problem for other maintainers with long directory paths who in their
> repositories who still use git without the fix.
>
> You could make the path shorter. Won't help at all with the release you
> just did, of course.
What I ended up doing was just to revert your patch, generating a tar
archive that matches what the version on kernel.org.
And originally I now recall that this was something we were worried
about, but we put off dealing with it until it caused problems :)
> I don't know if other tar implementations freak out when they see an
> empty name field. NetBSD's tar might seem a bit too strict here, but
> overall I think it's right in complaining.
Ok, thanks, I now agree.
> What makes the commit odd, by the way?
Sorry, I was originally thinking that you were working around a bug in
the NetBSD version of tar, not making it "more correct", which is
arguably the right thing to do here.
So I'll work with Konstantine to ensure we both are using the same
version of git in the future, it's our kernel.org infrastructure issue
here, not a git one, sorry for the noise.
greg k-h
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jonathan Nieder @ 2013-02-04 3:40 UTC (permalink / raw)
To: Jongman Heo; +Cc: Jeff King, Junio C Hamano, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <19086873.858651359944284535.JavaMail.weblogic@epml01>
Jongman Heo wrote:
> Unfortunately, the patch didn't help to me.
Thanks for testing. Did you apply the patch to the older version of
git that generates builtin/.depend/fetch.o.d or the newer version that
consumes it?
Curious,
Jonathan
^ permalink raw reply
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Junio C Hamano @ 2013-02-04 5:05 UTC (permalink / raw)
To: Greg KH; +Cc: Rene Scharfe, git, Konstantin Ryabitsev
In-Reply-To: <20130204004512.GB6243@kroah.com>
Greg KH <gregkh@linuxfoundation.org> writes:
>> 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.
You make it sound as if you have to pick the right one among 47
different versions, but I think over the lifetime of Git there was
only one time that output from "diff" would have affected the kup's
trick to avoid the transmission of patch text, and another that
output from "tar-tree" (aka "archive --format=tar") would have
afffected the transmission of tarballs. I do think it is feasible
if "kup" wanted to.
> 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.
As a tree grows larger over time, it may be just a matter of time
for somebody else to be hit by another deep path, though.
^ permalink raw reply
* Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: Junio C Hamano @ 2013-02-04 6:41 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Lars Hjemli, Jonathan Nieder, git, Heiko Voigt
In-Reply-To: <5106ECB6.9010801@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Am 28.01.2013 21:34, schrieb Junio C Hamano:
> ...
>> I was imagining that "foreach --untracked" could go something like this:
>>
>> * If you are inside an existing git repository, read its index to
>> learn the gitlinks in the directory and its subdirectories.
>>
>> * Start from the current directory and recursively apply the
>> procedure in this step:
>>
>> * Scan the directory and iterate over the ones that has ".git" in
>> it:
>>
>> * If it is a gitlinked one, show it, but do not descend into it
>> unless --recursive is given (e.g. you start from /home/jens,
>> find /home/jens/proj/ directory that has /home/jens/proj/.git
>> in it. /home/jens/.git/index knows that it is a submodule of
>> the top-level superproject. "proj" is handled, and it is up
>> to the --recursive option if its submodules are handled).
>>
>> * If it is _not_ a gitlinked one, show it and descend into it
>> (e.g. /home/jens/ is not a repository or /home/jens/proj is
>> not a tracked submodule) to apply this procedure recursively.
>>
>> Of course, without --untracked, we have no need to iterate over the
>> readdir() return values; instead we just scan the index of the
>> top-level superproject.
>
> Thanks for explaining, that makes tons of sense.
There is a small thinko above, though, and I'd like to correct it
before anybody takes the above too seriously as _the_ outline of the
design and implements it to the letter.
The --recursive option should govern both a tracked submodule and an
untracked one. When asking to list both existing submodules and
directories that could become submodules, you should be able to say
$ git submodule foreach --untracked
to list the direct submodules and the directories with .git in them
that are not yet submodules of the top-level superproject, but the
latter is limited to those with no parent directories with .git in
them (other than the top-level of the working tree of the
superproject). With
$ git submodule foreach --untracked --recursive
you would see submodules and their submodules recursively, and also
directories with .git in them (i.e. candidates to become direct
submodules of the superproject) and the directories with .git in
them inside such submodule candidates (i.e. candidates to become
direct submodules of the directories that could become direct
submodules of the superproject) recursively.
If we set things up this way:
mkdir -p a/b c/d &&
for d in . a a/b c c/d
do
git init $d &&
( cd $d && git commit --allow-empty -m initial )
done &&
git add a &&
( cd a && git add b )
The expected results for various combinations are:
* "git submodule foreach" would visit 'a' and nothing else;
* "git submodule foreach --recursive" would visit 'a' and 'a/b';
* "git submodule foreach --untracked" would visit 'a' and 'c'; and
* "git submodule foreach --untracked --recursive" would visit all four.
^ permalink raw reply
* Re: Re: Segmentation fault with latest git (070c57df)
From: Jongman Heo @ 2013-02-04 6:58 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, Junio C Hamano, Thomas Rast, git, Antoine Pelisse
Jonathan Nieder wrote:
> Jongman Heo wrote:
>
>> Unfortunately, the patch didn't help to me.
>
>Thanks for testing. Did you apply the patch to the older version of
>git that generates builtin/.depend/fetch.o.d or the newer version that
>consumes it?
>
>Curious,
>Jonathan
Hi, Jonathan,
I applied the patch to newer version.
This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~.
Thanks~!
Best regards,
Jongman Heo
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Junio C Hamano @ 2013-02-04 7:13 UTC (permalink / raw)
To: jongman.heo; +Cc: Jonathan Nieder, Jeff King, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <12070540.431901359961105650.JavaMail.weblogic@epml10>
Jongman Heo <jongman.heo@samsung.com> writes:
> Jonathan Nieder wrote:
>> Jongman Heo wrote:
>>
>>> Unfortunately, the patch didn't help to me.
>>
>>Thanks for testing. Did you apply the patch to the older version of
>>git that generates builtin/.depend/fetch.o.d or the newer version that
>>consumes it?
>>
>>Curious,
>>Jonathan
>
> Hi, Jonathan,
>
> I applied the patch to newer version.
>
> This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~.
> Thanks~!
Yeah, that result is understandable, as .depend/*.o.d files will not
be rebuilt when the rules to build them changes in the Makefile.
Applying the patch to the Makefile in the pristine old tree, run the
build (which will generate .depend/*.o.d files with the corrected
rules), then checking out the new tree and running the build again
without "make clean", with or with the patch applied, would validate
that the patch fixes the issue for old ccache.
Thanks Jonathan for diagnosing, fixing, and thanks Jongman for
testing.
^ permalink raw reply
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Duy Nguyen @ 2013-02-04 7:14 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jonathan Niedier
In-Reply-To: <vpqpq0hnlb1.fsf@grenoble-inp.fr>
On Mon, Feb 4, 2013 at 4:23 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> 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.
If we want to make it clear, I think the standard "* (no branch)" should become
* HEAD (detached)
or non-detached case:
* HEAD -> foo
Then we could present rebase/bisect information as
* HEAD (detached, bisecting)
* HEAD (detached, rebasing)
* foo (rebasing)
I don't want to make this line too long because it would break (well,
waste space in) column layout. So if we do this, no branch name added
for rebase/bisect.
--
Duy
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Junio C Hamano @ 2013-02-04 7:17 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, git
In-Reply-To: <20130201185827.GA22919@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I was specifically thinking of this (on top of your patch):
>
> diff --git a/remote-curl.c b/remote-curl.c
> index e6f3b63..63680a8 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char *service)
> last->buf_alloc = strbuf_detach(&buffer, &last->len);
> last->buf = last->buf_alloc;
>
> - if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (maybe_smart && !strbuf_cmp(&exp, &type)) {
> /*
> * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - strbuf_addf(&exp, "application/x-%s-advertisement", service);
> - if (strbuf_cmp(&exp, &type))
> - die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
>
> To just follow the dumb path if we don't get the content-type we expect.
> We may want to keep the '#' format check in addition (packet_get_line
> will check it and die, anyway, but we may want to drop back to
> considering it dumb, just to protect against a badly configured dumb
> server which uses our mime type, but I do not think it likely).
Yeah, but it doesn't cost anything to check, so let's do so.
Does this look good to both of you (relative to Shawn's patch)?
remote-curl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
last->buf_alloc = strbuf_detach(&buffer, &last->len);
last->buf = last->buf_alloc;
- if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+ strbuf_addf(&exp, "application/x-%s-advertisement", service);
+ if (maybe_smart &&
+ (5 <= last->len && last->buf[4] == '#') &&
+ !strbuf_cmp(&exp, &type)) {
/*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
- strbuf_addf(&exp, "application/x-%s-advertisement", service);
- if (strbuf_cmp(&exp, &type))
- die("invalid content-type %s", type.buf);
if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
die("%s has invalid packet header", refs_url);
if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
^ permalink raw reply related
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Duy Nguyen @ 2013-02-04 7:17 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jonathan Niedier
In-Reply-To: <CACsJy8CGqiahw3y42KRt61gChtfOucFHqZqn_uvLrj7j7KrQbg@mail.gmail.com>
On Mon, Feb 4, 2013 at 2:14 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> * foo (rebasing)
Well, this one does not make sense (or causes more confusion).
--
Duy
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-04 8:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vehgw5z7n.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 11:13:00PM -0800, Junio C Hamano wrote:
> Yeah, that result is understandable, as .depend/*.o.d files will not
> be rebuilt when the rules to build them changes in the Makefile.
> Applying the patch to the Makefile in the pristine old tree, run the
> build (which will generate .depend/*.o.d files with the corrected
> rules), then checking out the new tree and running the build again
> without "make clean", with or with the patch applied, would validate
> that the patch fixes the issue for old ccache.
>
> Thanks Jonathan for diagnosing, fixing, and thanks Jongman for
> testing.
Do we want to do anything with the other dependency hole I found here:
http://article.gmane.org/gmane.comp.version-control.git/215211
It's definitely a potential problem, but I don't think we have any
reports of it happening in practice, so it might not be worth worrying
about. Doing a clean version of the fix here:
http://article.gmane.org/gmane.comp.version-control.git/215212
would probably involve reorganizing our .depend directory structure,
unless somebody can cook up some clever use of make's patsubst.
-Peff
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Jeff King @ 2013-02-04 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, git
In-Reply-To: <7va9rk5z02.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:
> Does this look good to both of you (relative to Shawn's patch)?
>
> remote-curl.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index e6f3b63..933c69a 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
> last->buf_alloc = strbuf_detach(&buffer, &last->len);
> last->buf = last->buf_alloc;
>
> - if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (maybe_smart &&
> + (5 <= last->len && last->buf[4] == '#') &&
> + !strbuf_cmp(&exp, &type)) {
> /*
> * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - strbuf_addf(&exp, "application/x-%s-advertisement", service);
> - if (strbuf_cmp(&exp, &type))
> - die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
Yeah, I think that's fine. Thanks.
-Peff
^ permalink raw reply
* Re: Getting started contributing.
From: adamfraser @ 2013-02-04 8:42 UTC (permalink / raw)
To: git
In-Reply-To: <7v1ucx9o4m.fsf@alter.siamese.dyndns.org>
Thanks for all of the replies. I'll have a look into the suggestions and try
to find somewhere I can help out. :)
--
View this message in context: http://git.661346.n2.nabble.com/Getting-started-contributing-tp7576834p7576901.html
Sent from the git mailing list archive at Nabble.com.
^ 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