* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Jakub Narebski @ 2009-10-24 9:56 UTC (permalink / raw)
To: Andreas Schwab
Cc: Bernhard Reutner-Fischer, Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m28wf1unop.fsf@whitebox.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>> On Fri, Oct 23, 2009 at 02:26:53PM -0700, Junio C Hamano wrote:
>>>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>>> On Fri, Oct 23, 2009 at 10:25:24PM +0200, Bernhard Reutner-Fischer wrote:
>>>>> On Fri, Oct 23, 2009 at 10:15:43PM +0200, Bernhard Reutner-Fischer wrote:
>>>>>>
>>>>>> GNU tar-1.22 handles 'o' as no-same-owner only on extract,
>>>>>> on create, 'o' would be --old-archive.
>>>>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
>>>>> (cd blt && $(TAR) cf - .) | \
>>>>> - (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xfo -)
>>>>> + (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) x --no-numeric-owner -f -)
>>>>
>>>> argh, sorry! --no-same-owner of course.
>>>
>>> Either way, your change would break non-GNU tar implementations that are
>>> properly POSIX.1, isn't it?
>>
>> I suppose xf - -o would work?
>
> Isn't that the same as 'xfo -'?
>
> (tar isn't specified by POSIX, btw.)
I don't quite understand why 'o' has to be spelled using long name
--no-same-owner, instead of just correcting the ordering of "old style"
short options to have 'f' last, i.e.
$(TAR) xof -
and not (current)
$(TAR) xfo -
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Bernhard Reutner-Fischer @ 2009-10-24 10:05 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m28wf1unop.fsf@whitebox.home>
On Sat, Oct 24, 2009 at 11:49:10AM +0200, Andreas Schwab wrote:
>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>
>> On Fri, Oct 23, 2009 at 02:26:53PM -0700, Junio C Hamano wrote:
>>>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>>
>>>> On Fri, Oct 23, 2009 at 10:25:24PM +0200, Bernhard Reutner-Fischer wrote:
>>>>>On Fri, Oct 23, 2009 at 10:15:43PM +0200, Bernhard Reutner-Fischer wrote:
>>>>>>GNU tar-1.22 handles 'o' as no-same-owner only on extract,
>>>>>>on create, 'o' would be --old-archive.
>>>>>
>>>>>FYI this was prompted by:
>>>>>
>>>>>Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
>>>>>
>>>>>diff -rdup git-1.6.5.oorig/templates/Makefile git-1.6.5/templates/Makefile
>>>>>--- git-1.6.5.oorig/templates/Makefile 2009-10-11 03:42:04.000000000 +0200
>>>>>+++ git-1.6.5/templates/Makefile 2009-10-23 21:43:06.000000000 +0200
>>>>>@@ -50,4 +50,4 @@ clean:
>>>>> install: all
>>>>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
>>>>> (cd blt && $(TAR) cf - .) | \
>>>>>- (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xfo -)
>>>>>+ (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) x --no-numeric-owner -f -)
>>>>
>>>> argh, sorry! --no-same-owner of course.
>>>
>>>Either way, your change would break non-GNU tar implementations that are
>>>properly POSIX.1, isn't it?
>>
>> I suppose xf - -o would work?
>
>Isn't that the same as 'xfo -'?
Not really (if you do not permute the arguments which we don't in
busybox, for size reasons).
f specifies the file so "fo" acts on file "o".
"xof -" would work for me as well as "xf - -o", it's just that
"xfo -" does not work.
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Bernhard Reutner-Fischer @ 2009-10-24 10:06 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Andreas Schwab, Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m38wf1dsjf.fsf@localhost.localdomain>
On Sat, Oct 24, 2009 at 02:56:33AM -0700, Jakub Narebski wrote:
>Andreas Schwab <schwab@linux-m68k.org> writes:
>> Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>> On Fri, Oct 23, 2009 at 02:26:53PM -0700, Junio C Hamano wrote:
>>>>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>>>> On Fri, Oct 23, 2009 at 10:25:24PM +0200, Bernhard Reutner-Fischer wrote:
>>>>>> On Fri, Oct 23, 2009 at 10:15:43PM +0200, Bernhard Reutner-Fischer wrote:
>>>>>>>
>>>>>>> GNU tar-1.22 handles 'o' as no-same-owner only on extract,
>>>>>>> on create, 'o' would be --old-archive.
>
>>>>>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
>>>>>> (cd blt && $(TAR) cf - .) | \
>>>>>> - (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xfo -)
>>>>>> + (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) x --no-numeric-owner -f -)
>>>>>
>>>>> argh, sorry! --no-same-owner of course.
>>>>
>>>> Either way, your change would break non-GNU tar implementations that are
>>>> properly POSIX.1, isn't it?
>>>
>>> I suppose xf - -o would work?
>>
>> Isn't that the same as 'xfo -'?
>>
>> (tar isn't specified by POSIX, btw.)
>
>I don't quite understand why 'o' has to be spelled using long name
>--no-same-owner, instead of just correcting the ordering of "old style"
It doesn't have to be, right.
>short options to have 'f' last, i.e.
>
> $(TAR) xof -
>
>and not (current)
>
> $(TAR) xfo -
any of "xf - -o" or "xof -" would work for me.
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Andreas Schwab @ 2009-10-24 10:06 UTC (permalink / raw)
To: Jakub Narebski
Cc: Bernhard Reutner-Fischer, Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m38wf1dsjf.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> I don't quite understand why 'o' has to be spelled using long name
> --no-same-owner, instead of just correcting the ordering of "old style"
> short options to have 'f' last, i.e.
>
> $(TAR) xof -
>
> and not (current)
>
> $(TAR) xfo -
Both do exactly the same, traditionally. Note the lack of '-' preceding
'xfo', which does not use getopt-style option parsing.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Andreas Schwab @ 2009-10-24 10:44 UTC (permalink / raw)
To: Bernhard Reutner-Fischer; +Cc: Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <20091024100502.GG4615@mx.loc>
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
> On Sat, Oct 24, 2009 at 11:49:10AM +0200, Andreas Schwab wrote:
>>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>
>>> I suppose xf - -o would work?
>>
>>Isn't that the same as 'xfo -'?
>
> Not really (if you do not permute the arguments which we don't in
> busybox, for size reasons).
There is no argument permutation. The traditional argument parsing of
tar does not cluster option letters with option arguments.
Of course, just using 'xof -' will work around this busybox bug.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Nanako Shiraishi @ 2009-10-24 13:06 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Michael J Gruber, Jonathan Nieder
In-Reply-To: <1256377489-16719-1-git-send-email-felipe.contreras@gmail.com>
Quoting Felipe Contreras <felipe.contreras@gmail.com> writes:
> ...
> Reworded the getting started section based on comments from Michael J Gruber,
> Jonathan Nieder and Junio C Hamano.
I'm surprised that you ignored comments from the original
author of the document you are updating.
Date: Tue, 13 Oct 2009 22:49:40 -0400
Message-ID: <20091014024940.GB9700@fieldses.org>
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Andreas Schwab @ 2009-10-24 13:06 UTC (permalink / raw)
To: Bernhard Reutner-Fischer; +Cc: Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m2ocnxt6jl.fsf@whitebox.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>
>> On Sat, Oct 24, 2009 at 11:49:10AM +0200, Andreas Schwab wrote:
>>>Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> writes:
>>>
>>>> I suppose xf - -o would work?
>>>
>>>Isn't that the same as 'xfo -'?
>>
>> Not really (if you do not permute the arguments which we don't in
>> busybox, for size reasons).
>
> There is no argument permutation. The traditional argument parsing of
> tar does not cluster option letters with option arguments.
>
> Of course, just using 'xof -' will work around this busybox bug.
Like this.
Andreas.
>From ebadb41b346c305b94f27e3bb787bf0ba6bb8a5b Mon Sep 17 00:00:00 2001
From: Andreas Schwab <schwab@linux-m68k.org>
Date: Sat, 24 Oct 2009 15:01:03 +0200
Subject: [PATCH] Work around option parsing bug in the busybox tar implementation
Traditionally the first argument of the tar command was interpreted a
bundle of letters specifying the mode of operation and additional options,
with any option arguments taken from subsequent words on the command line
as needed. The implementation of tar in busybox apparently treats this
bundle as if preceded by a dash and then parses it by getopt rules, which
mishandles 'tar xfo -'. Use 'tar xof -' instead which is parsed the same
way by both traditional tar implementations and busybox.
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
templates/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/templates/Makefile b/templates/Makefile
index a12c6e2..408f013 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -50,4 +50,4 @@ clean:
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
(cd blt && $(TAR) cf - .) | \
- (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xfo -)
+ (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xof -)
--
1.6.5.1
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH] Use fast-forward
From: Nanako Shiraishi @ 2009-10-24 13:07 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <1256373092-15126-1-git-send-email-felipe.contreras@gmail.com>
Quoting Felipe Contreras <felipe.contreras@gmail.com>
> As suggested in the mailing list, now I've replaced all instances of 'fast
> forward' with 'fast-forward'.
I had an impression that the consensus from the previous
discussion was that there is no such consensus that this
is an improvement, because there isn't a clear-cut rule?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Felipe Contreras @ 2009-10-24 14:08 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git, Junio C Hamano, Michael J Gruber, Jonathan Nieder
In-Reply-To: <20091024220644.6117@nanako3.lavabit.com>
On Sat, Oct 24, 2009 at 4:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> ...
>> Reworded the getting started section based on comments from Michael J Gruber,
>> Jonathan Nieder and Junio C Hamano.
>
> I'm surprised that you ignored comments from the original
> author of the document you are updating.
I did not. His comment was that if we do this, it must be short, and
it is. If it's not, then it would be more productive to suggest ways
to trim it down.
> Date: Tue, 13 Oct 2009 22:49:40 -0400
> Message-ID: <20091014024940.GB9700@fieldses.org>
I've no way of figuring out what is that. Most people use a direct
link to a mail archive.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Björn Steinbrink @ 2009-10-24 14:14 UTC (permalink / raw)
To: Felipe Contreras
Cc: Nanako Shiraishi, git, Junio C Hamano, Michael J Gruber,
Jonathan Nieder
In-Reply-To: <94a0d4530910240708l7cb59b36s8d6ddebd4af48e7f@mail.gmail.com>
On 2009.10.24 17:08:06 +0300, Felipe Contreras wrote:
> On Sat, Oct 24, 2009 at 4:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> > Date: Tue, 13 Oct 2009 22:49:40 -0400
> > Message-ID: <20091014024940.GB9700@fieldses.org>
>
> I've no way of figuring out what is that. Most people use a direct
> link to a mail archive.
Having the Message-ID is quite useful, so you can search your local mbox
to find the right message. You can also use gmane's Message-ID search:
http://mid.gmane.org/message_id_here
So: http://mid.gmane.org/20091014024940.GB9700@fieldses.org
Björn
^ permalink raw reply
* Re: [PATCH] Use fast-forward
From: Felipe Contreras @ 2009-10-24 14:17 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20091024220709.6117@nanako3.lavabit.com>
On Sat, Oct 24, 2009 at 4:07 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Felipe Contreras <felipe.contreras@gmail.com>
>
>> As suggested in the mailing list, now I've replaced all instances of 'fast
>> forward' with 'fast-forward'.
>
> I had an impression that the consensus from the previous
> discussion was that there is no such consensus that this
> is an improvement, because there isn't a clear-cut rule?
I don't think anyone wants to preserve the current inconsistent use,
and I haven't seen arguments in favor of 'fast forward' either. My
understanding was that the comments were in favor of 'fast-forward'
but making it consistent across the code-base:
http://marc.info/?l=git&m=125532404330701&w=2
http://marc.info/?l=git&m=125535210203730&w=2
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Felipe Contreras @ 2009-10-24 14:22 UTC (permalink / raw)
To: Björn Steinbrink
Cc: Nanako Shiraishi, git, Junio C Hamano, Michael J Gruber,
Jonathan Nieder
In-Reply-To: <20091024141445.GA2078@atjola.homenet>
2009/10/24 Björn Steinbrink <B.Steinbrink@gmx.de>:
> On 2009.10.24 17:08:06 +0300, Felipe Contreras wrote:
>> On Sat, Oct 24, 2009 at 4:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
>> > Date: Tue, 13 Oct 2009 22:49:40 -0400
>> > Message-ID: <20091014024940.GB9700@fieldses.org>
>>
>> I've no way of figuring out what is that. Most people use a direct
>> link to a mail archive.
>
> Having the Message-ID is quite useful, so you can search your local mbox
> to find the right message.
I don't have a local mbox, and I suspect I'm not the only one.
> You can also use gmane's Message-ID search:
> http://mid.gmane.org/message_id_here
That's a nice trick, but then using the following link would serve
exactly the same purpose, wouldn't it?
> So: http://mid.gmane.org/20091014024940.GB9700@fieldses.org
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 3/3] git checkout --nodwim
From: David Roundy @ 2009-10-24 14:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Nanako Shiraishi, Avery Pennarun,
Alex Riesen, git, Jay Soffian
In-Reply-To: <7vzl7h8fjp.fsf@alter.siamese.dyndns.org>
On Sat, Oct 24, 2009 at 2:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> My take on it:
>>
>> 1) --no-porcelain
>>
>> 2) we all are bike-shedding, not being constructive at all
>
> You are right about (2), regarding the option name. I've queued one that
> uses --no-guess.
Perhaps a universal --plumbing flag would be handy? It'd be nice to be
able to pass any command --plumbing and it'll either behave in a
stable, predictable, plumbing-like way, or die with an error message
stating that it isn't a plumbing command. Right now, it's sort of
hard to figure out what is plumbing and what is porcelain. Some
commands are clear, but other commands labelled as plumbing in git(1)
are deprecated in favor of commands labelled as porcelain.
David
^ permalink raw reply
* Re: [PATCH] Use 'fast-forward' all over the place
From: Junio C Hamano @ 2009-10-24 17:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Michael J Gruber
In-Reply-To: <1256373092-15126-2-git-send-email-felipe.contreras@gmail.com>
Is this meant to replace the previous one that is already queued: a0c0ecb
(user-manual: use 'fast-forward', 2009-10-11)?
It seems that these mostly match a mechanical token replacement
"s/([fF])ast forward/$1ast-forward/g" in the Documentation area,
but I suspect there may be some manual fixes.
Token-replace is much harder to review than to produce, as the result of
such mechanical substitution needs to be examined to see if each change
makes sense individually.
I suspect the patch would have been much easier to the reviewers it it
stated somewhere in the log message:
(1) how the mechanical change was produced;
(2) what criteria was used to choose between leaving the mechanical
change as-is and rewording them manually; and
(3) where these non-mechanical changes are.
Here are the list of paths I looked at (during this sitting which did
not go til the end of the patch):
> diff --git a/Documentation/config.txt b/Documentation/config.txt
OK
> diff --git a/Documentation/git-http-push.txt b/Documentation/git-http-push.txt
OK
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
OK, except for two hunks below I am not absolutely sure.
> @@ -60,7 +60,7 @@ EXAMPLES below for details.
> Pushing an empty <src> allows you to delete the <dst> ref from
> the remote repository.
> +
> -The special refspec `:` (or `{plus}:` to allow non-fast forward updates)
> +The special refspec `:` (or `{plus}:` to allow non-fast-forward updates)
> directs git to push "matching" branches: for every branch that exists on
> the local side, the remote side is updated if a branch of the same name
> already exists on the remote side. This is the default operation mode
Hmm, is non-fast-forward a yet another compound word?
> @@ -342,9 +342,9 @@ git push origin :experimental::
Likewise.
> diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
> diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt
> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
> diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> diff --git a/Documentation/howto/maintain-git.txt b/Documentation/howto/maintain-git.txt
> diff --git a/Documentation/howto/revert-branch-rebase.txt b/Documentation/howto/revert-branch-rebase.txt
> diff --git a/Documentation/howto/update-hook-example.txt b/Documentation/howto/update-hook-example.txt
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
OK, except for this hunk I am not sure about.
> @@ -2115,7 +2115,7 @@ $ git checkout release && git pull
>
> Important note! If you have any local changes in these branches, then
> this merge will create a commit object in the history (with no local
> -changes git will simply do a "Fast forward" merge). Many people dislike
> +changes git will simply do a fast-forward merge). Many people dislike
> the "noise" that this creates in the Linux history, so you should avoid
> doing this capriciously in the "release" branch, as these noisy commits
> will become part of the permanent history when you ask Linus to pull
It may be Ok not to emphasize this word but that is not about "fast
forward" vs "fast-forward". It is more about "in this context, this word
does not have to be emphasized" kind of copy-editing which does not have
to be limited to the case where the "word" is 'fast-forward'.
It is my policy not to look at one patch for more than 30 minutes in one
sitting, because I'll get sloppier toward the end, so I'll stop here for
now. Help in reviewing the rest is appreciated.
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Junio C Hamano @ 2009-10-24 17:51 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Michael J Gruber, Jonathan Nieder
In-Reply-To: <1256377489-16719-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Reworded the getting started section based on comments from Michael J Gruber,
> Jonathan Nieder and Junio C Hamano.
Hmm, I thought JBF also had some input...
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Junio C Hamano @ 2009-10-24 18:19 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, git, Michael J Gruber, Jonathan Nieder
In-Reply-To: <7vy6n065os.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Reworded the getting started section based on comments from Michael J Gruber,
>> Jonathan Nieder and Junio C Hamano.
>
> Hmm, I thought JBF also had some input...
Ah, nevermind. Yes, he did have input, and I tend to agree with him, and
more importantly trust his judgement on the manual.
I think a "Getting started" section that only covers "git config" looks
way out of place in the beginning of this document.
Manuals by other people that teach "here is how you would do a hello-world
repository" would want to teach user.name before reaching that point, but
because the user-manual is written in such a way that it first introduces
concepts to understand what is going on without changing anything, we do
not have much need user.name until it gets to "Developing with git"
section.
"Many people prefer to teach it this way" does not justify "everybody must
teach it this way" an iota, when teaching "config user.name" upfront will
fit the flow of how they teach but does not fit the flow of how this
manual teaches [*1*].
I'm inclined to to discard the first patch.
The point of the original text the second patch touches was to show how
simple the contents of the configuration file is and give the users that
there is nothing magic there. While I do not like the second patch as-is,
because it destroys that nice property and treats the end users mindless
"cut-and-paste without thinking" sheeples, I think that it is rather vague
and unhelpful to the current target audience to say:
... The easiest way to do so is to make sure the following lines
appear in a file named .gitconfig in your home directory:
and the parts can use some improvement. For example, "home directory"
does not hold true for people on platforms that lack the concept. Keeping
the current "the following lines appear", rewording "in a file named
.gitconfig in your home directory" with "in your per-user configuration
file", keeping the display that shows how the config snippet should look
like, and using "config --global -e" might be a better approach.
[Footnote]
*1* Unless you are changing the flow of how this manual teaches at the
same time, that is. And no, I am not suggesting that we should start from
"let's do a hello-world repository from scratch". I think the current
"start from read-only and then learn how to grow history later" is one
valid way to teach.
^ permalink raw reply
* Re: [PATCH] Use 'fast-forward' all over the place
From: Felipe Contreras @ 2009-10-24 19:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael J Gruber
In-Reply-To: <7v3a587kc2.fsf@alter.siamese.dyndns.org>
On Sat, Oct 24, 2009 at 8:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Is this meant to replace the previous one that is already queued: a0c0ecb
> (user-manual: use 'fast-forward', 2009-10-11)?
Yes.
> It seems that these mostly match a mechanical token replacement
> "s/([fF])ast forward/$1ast-forward/g" in the Documentation area,
> but I suspect there may be some manual fixes.
>
> Token-replace is much harder to review than to produce, as the result of
> such mechanical substitution needs to be examined to see if each change
> makes sense individually.
I manually replaced each instance, and reviewed the patch myself. Most
of the changes are essentially the same, except a few instances:
"Fast forward" -> fast-forward
Fast Forward Only -> Fast-forward Only
> I suspect the patch would have been much easier to the reviewers it it
> stated somewhere in the log message:
>
> (1) how the mechanical change was produced;
There wasn't such.
> (2) what criteria was used to choose between leaving the mechanical
> change as-is and rewording them manually; and
If it wasn't straight forward. I considered the following straightforward:
fast forward -> fast-forward
fast forwarded -> fast-forwarded
fast forwarding -> fast-forwarding
fast forwardable -> fast-forwardable
non-fast forward -> non-fast-forward
Fast forward -> Fast-forward
Fast forwarding -> Fast-forwarding
> (3) where these non-mechanical changes are.
Mentioned on the second comment.
> Here are the list of paths I looked at (during this sitting which did
> not go til the end of the patch):
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>
> OK
>
>> diff --git a/Documentation/git-http-push.txt b/Documentation/git-http-push.txt
>
> OK
>
>> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
>
> OK, except for two hunks below I am not absolutely sure.
>
>> @@ -60,7 +60,7 @@ EXAMPLES below for details.
>> Pushing an empty <src> allows you to delete the <dst> ref from
>> the remote repository.
>> +
>> -The special refspec `:` (or `{plus}:` to allow non-fast forward updates)
>> +The special refspec `:` (or `{plus}:` to allow non-fast-forward updates)
>> directs git to push "matching" branches: for every branch that exists on
>> the local side, the remote side is updated if a branch of the same name
>> already exists on the remote side. This is the default operation mode
>
> Hmm, is non-fast-forward a yet another compound word?
Yes. AFAIK.
>> @@ -342,9 +342,9 @@ git push origin :experimental::
>
> Likewise.
>
>> diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
>> diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt
>> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
>> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
>> diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
>> diff --git a/Documentation/howto/maintain-git.txt b/Documentation/howto/maintain-git.txt
>> diff --git a/Documentation/howto/revert-branch-rebase.txt b/Documentation/howto/revert-branch-rebase.txt
>> diff --git a/Documentation/howto/update-hook-example.txt b/Documentation/howto/update-hook-example.txt
>> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
>
> OK, except for this hunk I am not sure about.
>
>> @@ -2115,7 +2115,7 @@ $ git checkout release && git pull
>>
>> Important note! If you have any local changes in these branches, then
>> this merge will create a commit object in the history (with no local
>> -changes git will simply do a "Fast forward" merge). Many people dislike
>> +changes git will simply do a fast-forward merge). Many people dislike
>> the "noise" that this creates in the Linux history, so you should avoid
>> doing this capriciously in the "release" branch, as these noisy commits
>> will become part of the permanent history when you ask Linus to pull
>
> It may be Ok not to emphasize this word but that is not about "fast
> forward" vs "fast-forward". It is more about "in this context, this word
> does not have to be emphasized" kind of copy-editing which does not have
> to be limited to the case where the "word" is 'fast-forward'.
I couldn't parse that. From what I can see "Fast forward" was
emphasized because the author thought the words didn't make much sense
separated. Now that the word is fast-forward, there's no need to
emphasize.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] tar: on extract, -o is --no-same-owner
From: Junio C Hamano @ 2009-10-24 19:25 UTC (permalink / raw)
To: Andreas Schwab
Cc: Bernhard Reutner-Fischer, Junio C Hamano, vda.linux, busybox, git
In-Reply-To: <m2ocnxuej2.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Like this.
Yeah, correct and it beautifly explains the issue. Thanks.
>>From ebadb41b346c305b94f27e3bb787bf0ba6bb8a5b Mon Sep 17 00:00:00 2001
> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Sat, 24 Oct 2009 15:01:03 +0200
> Subject: [PATCH] Work around option parsing bug in the busybox tar implementation
>
> Traditionally the first argument of the tar command was interpreted a
> bundle of letters specifying the mode of operation and additional options,
> with any option arguments taken from subsequent words on the command line
> as needed. The implementation of tar in busybox apparently treats this
> bundle as if preceded by a dash and then parses it by getopt rules, which
> mishandles 'tar xfo -'. Use 'tar xof -' instead which is parsed the same
> way by both traditional tar implementations and busybox.
>
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
> ---
> templates/Makefile | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/templates/Makefile b/templates/Makefile
> index a12c6e2..408f013 100644
> --- a/templates/Makefile
> +++ b/templates/Makefile
> @@ -50,4 +50,4 @@ clean:
> install: all
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
> (cd blt && $(TAR) cf - .) | \
> - (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xfo -)
> + (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xof -)
> --
> 1.6.5.1
>
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
^ permalink raw reply
* Re: [PATCH] add tests for git diff --submodule
From: Junio C Hamano @ 2009-10-24 19:25 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <4AE192AD.70603@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Apart from your changes necessary to make the test run again my changes are:
>
> - rename from "t4041-diff-submodule-summary.sh" to "t4041-diff-submodule.sh"
> - corrected all comments still speaking of "summary"
> - added tests to test the behaviour of "--submodule" and "--submodule=short"
Thanks for a nice summary.
^ permalink raw reply
* Re: [PATCH 3/3] git checkout --nodwim
From: Junio C Hamano @ 2009-10-24 19:25 UTC (permalink / raw)
To: David Roundy
Cc: Johannes Schindelin, Nanako Shiraishi, Avery Pennarun,
Alex Riesen, git, Jay Soffian
In-Reply-To: <117f2cc80910240759oa9f57e7h67f06816d37e328c@mail.gmail.com>
David Roundy <roundyd@physics.oregonstate.edu> writes:
> Perhaps a universal --plumbing flag would be handy?
Yes in general but it is unclear what aspect of its behaviour we will
be casting in stone with a generic --plumbing option in this case. I
also think that "checkout" Porcelain is not yet mature enough for us
to do this right now. For example, I am reasonably sure that somebody
motivated enough will teach it to touch submodule trees when switching
to another branch by default, and it is unclear if we should turn off
these expected additions when --plumbing is seen.
^ permalink raw reply
* Re: [PATCH] Use 'fast-forward' all over the place
From: Junio C Hamano @ 2009-10-24 19:44 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Michael J Gruber
In-Reply-To: <94a0d4530910241212s5d644eb6u66c6f96feafcaf10@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
>> I suspect the patch would have been much easier to the reviewers it it
>> stated somewhere in the log message:
>>
>> (1) how the mechanical change was produced;
>
> There wasn't such.
That is actually a bad news; it is even worse than mechanical spotting
followed by manual inspection. It would force us feel _more_ worried, as
we would then need to grep for leftovers that your manual conversion may
not have caught. Sigh...
>> (2) what criteria was used to choose between leaving the mechanical
>> change as-is and rewording them manually; and
>
> If it wasn't straight forward. I considered the following straightforward:
> fast forward -> fast-forward
> fast forwarded -> fast-forwarded
> fast forwarding -> fast-forwarding
> fast forwardable -> fast-forwardable
> non-fast forward -> non-fast-forward
> Fast forward -> Fast-forward
> Fast forwarding -> Fast-forwarding
All of these are what "s/([fF])ast forward/$1ast-forward/g" does, aren't
they?
> I couldn't parse that. From what I can see "Fast forward" was
> emphasized because the author thought the words didn't make much sense
> separated. Now that the word is fast-forward, there's no need to
> emphasize.
Even after your patch, hunk beginning on line 1384 of the
user-manual says
... then git just performs a "fast-forward"; the head of the ...
and I think you did the right thing by keeping these dq here. This is the
first occurrence of the word followed by its explanation and for that
reason, the word deserves to be emphasized---IOW, the context calls for an
emphasis.
In the "Important note!" part, we talk about the pull operation that
normally creates a merge commit, and _in contrast_, under a particular
condition (namely, "no local changes"), it does something different
(namely, a "fast-forward"). We should keep the emphasis on "fast-forward"
here for exactly the same reason---the context calls for an emphasis.
I was about to read the rest of the patch (the non-doc part) but instead I
ended up spending another 20 minutes writing this message, so the review
by me on the remainder has to wait til sometime later.
^ permalink raw reply
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Felipe Contreras @ 2009-10-24 20:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Michael J Gruber, Jonathan Nieder, J. Bruce Fields,
Hannu Koivisto, Jeff King, Wincent Colaiuta, Matthias Lederhofer
In-Reply-To: <7vr5ss64e5.fsf@alter.siamese.dyndns.org>
On Sat, Oct 24, 2009 at 9:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Reworded the getting started section based on comments from Michael J Gruber,
>>> Jonathan Nieder and Junio C Hamano.
>>
>> Hmm, I thought JBF also had some input...
>
> Ah, nevermind. Yes, he did have input, and I tend to agree with him, and
> more importantly trust his judgement on the manual.
JBF said this[1]:
If we have to do this, just keep it short....
And I am trying to keep it short.
> I think a "Getting started" section that only covers "git config" looks
> way out of place in the beginning of this document.
Now you are saying that the fact that it's short is a bad thing? That
goes against to what JBF said.
And let's not forget your previous comments
[2]: I think a "getting started" section near the beginning of the manual is a
good idea (and ll.40- is a very early part of the manual).
[3]: I actually wish this section appeared a lot earlier in the document, but
that is a separate issue.
> Manuals by other people that teach "here is how you would do a hello-world
> repository" would want to teach user.name before reaching that point, but
> because the user-manual is written in such a way that it first introduces
> concepts to understand what is going on without changing anything, we do
> not have much need user.name until it gets to "Developing with git"
> section.
>
> "Many people prefer to teach it this way" does not justify "everybody must
> teach it this way" an iota, when teaching "config user.name" upfront will
> fit the flow of how they teach but does not fit the flow of how this
> manual teaches [*1*].
Nobody argued that "everybody must teach it this way", the argument
was that most people find it easier, and considering the section is
about "developing with git" it is sensible to avoid burdening the
reader with concepts that don't pertain to the objective at hand,
which is getting them to configure their user.
And let's not forget that the current text is broken for Windows users.
> I'm inclined to to discard the first patch.
And you decided to mention that after many people including you, have
agreed that it's a good idea?
> The point of the original text the second patch touches was to show how
> simple the contents of the configuration file is and give the users that
> there is nothing magic there. While I do not like the second patch as-is,
> because it destroys that nice property and treats the end users mindless
> "cut-and-paste without thinking" sheeples,
What's wrong with teaching one thing at a time? Configuring the user
is something so essential, I don't think it makes sense to make the
task difficult on purpose. Some people might avoid doing it precisely
because of that.
> I think that it is rather vague
> and unhelpful to the current target audience to say:
>
> ... The easiest way to do so is to make sure the following lines
> appear in a file named .gitconfig in your home directory:
>
> and the parts can use some improvement. For example, "home directory"
> does not hold true for people on platforms that lack the concept. Keeping
> the current "the following lines appear", rewording "in a file named
> .gitconfig in your home directory" with "in your per-user configuration
> file", keeping the display that shows how the config snippet should look
> like, and using "config --global -e" might be a better approach.
If you read the results of the last git survey you'll see that the
area that needs most improvement is the documentation. Also I still
see many people doing commits without configuring the user name and
email properly and so I've tried very hard to improve the user manual
to make it easier for them to understand they must do that. In the
process I've added the --edit option to 'git config' and the new
"getting started" section, in order to address all the issues
mentioned in previous threads and gone through several iterations of
these patches already.
I'm starting to think that all the previous "constructive" feedback
was actually targeted to deter people from making any changes.
I'm CC'ing people that have been involved in previous threads.
> [Footnote]
>
> *1* Unless you are changing the flow of how this manual teaches at the
> same time, that is. And no, I am not suggesting that we should start from
> "let's do a hello-world repository from scratch". I think the current
> "start from read-only and then learn how to grow history later" is one
> valid way to teach.
I also don't think the flow should be changed, that's why I didn't put
the user configuration on the "getting started" section. It goes into
the "developing with git" section.
[1] http://article.gmane.org/gmane.comp.version-control.git/130236
[2] http://article.gmane.org/gmane.comp.version-control.git/115649
[3] http://article.gmane.org/gmane.comp.version-control.git/106667
--
Felipe Contreras
^ permalink raw reply
* [PATCH 0/2] gitk: Fix error display when Tcl is too old
From: Bernt Hansen @ 2009-10-24 20:20 UTC (permalink / raw)
To: git; +Cc: Paul Mackerras, Bernt Hansen
The following patch series cleans up error message reporting when your
version of Tcl is too old.
The old code checked the Tcl version first and then tried to report
the error with show_error. show_error uses msgcat for translation but
msgcat is not yet initialized when we are checking the Tcl version
requirement.
The first patch moves the initialization of msgcat before the check
for the Tcl version. This version will fail is msgcat is not
available.
The second patch handles the case where the msgcat package is not
available by providing a default mc procedure than just returns the
argument text unchanged (essentially bypassing message text
translation).
This lets us continue to use show_error as-is.
Bernt Hansen (2):
gitk: Initialize msgcat before first use
gitk: Provide a default mc function if msgcat is not available
gitk | 46 ++++++++++++++++++++++++++--------------------
1 files changed, 26 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH 1/2] gitk: Initialize msgcat before first use
From: Bernt Hansen @ 2009-10-24 20:20 UTC (permalink / raw)
To: git; +Cc: Paul Mackerras, Bernt Hansen
In-Reply-To: <1256415640-10328-1-git-send-email-bernt@norang.ca>
The error text generated when your version of Tcl is too old is
translated with msgcat (mc) before msgcat is initialized. This
causes Tcl to abort with:
Error in startup script: invalid command name "mc"
We now initialize msgcat first before we check the Tcl version. Msgcat
is available since Tcl 8.1.
Signed-off-by: Bernt Hansen <bernt@norang.ca>
---
gitk | 41 +++++++++++++++++++++--------------------
1 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/gitk b/gitk
index a0214b7..d4cd566 100755
--- a/gitk
+++ b/gitk
@@ -11004,7 +11004,27 @@ proc get_path_encoding {path} {
return $tcl_enc
}
-# First check that Tcl/Tk is recent enough
+# First setup up mc for translating text
+## For msgcat loading, first locate the installation location.
+if { [info exists ::env(GITK_MSGSDIR)] } {
+ ## Msgsdir was manually set in the environment.
+ set gitk_msgsdir $::env(GITK_MSGSDIR)
+} else {
+ ## Let's guess the prefix from argv0.
+ set gitk_prefix [file dirname [file dirname [file normalize $argv0]]]
+ set gitk_libdir [file join $gitk_prefix share gitk lib]
+ set gitk_msgsdir [file join $gitk_libdir msgs]
+ unset gitk_prefix
+}
+
+## Internationalization (i18n) through msgcat and gettext. See
+## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
+package require msgcat
+namespace import ::msgcat::mc
+## And eventually load the actual message catalog
+::msgcat::mcload $gitk_msgsdir
+
+# Check that Tcl/Tk is recent enough
if {[catch {package require Tk 8.4} err]} {
show_error {} . [mc "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
Gitk requires at least Tcl/Tk 8.4."]
@@ -11096,25 +11116,6 @@ if {[tk windowingsystem] eq "aqua"} {
set ctxbut <Button-3>
}
-## For msgcat loading, first locate the installation location.
-if { [info exists ::env(GITK_MSGSDIR)] } {
- ## Msgsdir was manually set in the environment.
- set gitk_msgsdir $::env(GITK_MSGSDIR)
-} else {
- ## Let's guess the prefix from argv0.
- set gitk_prefix [file dirname [file dirname [file normalize $argv0]]]
- set gitk_libdir [file join $gitk_prefix share gitk lib]
- set gitk_msgsdir [file join $gitk_libdir msgs]
- unset gitk_prefix
-}
-
-## Internationalization (i18n) through msgcat and gettext. See
-## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
-package require msgcat
-namespace import ::msgcat::mc
-## And eventually load the actual message catalog
-::msgcat::mcload $gitk_msgsdir
-
catch {source ~/.gitk}
font create optionfont -family sans-serif -size -12
--
1.6.5.1.69.g36942
^ permalink raw reply related
* [PATCH 2/2] gitk: Provide a default mc function if msgcat is not available
From: Bernt Hansen @ 2009-10-24 20:20 UTC (permalink / raw)
To: git; +Cc: Paul Mackerras, Bernt Hansen
In-Reply-To: <1256415640-10328-1-git-send-email-bernt@norang.ca>
Msgcat is available since Tcl 8.1. For really old versions of Tcl
provide a default mc that just returns the text untranslated. This
allows the Tcl version check to return the error in a window instead
of making Tcl abort when attempting to load the msgcat package.
Signed-off-by: Bernt Hansen <bernt@norang.ca>
---
I'm not sure if we care about Tcl versions older than 8.1 but this at
least shows the error in the window with the [OK] button.
gitk | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/gitk b/gitk
index d4cd566..bff891d 100755
--- a/gitk
+++ b/gitk
@@ -11019,10 +11019,15 @@ if { [info exists ::env(GITK_MSGSDIR)] } {
## Internationalization (i18n) through msgcat and gettext. See
## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
-package require msgcat
-namespace import ::msgcat::mc
-## And eventually load the actual message catalog
-::msgcat::mcload $gitk_msgsdir
+if {[catch {package require msgcat}]} {
+ proc mc {arg} {
+ return $arg
+ }
+} else {
+ namespace import ::msgcat::mc
+ ## And eventually load the actual message catalog
+ ::msgcat::mcload $gitk_msgsdir
+}
# Check that Tcl/Tk is recent enough
if {[catch {package require Tk 8.4} err]} {
--
1.6.5.1.69.g36942
^ permalink raw reply related
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