* Re: [PATCH 0/2] Trivial cleanups in the post-receive-email script
From: Junio C Hamano @ 2012-02-27 19:01 UTC (permalink / raw)
To: mhagger; +Cc: git, Kevin P. Fleming, Andy Parkins
In-Reply-To: <1330367650-23091-1-git-send-email-mhagger@alum.mit.edu>
Shows that nobody uses these sample script.
Will apply. I wish all the patches were this easy ;-).
Thanks.
^ permalink raw reply
* Re: [PATCH] fsck: do not print dangling objects by default
From: Jeff King @ 2012-02-27 19:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Clemens Buchacher, git
In-Reply-To: <7vhayddxgp.fsf@alter.siamese.dyndns.org>
On Sun, Feb 26, 2012 at 02:46:46PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I think that both the ultimate goal explained above, and the direction in
> > which the documentation updates tries to move us, are good. I only gave a
> > cursory look at the code changes, but what they implement seems to match
> > the intention.
> >
> > Of course I may be missing something, so objections from others to argue
> > why we shouldn't do this is very much welcomed to stop me and Clemens ;-).
>
> Let's start with the obvious.
>
> It is much easier for a user to use a new option on the command line when
> he wants to use an improved behaviour when he runs the command manually.
> Having to update scripts that run the command to act on its output, on the
> other hand, is much more painful to the users.
>
> And the intended audience for this change clearly is interactive users
> that follow the user-manual to try things out.
>
> Given that, isn't it not just sufficient but actually better to instead
> add a new --no-dangling option and keep the default unchanged?
But if your intended audience is users who are confused by the dangling
warnings, explaining to them to use --no-dangling is not really
improving the situation. Of course, it is fsck, so I wonder how often
clueless people are really running it in the first place (i.e., it is
not and should not be part of most users' typical workflows). If it is
simply the case that they are being told to run "git fsck" by more
expert users without understanding what it does, then I could buy the
argument that those expert users could just as easily say "git fsck
--no-dangling".
-Peff
^ permalink raw reply
* Re: git-cherries
From: Jeff King @ 2012-02-27 19:27 UTC (permalink / raw)
To: Thien-Thi Nguyen; +Cc: git
In-Reply-To: <874nucee98.fsf@gnuvola.org>
On Mon, Feb 27, 2012 at 11:56:19AM +0100, Thien-Thi Nguyen wrote:
> For my personal use, i wrote git-cherries, attached.
> It commits each hunk of every modified file separately
> (creating cherries to cherry-pick later, you see).
>
> I am writing to ask if this is already in Git somewhere,
> and if not, for tips on how to make it faster / more elegant.
So if I understand correctly, this just creates a series of commits, one
per hunk, of what's in your working tree. And the commit messages won't
be useful, so this is really about recording the work somewhere so that
you can pick it out later using "git cherry-pick --no-commit", make a
real commit from some subset of the cherries, and then throw away the
cherries?
I think you could do this more simply by putting everything in a single
throw-away commit, then using "git checkout -p $throwaway" to pick the
individual cherries from the single commit. You don't grab the commit
message from $throwaway as you might with cherry-pick, but by definition
it's not a very good commit message anyway.
-Peff
^ permalink raw reply
* Re: [PATCH] fsck: do not print dangling objects by default
From: Junio C Hamano @ 2012-02-27 19:29 UTC (permalink / raw)
To: Jeff King; +Cc: Clemens Buchacher, git
In-Reply-To: <20120227191846.GB1600@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Given that, isn't it not just sufficient but actually better to instead
>> add a new --no-dangling option and keep the default unchanged?
>
> ... Of course, it is fsck, so I wonder how often clueless people are
> really running it in the first place (i.e., it is not and should not be
> part of most users' typical workflows). If it is simply the case that
> they are being told to run "git fsck" by more expert users without
> understanding what it does, then I could buy the argument that those
> expert users could just as easily say "git fsck --no-dangling".
Yes, that was certainly part of my pros-and-cons analysis. If you run
"git fsck" without "--no-dangling" without reading the manual, you may
get confused, but that is *not* the primary audience. People who are
curious can read the manual and figure it out, and the need for "fsck" is
much rarer these days, compared to 2005 ;-)
In that context, only large downsides of potentially breaking and having
to adjust existing scripts remains without much upsides, if we were to
switch the default.
^ permalink raw reply
* Re: [PATCH] branch: don't assume the merge filter ref exists
From: Jeff King @ 2012-02-27 19:30 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Bernhard Reutner-Fischer, git, Junio C Hamano
In-Reply-To: <1330355513-22351-1-git-send-email-cmn@elego.de>
On Mon, Feb 27, 2012 at 04:11:53PM +0100, Carlos Martín Nieto wrote:
> print_ref_list looks up the merge_filter_ref and assumes that a valid
> pointer is returned. When the object doesn't exist, it tries to
> dereference a NULL pointer. This can be the case when git branch
> --merged is given an argument that isn't a valid commit name.
>
> Check whether the lookup returns a NULL pointer and die with an error
> if it does. Add a test, while we're at it.
>
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> ---
>
> It certainly looks like --merged was only ever supposed to be used
> with branch names, as it assumed that get_sha1() would catch the
> errors.
>
> I'm not sure if "bad object" or "invalid object" fits better. "bad
> object" might have a stronger implication that it exists but is
> corrupt.
You would also get NULL if the object exists but is not a commit. Maybe:
die("object '%s' does not point to a commit", ...)
would be better? It covers the wrong-type case, and is still technically
true when the object does not exist.
-Peff
^ permalink raw reply
* Re: [PATCH] branch: don't assume the merge filter ref exists
From: Junio C Hamano @ 2012-02-27 19:33 UTC (permalink / raw)
To: Jeff King; +Cc: Carlos Martín Nieto, Bernhard Reutner-Fischer, git
In-Reply-To: <20120227193044.GD1600@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> You would also get NULL if the object exists but is not a commit. Maybe:
>
> die("object '%s' does not point to a commit", ...)
>
> would be better? It covers the wrong-type case, and is still technically
> true when the object does not exist.
For this particular message I like the above a lot better. The output
from "git grep -e 'invalid object' -e 'bad object'" seems to show that
the use of both are fairly evenly distributed.
^ permalink raw reply
* Re: [PATCH] branch: don't assume the merge filter ref exists
From: Jeff King @ 2012-02-27 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, Bernhard Reutner-Fischer, git
In-Reply-To: <7vk4386pgi.fsf@alter.siamese.dyndns.org>
On Mon, Feb 27, 2012 at 11:33:49AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > You would also get NULL if the object exists but is not a commit. Maybe:
> >
> > die("object '%s' does not point to a commit", ...)
> >
> > would be better? It covers the wrong-type case, and is still technically
> > true when the object does not exist.
>
> For this particular message I like the above a lot better. The output
> from "git grep -e 'invalid object' -e 'bad object'" seems to show that
> the use of both are fairly evenly distributed.
It looks like "bad object" generally comes from parse_object failing,
which makes sense. It either means object corruption or you fed a full
40-char sha1 that didn't exist (which, if you are being that specific,
probably is an indication of broken-ness in your repository).
It looks like "invalid object" comes from failing to access the subject
of an annotated tag or an entry in a tree, both of which would meet the
same criteria (corruption or a missing 40-char sha1).
I don't think bad versus invalid in existing cases is a big deal, as
they are both used consistently. But in this case, I think either would
be wrong, since it is equally likely that the user gave an existing, OK
object of the wrong type.
-Peff
^ permalink raw reply
* [PATCH v2 1/2] CodingGuidelines: Add a note about spaces after redirection
From: Tim Henigan @ 2012-02-24 23:12 UTC (permalink / raw)
To: git, gitster; +Cc: tim.henigan
During code review of some patches, it was noted that redirection operators
should have space before, but no space after them.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
Updated to include double-quotes around redirection target and also
document why they are needed.
Documentation/CodingGuidelines | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4830086..5a190b9 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -35,6 +35,16 @@ For shell scripts specifically (not exhaustive):
- Case arms are indented at the same depth as case and esac lines.
+ - Redirection operators should be written with space before, but
+ no space after them. For example:
+ 'echo test >"$file"' is preferred over
+ 'echo test > "$file"'
+
+ Note that even though it is not required by POSIX to double-
+ quote the redirection target in a variable (as shown above),
+ our code does so because some versions of bash issue a warning
+ without them.
+
- We prefer $( ... ) for command substitution; unlike ``, it
properly nests. It should have been the way Bourne spelled
it from day one, but unfortunately isn't.
--
1.7.9.1
^ permalink raw reply related
* [PATCH v2 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts
From: Tim Henigan @ 2012-02-24 23:12 UTC (permalink / raw)
To: git, gitster; +Cc: tim.henigan
In-Reply-To: <1330125178-9194-1-git-send-email-tim.henigan@gmail.com>
During the code review of a recent patch, it was noted that shell scripts
must not use 'which'. The output of the command is not machine parseable
and its exit code is not reliable across platforms.
It is better to use 'type' to accomplish this task.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
Updated to the documentation pattern recommended by Junio Hamano:
"If you want to do Z, use X not Y, because Y is broken ..."
I grepped through the code and found the 'type <command' is indeed used
in place of 'which' to test for the presence of commands on $PATH.
Documentation/CodingGuidelines | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 5a190b9..816c5ad 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -49,6 +49,11 @@ For shell scripts specifically (not exhaustive):
properly nests. It should have been the way Bourne spelled
it from day one, but unfortunately isn't.
+ - If you want to find out if a command is available on the user's
+ $PATH, you should use 'type <command>', instead of 'which'.
+ The output of 'which' is not machine parseable and its exit code
+ is not reliable across platforms.
+
- We use POSIX compliant parameter substitutions and avoid bashisms;
namely:
--
1.7.9.1
^ permalink raw reply related
* Re: [PATCH 01/11] Add more large blob test cases
From: Peter Baumann @ 2012-02-27 20:18 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1330329315-11407-2-git-send-email-pclouds@gmail.com>
A minor spelling error in the text.
On Mon, Feb 27, 2012 at 02:55:05PM +0700, Nguyễn Thái Ngọc Duy wrote:
> New test cases list commands that should work when memory is
> limited. All memory allocation functions (*) learn to reject any
> allocation larger than $GIT_ALLOC_LIMIT if set.
>
> (*) Not exactly all. Some places do not use x* functions, but
> malloc/calloc directly, notably diff-delta. These could path should
^code
> never be run on large blobs.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
-Peter
^ permalink raw reply
* Re: [PATCH] send-email: document the --smtp-debug option
From: Junio C Hamano @ 2012-02-27 20:19 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, greened
In-Reply-To: <1330359773-16759-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> The option was already shown in -h output, so it should be documented
> in the man page.
>
> Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
> Suggested-by: David A. Greene <greened@obbligato.org>
> ---
> David Greene wrote:
>> I don't think --smtp-debug is documented in the man pages. Was that a
>> deliberate decision or an oversight?
Probably halfway in between "Meh"; anybody who is willing to look into the
issue will notice when he opens the script and reads it for the first time
anyway, so why bother.
But now "A patch already exists, and it does not seem to have any funny
letters that may screw up asciidoc, so why bother rejecting" ;-)
Will apply; thanks.
> Documentation/git-send-email.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 327233c..3241170 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -198,6 +198,10 @@ must be used for each option.
> if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
> then authentication is not attempted.
>
> +--smtp-debug=0|1::
> + Enable (1) or disable (0) debug output. If enabled, SMTP
> + commands and replies will be printed. Useful to debug TLS
> + connection and authentication problems.
>
> Automating
> ~~~~~~~~~~
^ permalink raw reply
* Re: [PATCH] grep -P: add tests for matching ^ and $
From: Junio C Hamano @ 2012-02-27 20:21 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, gitster, michal.kiedrowicz
In-Reply-To: <1330361149-26741-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> From: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
>
> When a fix for matching ^ and $ with -P was commited to master in
> fba4f1 (grep -P: Fix matching ^ and $), the tests were missing the
> LIBPCRE prerequisite check and were dropped from the patch. Here are
> the tests guarded with LIBPCRE.
Thanks.
The real reason I separated it was because I didn't want to worry about
the test part when merging this down to older maintenance releases.
^ permalink raw reply
* Bug: post-receive-email sometimes misses commits
From: Michael Haggerty @ 2012-02-27 20:39 UTC (permalink / raw)
To: git discussion list; +Cc: Andy Parkins
In the following scenario, suppose that commit X is already in the
repository (referenced by master), and I push references A and B at the
same time:
-- master
/
o---X---*'--*'--* A
\
*---* B
The commits marked with "*" are new to the repository, and therefore the
emails generated by post-receive-email should summarize all five of
them. But in fact the commits marked with a prime (i.e., A^ and A^^)
are NOT included in any email.
The problem is that post-receive-email operates on one reference update
at a time, *after* the references have been updated. It generates the
"new" commits for A using the equivalent of
git rev-list ^master ^B A
and it generates the "new" commits for B using the equivalent of
git rev-list ^master ^B A
Neither of these ranges includes the commits common to branches A and B
and therefore those commits do not appear in any of the email notifications.
I first observed this problem in a slightly more complicated scenario
where a bugfix branch was merged to two release branches, then the two
release branches (but not the bugfix reference) were pushed:
o---o-------* release-A
/ /
o---o---o---*'--*'--*' bugfix
\ \
o---o-------* release-B
In this case only the merge commits were included in the commit emails;
the bugfixes commits themselves were never mentioned.
It seems like this problem can only be fixed by having
post-receive-email process all of the reference updates at once so that
the email generation for one reference update can take into account the
other references that were updated at the same time.
I don't plan to work on this problem; locally, we are using a Python
script that was derived from post-receive-email and enhanced in several
ways rather than using the Shell-script original. So when I have fixed
our Python script my own itch will have been scratched. If there is
interest in the Python email-notification script please let me know and
there is a chance I might clean it up enough to be released.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Bug: post-receive-email sometimes misses commits
From: Michael Haggerty @ 2012-02-27 20:43 UTC (permalink / raw)
To: git discussion list; +Cc: Andy Parkins
In-Reply-To: <4F4BEA09.90600@alum.mit.edu>
On 02/27/2012 09:39 PM, Michael Haggerty wrote:
> [...] It generates the
> "new" commits for A using the equivalent of
>
> git rev-list ^master ^B A
>
> and it generates the "new" commits for B using the equivalent of
>
> git rev-list ^master ^B A
Sorry, that last line should of course be
git rev-list ^master ^A B
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 2/3] parse-options: allow positivation of options starting, with no-
From: Junio C Hamano @ 2012-02-27 20:48 UTC (permalink / raw)
To: René Scharfe
Cc: Thomas Rast, git, Bert Wesarg, Geoffrey Irving,
Johannes Schindelin, Pierre Habouzit
In-Reply-To: <4F4BC3B3.7080000@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The patch does not forbid adding "no-" to an option that already starts
> with "no-". This stricter rule would be easy to add, but since that is
> currently the only way to negate such options, it would break backwards
> compatibility and thus should be added in a separate patch, if at all.
>
> With the patch, the following guidelines are followed:
>
> - "no-" means no, for both developers and users.
> - The user doesn't have to to say "no-no-".
>
> The results feels simpler to me.
Sounds fair.
I agree that the backward compatibility of --no-no-foo is a potential
problem, if any of the actions controlled by "--no-foo" option defaults to
the behaviour when "--no-foo" is given. Among the existing 13 that you
listed, I do not think there is any that tempts any existing user to ask
for negation with "--no-no-foo" form, so I think we should be Ok.
^ permalink raw reply
* Re: [PATCH v2 1/2] CodingGuidelines: Add a note about spaces after redirection
From: Junio C Hamano @ 2012-02-27 20:55 UTC (permalink / raw)
To: Tim Henigan; +Cc: git
In-Reply-To: <1330125178-9194-1-git-send-email-tim.henigan@gmail.com>
Thanks; will apply both.
^ permalink raw reply
* Re: Announcing nntpgit
From: Michael Haggerty @ 2012-02-27 21:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Corbet, git
In-Reply-To: <7v62evykrq.fsf@alter.siamese.dyndns.org>
On 02/24/2012 10:38 PM, Junio C Hamano wrote:
> This is something I wanted to write (or see somebody write so that I can
> use it ;-)) even before I became the maintainer of this project, as I
> practically live inside GNUS, but never got around to go beyond the design
> phase.
>
> How do you handle message threading (References: and In-Reply-To:)?
>
> Would a commit on the "mainline" (a rough approximation of it would be
> "log --first-parent" starting from the tip) form the discussion starter
> article, and any side branch that fork from them would be a discussion
> thread starting at the commit?
>From contrib/hooks/post-receive-email I've locally derived a Python
version that sends one email message per commit. It generates a thread
of messages for each reference that was pushed, starting with a summary
of how the reference was changed, and one followup email for each new
commit added to that reference. In a threaded mail reader, this groups
all of the changes to a reference due to a particular push in a thread
that can easily be expanded, collapsed, or deleted as a group. I find
this a convenient workflow.
I believe that NNTP threading works much the same way as email
threading, so probably the same style could be used there. My script is
still not published, though.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: git-subtree Ready #2
From: Junio C Hamano @ 2012-02-27 21:06 UTC (permalink / raw)
To: David A. Greene; +Cc: Avery Pennarun, Jeff King, git
In-Reply-To: <87ty2ft0tm.fsf@smith.obbligato.org>
greened@obbligato.org (David A. Greene) writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> I'm happy to do either (rebase or filter-branch). Just let me know.
>>
>> I would understand Avery's "should we filter-branch/rebase, or is it OK
>> as-is?", but I do not understand what you mean by "either rebase or
>> filter-branch is fine".
>
> Sorry, got mixed up there. I'm not that familiar with filter-branch.
> Now I understand you do both. :)
>
> So have we decided to keep the history?
I think the discussion so far was:
- Peff suggested to keep the history with a true merge;
- I said the history before the final commit in Avery's tree did not look
so useful for future archaeology; and then
- Avery corrected me that there are contributions by other people and the
credits will be lost if we discarded the history;
and everybody (including me) now favors to have the history.
So the answer to your question is yes, but I do not think we heard opinion
from anybody regarding the question by Avery yet. I personally do not see
how it would help us if the old history is rewritten at this point.
^ permalink raw reply
* Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Johannes Sixt @ 2012-02-27 21:19 UTC (permalink / raw)
To: Jens Lehmann
Cc: Junio C Hamano, Git Mailing List, Antony Male, Phil Hord, msysGit
In-Reply-To: <4F4A8EF2.3020901@web.de>
Am 26.02.2012 20:58, schrieb Jens Lehmann:
> Am 26.02.2012 18:38, schrieb Johannes Sixt:
>> - a=$(cd "$gitdir" && pwd)
>> - b=$(cd "$path" && pwd)
>> + a=$(cd "$gitdir" && pwd -W)
>> + b=$(cd "$path" && pwd -W)
>> while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>> do
>> a=${a#*/} b=${b#*/};
>
> I don't understand why you need this. Does "pwd" sometimes return a
> path starting with "c:/" and sometimes "/c/" depending on what form
> you use when you cd into that directory?
It looks like this is the case. I was surprised as well. I hoped that
pwd -P would fix it, but it makes no difference. I should have tested
pwd -L as well, but I forgot.
> - gitdir=$(git rev-parse --git-dir)
> + gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
I don't like pipelines of this kind because they fork yet another
process. But it looks like there are not that many alternatives...
-- Hannes
^ permalink raw reply
* Re: [PATCH] fsck: do not print dangling objects by default
From: Clemens Buchacher @ 2012-02-27 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vr4xg6pn2.fsf@alter.siamese.dyndns.org>
On Mon, Feb 27, 2012 at 11:29:53AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> Given that, isn't it not just sufficient but actually better to instead
> >> add a new --no-dangling option and keep the default unchanged?
> >
> > ... Of course, it is fsck, so I wonder how often clueless people are
> > really running it in the first place (i.e., it is not and should not be
> > part of most users' typical workflows). If it is simply the case that
> > they are being told to run "git fsck" by more expert users without
> > understanding what it does, then I could buy the argument that those
> > expert users could just as easily say "git fsck --no-dangling".
>
> Yes, that was certainly part of my pros-and-cons analysis. If you run
> "git fsck" without "--no-dangling" without reading the manual, you may
> get confused, but that is *not* the primary audience.
It is not my only concern that users might be confused. I believe the
command prints a lot of useless messages, which is by itself a UI
deficiency. But even worse, those numerous messages tend to hide an
actual problem in a long scrollback buffer. Sometimes my scrollback
buffer is not even large enough and I have to re-run fsck (which is not
exactly a fast command), just so I can grep out the dangling blobs.
> People who are curious can read the manual and figure it out, and the
> need for "fsck" is much rarer these days, compared to 2005 ;-)
In my opinion, the need for fsck is much more common these days. With
the alternates feature, it happens all the time that a repository breaks
if one is not extremely careful.
> In that context, only large downsides of potentially breaking and having
> to adjust existing scripts remains without much upsides, if we were to
> switch the default.
There is something wrong with weighting a UI improvement against
convenient use in scripts. If that were the issue, then we should add a
plumbing version for all commands, like we do for git status
--porcelain. Otherwise we can never change anything any more.
^ permalink raw reply
* Re: git-subtree Ready #2
From: Jeff King @ 2012-02-27 21:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David A. Greene, Avery Pennarun, git
In-Reply-To: <7vobsk56md.fsf@alter.siamese.dyndns.org>
On Mon, Feb 27, 2012 at 01:06:02PM -0800, Junio C Hamano wrote:
> >>> I'm happy to do either (rebase or filter-branch). Just let me know.
> >>
> >> I would understand Avery's "should we filter-branch/rebase, or is it OK
> >> as-is?", but I do not understand what you mean by "either rebase or
> >> filter-branch is fine".
> >
> > Sorry, got mixed up there. I'm not that familiar with filter-branch.
> > Now I understand you do both. :)
> >
> > So have we decided to keep the history?
>
> I think the discussion so far was:
>
> - Peff suggested to keep the history with a true merge;
>
> - I said the history before the final commit in Avery's tree did not look
> so useful for future archaeology; and then
>
> - Avery corrected me that there are contributions by other people and the
> credits will be lost if we discarded the history;
>
> and everybody (including me) now favors to have the history.
>
> So the answer to your question is yes, but I do not think we heard opinion
> from anybody regarding the question by Avery yet. I personally do not see
> how it would help us if the old history is rewritten at this point.
Yeah, I don't see much point in rewriting. If parts of the history suck,
then so be it. It's probably not that big to store. And while it's
sometimes easier to fix bad commit messages when they are recent and in
your memory (rather than trying to remember later what you meant to
say), I think it is already too late for that. Any archaeology you do
now to make good commit messages could probably just as easily be done
if and when somebody actually needs the commit message later (emphasis
on the "if" -- it's likely that nobody will care about most of the
commit messages later at all).
-Peff
^ permalink raw reply
* Re: git-subtree Ready #2
From: Jeff King @ 2012-02-27 21:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David A. Greene, Avery Pennarun, git
In-Reply-To: <20120227212157.GA19779@sigill.intra.peff.net>
On Mon, Feb 27, 2012 at 04:21:57PM -0500, Jeff King wrote:
> > So the answer to your question is yes, but I do not think we heard opinion
> > from anybody regarding the question by Avery yet. I personally do not see
> > how it would help us if the old history is rewritten at this point.
>
> Yeah, I don't see much point in rewriting. If parts of the history suck,
> then so be it. It's probably not that big to store. And while it's
> sometimes easier to fix bad commit messages when they are recent and in
> your memory (rather than trying to remember later what you meant to
> say), I think it is already too late for that. Any archaeology you do
> now to make good commit messages could probably just as easily be done
> if and when somebody actually needs the commit message later (emphasis
> on the "if" -- it's likely that nobody will care about most of the
> commit messages later at all).
Sorry, the "you" there is meant to be David. Forgot who I was responding
to for a minute.
-Peff
^ permalink raw reply
* Re: [PATCH] fsck: do not print dangling objects by default
From: Jeff King @ 2012-02-27 21:33 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Junio C Hamano, git
In-Reply-To: <20120227211316.GA29081@ecki>
On Mon, Feb 27, 2012 at 10:13:16PM +0100, Clemens Buchacher wrote:
> > Yes, that was certainly part of my pros-and-cons analysis. If you run
> > "git fsck" without "--no-dangling" without reading the manual, you may
> > get confused, but that is *not* the primary audience.
>
> It is not my only concern that users might be confused. I believe the
> command prints a lot of useless messages, which is by itself a UI
> deficiency. But even worse, those numerous messages tend to hide an
> actual problem in a long scrollback buffer. Sometimes my scrollback
> buffer is not even large enough and I have to re-run fsck (which is not
> exactly a fast command), just so I can grep out the dangling blobs.
Yeah, but doesn't adding "--no-dangling" solve that issue?
> > People who are curious can read the manual and figure it out, and the
> > need for "fsck" is much rarer these days, compared to 2005 ;-)
>
> In my opinion, the need for fsck is much more common these days. With
> the alternates feature, it happens all the time that a repository breaks
> if one is not extremely careful.
Interesting. I can't remember the last time I ran fsck. But I am not the
only git user, and my assumptions are only based on my own experience,
not other data.
> > In that context, only large downsides of potentially breaking and having
> > to adjust existing scripts remains without much upsides, if we were to
> > switch the default.
>
> There is something wrong with weighting a UI improvement against
> convenient use in scripts. If that were the issue, then we should add a
> plumbing version for all commands, like we do for git status
> --porcelain. Otherwise we can never change anything any more.
I don't think it's convenience as much as scripts which try to detect
dangling by using "git fsck" will now be broken with a really bad
failure mode (i.e., they used to run "git fsck" and parse the dangling
lines on stderr, but now they will erroneously think that nothing is
dangling!).
If we really care about that, then the right answer is our usual:
1. Introduce --dangling/--no-dangling, leaving the default.
2. Wait a while until the features are well-established.
3. Scripts adapt to use --dangling if they want it.
4. Wait a while for scripts to be updated.
5. Flip the default to --no-dangling.
I wrote but didn't send in my last email a long analysis of why it is
wrong to combine fsck's default mode of operation (which is really about
"is this repository corrupted or broken") with checking for dangling
objects (which is something you do when you are looking for lost work).
But I don't think that is even really up for debate. If we were
designing fsck from scratch, I think everybody in the thread has the
feeling that reporting dangling by default is wrong. And we should
definitely add "--no-dangling" to address that.
But flipping the default is a different story, and that has to weigh the
potential breakage above versus having a good UI. I'm on the fence about
whether the above is a big enough problem to care about (mostly because
I'm not sure anybody has actually written such a script).
-Peff
^ permalink raw reply
* Re: [PATCH] fsck: do not print dangling objects by default
From: Junio C Hamano @ 2012-02-27 21:34 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Jeff King, git
In-Reply-To: <20120227211316.GA29081@ecki>
Clemens Buchacher <drizzd@aon.at> writes:
> There is something wrong with weighting a UI improvement against
> convenient use in scripts. If that were the issue, then ...
But that is not the issue. Nobody said anything about convenience for
script writers.
I am talking about regressions to already written long time ago and used
by many people _without_ them having to worry about how it internally
works. By switching the default, you are suddenly making them worry about
it again.
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Johannes Schindelin @ 2012-02-27 21:41 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jens Lehmann, Junio C Hamano, Git Mailing List, Antony Male,
Phil Hord, msysGit
In-Reply-To: <4F4BF357.8020407@kdbg.org>
Hi,
On Mon, 27 Feb 2012, Johannes Sixt wrote:
> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>
> > - gitdir=$(git rev-parse --git-dir)
> > + gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
>
> I don't like pipelines of this kind because they fork yet another
> process. But it looks like there are not that many alternatives...
Not many, but some rather straight-forward ones. E.g.
# quoted to preserve tabs & newslines (Windows users are creative)
gitdir="$(git rev-parse --git-dir)"
case "$gitdir" in
[A-Za-z]:*)
gitdir="/${gitdir%%:*}${gitdir#?:}"
;;
esac
Note: untested.
Note2: I did not add error handling, either.
Note3: yes, ${...} is available in bash and it does not fork. Neither does
case ... esac
Ciao,
Johannes
^ 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