* [PATCH v2] format-patch: autonumber by default
From: Brian Gernhardt @ 2008-10-02 20:55 UTC (permalink / raw)
To: Git List
Cc: Giuseppe Bilotta, Jakub Narebski, Johannes Schindelin,
Shawn O. Pearce, Andreas Ericsson
In-Reply-To: <91634D16-B28A-4458-97A9-C469B5AF4E5D@silverinsanity.com>
format-patch is most commonly used for multiple patches at once when
sending a patchset, in which case we want to number the patches; on
the other hand, single patches are not usually expected to be
numbered.
In other words, the typical behavior expected from format-patch is the
one obtained by enabling autonumber, so we set it to be the default.
Users that want to disable numbering for a particular patchset can do
so with the existing -N command-line switch. Users that want to
change the default behavior can use the format.numbering config key.
Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
_This_ patch changes the default and makes the config variable act
properly. Does not pass tests. 4014 and 4021 would have to be
updated for the new defaults.
Documentation/config.txt | 9 +++++----
Documentation/git-format-patch.txt | 8 +++++---
builtin-log.c | 3 ++-
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index da18a54..5ba3ffa 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -640,10 +640,11 @@ fetch.unpackLimit::
`transfer.unpackLimit` is used instead.
format.numbered::
- A boolean which can enable sequence numbers in patch subjects.
- Setting this option to "auto" will enable it only if there is
- more than one patch. See --numbered option in
- linkgit:git-format-patch[1].
+ A boolean which can enable or disable sequence numbers in patch
+ subjects. It defaults to "auto" which enables it only if there
+ is more than one patch. It can be enabled or disabled for all
+ messages by setting it to "true" or "false". See --numbered
+ option in linkgit:git-format-patch[1].
format.headers::
Additional email headers to include in a patch to be submitted
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index adb4ea7..ac36ce8 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -58,8 +58,10 @@ output, unless the --stdout option is specified.
If -o is specified, output files are created in <dir>. Otherwise
they are created in the current working directory.
-If -n is specified, instead of "[PATCH] Subject", the first line
-is formatted as "[PATCH n/m] Subject".
+By default, the subject of a single patch is "[PATCH] First Line" and
+the subject when multiple patches are output is "[PATCH n/m] First
+Line". To force 1/1 to be added for a single patch, use -n. To omit
+patch numbers from the subject, use -N
If given --thread, 'git-format-patch' will generate In-Reply-To and
References headers to make the second and subsequent patch mails appear
@@ -81,7 +83,7 @@ include::diff-options.txt[]
-n::
--numbered::
- Name output in '[PATCH n/m]' format.
+ Name output in '[PATCH n/m]' format, even with a single patch.
-N::
--no-numbered::
diff --git a/builtin-log.c b/builtin-log.c
index fc5e4da..ee7c34e 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -426,7 +426,7 @@ static int istitlechar(char c)
static const char *fmt_patch_suffix = ".patch";
static int numbered = 0;
-static int auto_number = 0;
+static int auto_number = 1;
static char **extra_hdr;
static int extra_hdr_nr;
@@ -485,6 +485,7 @@ static int git_format_config(const char *var, const char *value, void *cb)
return 0;
}
numbered = git_config_bool(var, value);
+ auto_number = auto_number && numbered;
return 0;
}
--
1.6.0.2.589.gcd70
^ permalink raw reply related
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Petr Baudis @ 2008-10-02 20:56 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <cb7bb73a0810021230u2ec512c0l577b3146cffccb3e@mail.gmail.com>
On Thu, Oct 02, 2008 at 09:30:42PM +0200, Giuseppe Bilotta wrote:
> On Thu, Oct 2, 2008 at 5:34 PM, Petr Baudis <pasky@suse.cz> wrote:
>
> > Just nits...
> >> + }
> >> +
> >> my ($refname, $pathname) = split(/:/, $path_info, 2);
> >> if (defined $pathname) {
> >> - # we got "project.git/branch:filename" or "project.git/branch:dir/"
> >> + # we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
> >> # we could use git_get_type(branch:pathname), but it needs $git_dir
> >> $pathname =~ s,^/+,,;
> >> if (!$pathname || substr($pathname, -1) eq "/") {
> >
> > But the old variant is still possible, maybe the comments should
> > indicate that the action/ part is optional.
>
> I put the action/ part in square brackets. (e.g.
> project.git/[action/]branch:filename). Is this good enough?
That's perfect.
> >> @@ -534,8 +575,9 @@ sub evaluate_path_info {
> >> $file_name ||= validate_pathname($pathname);
> >> } elsif (defined $refname) {
> >> # we got "project.git/branch"
> >> - $action ||= "shortlog";
> >> - $hash ||= validate_refname($refname);
> >> + $action ||= "shortlog";
> >> + $hash ||= validate_refname($refname);
> >> + $hash_base ||= validate_refname($refname);
> >> }
> >> }
> >> evaluate_path_info();
> >
> > What is this good for?
>
> The purpose of what? setting both $hash and $hash_base was something
> that I found was needed in some extreme cases, as discussed with
> Jakub. Proposals for recommended cleaner but equally fast way to
> handle it. If you're referring to the whitespace, I was just lining up
> the entries. Should I do it in a separate patch?
I refer to the setting of $hash_base (I'm not huge fan of the whitespace
aligning, but I don't really care). What extreme cases are these?
I think you should describe that in the code since it's not really
obvious. Maybe I could find it in older threads but I should understand
the code just from reading it. :-)
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-02 21:05 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, Jakub Narebski, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <20081002205603.GW10360@machine.or.cz>
>> >> @@ -534,8 +575,9 @@ sub evaluate_path_info {
>> >> $file_name ||= validate_pathname($pathname);
>> >> } elsif (defined $refname) {
>> >> # we got "project.git/branch"
>> >> - $action ||= "shortlog";
>> >> - $hash ||= validate_refname($refname);
>> >> + $action ||= "shortlog";
>> >> + $hash ||= validate_refname($refname);
>> >> + $hash_base ||= validate_refname($refname);
>> >> }
>> >> }
>> >> evaluate_path_info();
>> >
>> > What is this good for?
>>
>> The purpose of what? setting both $hash and $hash_base was something
>> that I found was needed in some extreme cases, as discussed with
>> Jakub. Proposals for recommended cleaner but equally fast way to
>> handle it. If you're referring to the whitespace, I was just lining up
>> the entries. Should I do it in a separate patch?
>
> I refer to the setting of $hash_base (I'm not huge fan of the whitespace
> aligning, but I don't really care). What extreme cases are these?
> I think you should describe that in the code since it's not really
> obvious. Maybe I could find it in older threads but I should understand
> the code just from reading it. :-)
In preparing the new patchset, I've put a big comment block explaining
why we need to set both $hash and $hash_base in this code-path:
# we got "project.git/[action/]branch". in this case
# we set both $hash and $hash_base because different actions
# need one or the other to be set to behave correctly.
#
# For example, if $hash_base is not set then the blob and
# history links on the page project.git/tree/somebranch will
# assume a $hash_base of HEAD instead of the correct
# somebranch.
# Conversely, not setting $hash will make URLs such as
# project.git/shortlog/somebranch display the wrong page.
#
# Since it also turns out that the unused one is properly
# overwritten as needed, setting both is quite safe.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] format-patch: autonumber by default
From: Jeff King @ 2008-10-02 21:06 UTC (permalink / raw)
To: Giuseppe Bilotta
Cc: git, Jakub Narebski, Johannes Schindelin, Shawn O. Pearce
In-Reply-To: <1222978500-5780-1-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, Oct 02, 2008 at 10:15:00PM +0200, Giuseppe Bilotta wrote:
> Documentation/config.txt | 5 +++--
> Documentation/git-format-patch.txt | 9 ++++++---
> builtin-log.c | 6 +++++-
> 3 files changed, 14 insertions(+), 6 deletions(-)
Thanks, the documentation updates look good to me. There are also some
updates required in t4013 (since the expected outputs for some instances
will now be numbered) and t4020 (which explicitly checks that the
default is no numbering). Probably t4020 should be modified in light of
the new default, like so:
---
t/t4021-format-patch-numbered.sh | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
index 43d64bb..390af23 100755
--- a/t/t4021-format-patch-numbered.sh
+++ b/t/t4021-format-patch-numbered.sh
@@ -45,17 +45,22 @@ test_numbered() {
grep "^Subject: \[PATCH 2/2\]" $1
}
-test_expect_success 'Default: no numbered' '
+test_expect_success 'single patch defaults to no numbers' '
+ git format-patch --stdout HEAD~1 >patch0.single &&
+ test_single_no_numbered patch0.single
+'
+
+test_expect_success 'multiple patch defaults to numbered' '
- git format-patch --stdout HEAD~2 >patch0 &&
- test_no_numbered patch0
+ git format-patch --stdout HEAD~2 >patch0.multiple &&
+ test_numbered patch0.multiple
'
test_expect_success 'Use --numbered' '
- git format-patch --numbered --stdout HEAD~2 >patch1 &&
- test_numbered patch1
+ git format-patch --numbered --stdout HEAD~1 >patch1 &&
+ test_single_numbered patch1
'
--
1.6.0.2.570.g2c958
^ permalink raw reply related
* Re: [PATCH] git commit: Repaint the output format bikeshed (again)
From: Jeff King @ 2008-10-02 21:13 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Shawn Pearce, Git Mailing List
In-Reply-To: <48E45ECC.8070104@op5.se>
On Thu, Oct 02, 2008 at 07:40:28AM +0200, Andreas Ericsson wrote:
> No sign-off.
Sorry, mistakenly omitted.
Signed-off-by: Jeff King <peff@peff.net>
>> + printf("[%s%s]: created ",
>> + !prefixcmp(head, "refs/heads/") ?
>> + head + 11 :
>> + !strcmp(head, "HEAD") ?
>> + "detached HEAD" :
>> + head,
>> + initial_commit ? " (root-commit)" : "");
>>
>
> Personally, I'm not overly fond of things like
> something ? yay : nay_but_try ? worked_now : still_no_go
> since I find them hard to read without thinking a lot.
Hmm, I find them more readable. :) And often easier to visually see that
no matter what happens, the result has _some_ value (whereas with
if/else, you have to make sure that all branchs set the value). But I
am happy to change it to:
const char *branch;
...
if (!prefixcmp(head, "refs/heads/"))
branch = head + 11;
else if (!strcmp(head, "HEAD"))
branch = "detached HEAD";
else
branch = head;
However, I found your mail somewhat unexpected. Rather than comments on
the code, I expected rather "yes, I do like this better" or "no, I think
we should go with the other one." But maybe you are just sick of
weighing in. ;)
-Peff
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Petr Baudis @ 2008-10-02 22:04 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <cb7bb73a0810021405j68b0a164i9469e64afc543ebf@mail.gmail.com>
On Thu, Oct 02, 2008 at 11:05:18PM +0200, Giuseppe Bilotta wrote:
> In preparing the new patchset, I've put a big comment block explaining
> why we need to set both $hash and $hash_base in this code-path:
>
> # we got "project.git/[action/]branch". in this case
> # we set both $hash and $hash_base because different actions
> # need one or the other to be set to behave correctly.
> #
> # For example, if $hash_base is not set then the blob and
> # history links on the page project.git/tree/somebranch will
> # assume a $hash_base of HEAD instead of the correct
> # somebranch.
> # Conversely, not setting $hash will make URLs such as
> # project.git/shortlog/somebranch display the wrong page.
> #
> # Since it also turns out that the unused one is properly
> # overwritten as needed, setting both is quite safe.
Ok, but is this related to the pathinfo changes? Or is this fixing a
separate bug? (Not that I would want to bother you with splitting this,
but you should at least mention it in the commit message... and it's
fairly isolated anyway.)
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Jakub Narebski @ 2008-10-02 22:41 UTC (permalink / raw)
To: Petr Baudis; +Cc: Giuseppe Bilotta, git, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <20081002220439.GX10360@machine.or.cz>
Petr Baudis wrote:
> On Thu, Oct 02, 2008 at 11:05:18PM +0200, Giuseppe Bilotta wrote:
> >
> > In preparing the new patchset, I've put a big comment block explaining
> > why we need to set both $hash and $hash_base in this code-path:
> >
> > # we got "project.git/[action/]branch". in this case
> > # we set both $hash and $hash_base because different actions
> > # need one or the other to be set to behave correctly.
> > #
> > # For example, if $hash_base is not set then the blob and
'blob' without $file_name doesn't have sense, but 'tree' does.
Probably should be s/blob/tree/ above.
> > # history links on the page project.git/tree/somebranch will
> > # assume a $hash_base of HEAD instead of the correct
> > # somebranch.
> > # Conversely, not setting $hash will make URLs such as
> > # project.git/shortlog/somebranch display the wrong page.
> > #
> > # Since it also turns out that the unused one is properly
> > # overwritten as needed, setting both is quite safe.
>
> Ok, but is this related to the pathinfo changes? Or is this fixing a
> separate bug? (Not that I would want to bother you with splitting this,
> but you should at least mention it in the commit message... and it's
> fairly isolated anyway.)
Yes, it related to path_info changes. With current code the only way
to get to no $file_name branch of evaluate_path_info() code was to have
URL which looks like project/branch, for which 'shortlog' action was
chosen (if not specified by CGI query parameters), and for 'shortlog'
action the correct way was to use $hash. Now there can be
project/tree/branch to show top directory of given branch, and this
as described required $hash_base set.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [JGIT PATCH 4/5] Expose the critical receive configuration options to JGit
From: Shawn O. Pearce @ 2008-10-02 22:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810012254.09169.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
>
> The repo configuration setup fails. I'll squash this in
Whoops. Thanks.
> I also noted we try to read ~/.gitconfig which may give us som headaches
> later on.
Oy. Issue #42 on the issue tracker for it. We shouldn't be reading
that during the tests.
--
Shawn.
^ permalink raw reply
* [PATCH] git-stash.sh: fix flawed fix of invalid ref handling (commit da65e7c1)
From: Brandon Casey @ 2008-10-02 23:52 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Git Mailing List
In-Reply-To: <DDpCKmlWUqX7hHbNNF45UGo81L7NuUHMbU3vXSfhHU60An5PUqIvRA@cipher.nrlssc.navy.mil>
The referenced commit tried to fix a flaw in stash's handling of a user
supplied invalid ref. i.e. 'git stash apply fake_ref@{0}' should fail
instead of applying stash@{0}. But, it did so in a naive way by avoiding the
use of the --default option of rev-parse, and instead manually supplied the
default revision if the user supplied an empty command line. This prevented
a common usage scenario of supplying flags on the stash command line (i.e.
non-empty command line) which would be parsed by lower level git commands,
without supplying a specific revision. This should fall back to the default
revision, but now it causes an error. e.g. 'git stash show -p'
The correct fix is to use the --verify option of rev-parse, which fails
properly if an invalid ref is supplied, and still allows falling back to a
default ref when one is not supplied.
Convert stash-drop to use --verify while we're at it, since specifying
multiple revisions for any of these commands is also an error and --verify
makes it so.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
The patch is simpler than it looks, since it includes a revert of my
original patch.
Here is 'git diff da65e7c1^:git-stash.sh git-stash.sh':
@@ -144,17 +144,16 @@ show_stash () {
then
flags=--stat
fi
- s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
- w_commit=$(git rev-parse --verify "$s") &&
- b_commit=$(git rev-parse --verify "$s^") &&
+ w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
+ b_commit=$(git rev-parse --verify "$w_commit^") &&
git diff $flags $b_commit $w_commit
}
@@ -169,7 +168,7 @@ apply_stash () {
# stash records the work tree, and is a merge between the
# base commit (first parent) and the index tree (second parent).
- s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+ s=$(git rev-parse --verify --default $ref_stash "$@") &&
w_tree=$(git rev-parse --verify "$s:") &&
b_tree=$(git rev-parse --verify "$s^1:") &&
i_tree=$(git rev-parse --verify "$s^2:") ||
@@ -229,7 +228,7 @@ drop_stash () {
shift
fi
# Verify supplied argument looks like a stash entry
- s=$(git rev-parse --revs-only --no-flags "$@") &&
+ s=$(git rev-parse --verify "$@") &&
git rev-parse --verify "$s:" > /dev/null 2>&1 &&
git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
I already messed this up once, so a review is appreciated. Any reason
--verify wasn't used in the first place?
I know why it wasn't used in drop_stash(), it's because I just copied it
from apply_stash().
-brandon
git-stash.sh | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 42f626f..b9ace99 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -145,16 +145,8 @@ show_stash () {
flags=--stat
fi
- if test $# = 0
- then
- set x "$ref_stash@{0}"
- shift
- fi
-
- s=$(git rev-parse --revs-only --no-flags "$@")
-
- w_commit=$(git rev-parse --verify "$s") &&
- b_commit=$(git rev-parse --verify "$s^") &&
+ w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
+ b_commit=$(git rev-parse --verify "$w_commit^") &&
git diff $flags $b_commit $w_commit
}
@@ -170,19 +162,13 @@ apply_stash () {
shift
esac
- if test $# = 0
- then
- set x "$ref_stash@{0}"
- shift
- fi
-
# current index state
c_tree=$(git write-tree) ||
die 'Cannot apply a stash in the middle of a merge'
# stash records the work tree, and is a merge between the
# base commit (first parent) and the index tree (second parent).
- s=$(git rev-parse --revs-only --no-flags "$@") &&
+ s=$(git rev-parse --verify --default $ref_stash "$@") &&
w_tree=$(git rev-parse --verify "$s:") &&
b_tree=$(git rev-parse --verify "$s^1:") &&
i_tree=$(git rev-parse --verify "$s^2:") ||
@@ -242,7 +228,7 @@ drop_stash () {
shift
fi
# Verify supplied argument looks like a stash entry
- s=$(git rev-parse --revs-only --no-flags "$@") &&
+ s=$(git rev-parse --verify "$@") &&
git rev-parse --verify "$s:" > /dev/null 2>&1 &&
git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
--
1.6.0.2.323.g7c850
^ permalink raw reply related
* [PATCH] rebase -i: proper prepare-commit-msg hook argument when squashing
From: SZEDER Gábor @ 2008-10-03 0:08 UTC (permalink / raw)
To: git; +Cc: SZEDER Gábor
One would expect that the prepare-commit-msg hook gets 'squash' as the
second argument when squashing commits with 'rebase -i'. However,
that was not the case, as it got 'merge' instead. This patch fixes
the problem.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
git-rebase--interactive.sh | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index edb6ec6..ec4299a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -304,23 +304,28 @@ do_next () {
mark_action_done
make_squash_message $sha1 > "$MSG"
+ failed=f
+ author_script=$(get_author_ident_from_commit HEAD)
+ output git reset --soft HEAD^
+ pick_one -n $sha1 || failed=t
case "$(peek_next_command)" in
squash|s)
EDIT_COMMIT=
USE_OUTPUT=output
+ MSG_OPT=-F
+ MSG_FILE="$MSG"
cp "$MSG" "$SQUASH_MSG"
;;
*)
EDIT_COMMIT=-e
USE_OUTPUT=
+ MSG_OPT=
+ MSG_FILE=
rm -f "$SQUASH_MSG" || exit
+ cp -v "$MSG" "$GIT_DIR"/SQUASH_MSG
+ rm -f "$GIT_DIR"/MERGE_MSG || exit
;;
esac
-
- failed=f
- author_script=$(get_author_ident_from_commit HEAD)
- output git reset --soft HEAD^
- pick_one -n $sha1 || failed=t
echo "$author_script" > "$DOTEST"/author-script
if test $failed = f
then
@@ -329,7 +334,7 @@ do_next () {
GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
- $USE_OUTPUT git commit --no-verify -F "$MSG" $EDIT_COMMIT || failed=t
+ $USE_OUTPUT git commit --no-verify $MSG_OPT "$MSG_FILE" $EDIT_COMMIT || failed=t
fi
if test $failed = t
then
--
1.6.0.2.430.gfc53
^ permalink raw reply related
* git rebase -- a suggestion
From: Robin Burchell @ 2008-10-03 0:10 UTC (permalink / raw)
To: git
Hi,
This is my first mail to this list, so I hope I'm not breaking any
form of ettiquette, etc. If I do step on any toes, feel free to bop me
on the head with a rubber mallet, or steer me in the right direction.
That over, I have a suggestion for `git rebase', from the perspective
of a newcomer.
I've been using git instead of svn (and various other VCS) now for
about a month, and am finding it quite a refreshing change.
I have also recently started a collaborative project exclusively with
git (well, pulling changes from a git-svn repo I don't control) which
has been a valuable ..learning experience.
With this in mind, I'd like to mention exactly what I did.
Upstream had issued a new commit, so I, not knowing the possible
dangers used git-svn rebase to pull in the new changes to our tree.
This "appeared" to work fine, but alarm bells were already going off
in my head before I typed the command (I didn't know at the time I
could merge svn trees like I could normal git branches) as I knew that
rebase rewrote history, and I saw it do this to about 300 commits.
It promptly made merging absolute hell with the other few members of
my team, as it would.
Granted - this is a mistake on my part, and probably a common newbie
one, but something that came to mind when thinking about it later:
would it perhaps be an idea to have a way to mark a tree 'public', and
disallow rebase *unless* --force was passed, or it was a public tree?
(Then again, the alternative might be more 'intelligent' for new
users: start off with branches defaulting to private, and marking them
public, disallowing use of rebase, etc).
Thoughts, feedback, etc are welcome.
--
Robin Burchell
^ permalink raw reply
* Re: [PATCH] git commit: Repaint the output format bikeshed (again)
From: Shawn O. Pearce @ 2008-10-03 0:15 UTC (permalink / raw)
To: Jeff King; +Cc: Andreas Ericsson, Git Mailing List
In-Reply-To: <20081002211309.GB29480@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Thu, Oct 02, 2008 at 07:40:28AM +0200, Andreas Ericsson wrote:
> > [... many many many many many things about paints ...]
I think painting is over for now. Time to let the paint dry.
I applied Jeff's patch:
[jk/bikeshed] created bd8098f: "reformat informational commit message"
--
Shawn.
^ permalink raw reply
* Re: [PATCH] gitweb: Identify all summary metadata table rows
From: Shawn O. Pearce @ 2008-10-03 0:30 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, Petr Baudis
In-Reply-To: <1222957505-5150-1-git-send-email-pasky@suse.cz>
Petr Baudis <pasky@suse.cz> wrote:
> In the metadata table of the summary page, all rows have their
> id (or class in case of URL) set now. This for example lets sites
> easily disable fields they do not want to show in their custom
> stylesheet (e.g. they are overly technical or irrelevant for the site).
>
> Many of my other patches depend on this, so I would appreciate to hear
> as soon as possible if someone has an issue with this patch.
Its pretty trivial and painless. So its applied. Marking up unique
elements so you can control them via CSS isn't rocket science. ;-)
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index da474d0..bd8124a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4070,10 +4070,10 @@ sub git_summary {
>
> print "<div class=\"title\"> </div>\n";
> print "<table class=\"projects_list\">\n" .
> - "<tr><td>description</td><td>" . esc_html($descr) . "</td></tr>\n" .
> - "<tr><td>owner</td><td>" . esc_html($owner) . "</td></tr>\n";
> + "<tr id=\"metadata_desc\"><td>description</td><td>" . esc_html($descr) . "</td></tr>\n" .
> + "<tr id=\"metadata_owner\"><td>owner</td><td>" . esc_html($owner) . "</td></tr>\n";
> if (defined $cd{'rfc2822'}) {
> - print "<tr><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n";
> + print "<tr id=\"metadata_lchange\"><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n";
> }
>
> # use per project git URL list in $projectroot/$project/cloneurl
> @@ -4083,7 +4083,7 @@ sub git_summary {
> @url_list = map { "$_/$project" } @git_base_url_list unless @url_list;
> foreach my $git_url (@url_list) {
> next unless $git_url;
> - print "<tr><td>$url_tag</td><td>$git_url</td></tr>\n";
> + print "<tr class=\"metadata_url\"><td>$url_tag</td><td>$git_url</td></tr>\n";
> $url_tag = "";
> }
> print "</table>\n";
--
Shawn.
^ permalink raw reply
* Re: [PATCH] gitweb: Identify all summary metadata table rows
From: Jakub Narebski @ 2008-10-03 1:04 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Petr Baudis, git, Petr Baudis
In-Reply-To: <20081003003053.GT21310@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Petr Baudis <pasky@suse.cz> wrote:
> > In the metadata table of the summary page, all rows have their
> > id (or class in case of URL) set now. This for example lets sites
> > easily disable fields they do not want to show in their custom
> > stylesheet (e.g. they are overly technical or irrelevant for the site).
> >
> > Many of my other patches depend on this, so I would appreciate to hear
> > as soon as possible if someone has an issue with this patch.
>
> Its pretty trivial and painless. So its applied. Marking up unique
> elements so you can control them via CSS isn't rocket science. ;-)
Somehow original patch didn't appear (yet) on git mailing list;
perhaps it ran afoul vger anti-SPAM filter...
> > + "<tr id=\"metadata_desc\"><td>description</td><td>" . esc_html($descr) . "</td></tr>\n" .
> > + "<tr id=\"metadata_owner\"><td>owner</td><td>" . esc_html($owner) . "</td></tr>\n";
> > + print "<tr id=\"metadata_lchange\"><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n";
> > + print "<tr class=\"metadata_url\"><td>$url_tag</td><td>$git_url</td></tr>\n";
I like the idea, and I think this can be seen as beginning to use
either microformats, or RDFa. You have marked those fragments for CSS
to show or hide; microformats are about marking those fragments for
machine to parse. Although microformats usually use 'class' (or 'rel'
for links), not 'id' attribute for classifying data...
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Jakub Narebski @ 2008-10-03 0:48 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <cb7bb73a0810020243v37759f79xfde4c32c49e1a375@mail.gmail.com>
On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
> On Thu, Oct 2, 2008 at 10:59 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Giuseppe Bilotta wrote:
>>> - $action ||= "shortlog";
>>> - $hash ||= validate_refname($refname);
>>> + $action ||= "shortlog";
>>> + $hash ||= validate_refname($refname);
>>> + $hash_base ||= validate_refname($refname);
>>> }
>>> }
>>
>> This hunk is IMHO incorrect. First, $refname is _either_ $hash, or
>> $hash_base; it cannot be both. Second, in most cases (like the case
>> of 'shortlog' action, either explicit or implicit) it is simply $hash;
>> I think it can be $hash_base when $file_name is not set only in
>> singular exception case of 'tree' view for the top tree (lack of
>> filename is not an error, but is equivalent to $file_name='/').
>
> OTOH, while setting both $hash and $hash_base has worked fine for me
> so far (because the right one is automatically used and apparently
> setting the other doesn't hurt), choosing which one to set is a much
> hairier case. Do you have suggestions for a better way to always make
> it work?
Well, it is either checking $action and setting either $hash or
$hash_base, or setting both, with some comments on why and when it is
needed (as discussed on #git). IIUC $hash_base is needed only for
filename-taking tree actions which acts on top-tree, and therefore
don't need $file_name, like 'project/tree/branch' or related
'project/history/branch' (the latter is practically almost equivalent
to 'project/shortlog/branch' or 'project/branch').
I'm not sure if it wouldn't be better to call validate_refname($refname)
once, either as:
$hash_base ||= $hash ||= validate_refname($refname);
but that might be incorrect in the obscure case of setting $hash via 'h'
CGI query parameter, and letting gitweb to set-up $hash_base via
path_info, so perhaps ($refname is local to evaluate_path_info, IIRC)
$refname = validate_refname($refname);
$hash ||= $refname;
$hash_base ||= $refname;
But that is just nitpicking this fragment of code to death. In short:
either check which of $hash and $hash_base to set in this branch of
conditional, or explain why setting both $hash and $hash_base is needed,
and why it is acceptable, either as comments, or in commit message.
>>> @@ -631,8 +642,15 @@ sub href (%) {
>>> if ($params{-replay}) {
>>> while (my ($name, $symbol) = each %mapping) {
>>> if (!exists $params{$name}) {
>>> - # to allow for multivalued params we use arrayref form
>>> - $params{$name} = [ $cgi->param($symbol) ];
>>> + # the parameter we want to recycle may be either part of the
>>> + # list of CGI parameter, or recovered from PATH_INFO
>>> + if ($cgi->param($symbol)) {
>>> + # to allow for multivalued params we use arrayref form
>>> + $params{$name} = [ $cgi->param($symbol) ];
>>> + } else {
>>> + no strict 'refs';
>>> + $params{$name} = $$name if $$name;
>>
>> I would _perhaps_ add here comment that multivalued parameters can come
>> only from CGI query string, so there is no need for something like:
>>
>> + $params{$name} = (ref($$name) ? @$name : $$name) if $$name;
>>
>>> + }
>>> }
>>> }
>>> }
>>
>> This fragment is a bit of ugly code, hopefully corrected in later patch.
>> I think it would be better to have 'refactor parsing/validation of input
>> parameters' to be very fist patch in series; I am not sure but I suspect
>> that is a kind of bugfix for current "$project/$hash" ('shortlog' view)
>> and "$project/$hash_base:$file_name" ('blob_plain' and 'tree' view)
>> path_info.
>
> But implementing the path_info parsing first makes the input param
> refactoring SO much nicer that I would rather put a comment here
> saying "this code sucks: we should rather collect all input
> parameters" and then clean it up on the subsequent patch.
Why not cleanup first?
When implementing href(..., -replay=>1) I have forgot that some of
gitweb parameters are implicitly passed ($project, because it is needed
in most gitweb links), and some can be passed via path_info ($hash
and/or $hash_base, $file_name). Your code adds $action to the mix, but
it doesn't change the fact that 1.) even before your code -replay case
was incorrect for some path_info links (handcrafted, as gitweb generates
only $project via path_info); 2.) code you have added is a bit ugly.
Besides using variables change a little meaning of -replay, namely
in your code gitweb always sets action, even for non-path_name links
when we started from "default action" (i.e. without action set) links.
I guess this is mainly theoretical issue, as I don't think that default
views use many -replay links.
P.S. with the idea of pushing parameters obtained not from CGI query
string to $cgi->param() via "$cgi->param($name, $value);" or in named
params form "$cgi->(-name=>$name, -value=>$value);" you would not need
to change (a bit hacky, admittedly) href(...,-replay=>1) code.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] gitweb: refactor input parameters parse/validation
From: Jakub Narebski @ 2008-10-03 1:36 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1222906234-8182-3-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
> Since input parameters can now be obtained both from CGI parameters and
> PATH_INFO, we would like most of the code to be agnostic about the way
> parameters were retrieved.
>
I'd prefer that this cleanup/refactoring patch was _first_ patch in
the series, as we were able to obtain parameters both from CGI query
parameters and from PATH_INFO ($project, $hash or $hash_base+$file_name)
_before_ first patch in this series. So it correct not only issue
introduced by first patch (and fixed somewhat there), but what was
outstanding (but rare because gitweb did not generate such links)
issue.
> We thus collect all the parameters into the new %input_params hash,
> removing the need for ad-hoc kludgy code in href().
Alternate solution would be push data from PATH_INFO in query params
data using (for example)
$cgi->param('a', $action);
or, naming parameters explicitely
$cgi->param(-name=>'a', -value=>$action);
This avoids need for additional variable, reuses current code, and
nicely sidesteps issue whether to use long names for keys ('action',
'file_name') or short ones from CGI query ('a', 'f').
It would probably has to be at least partially written to check which
of those two solutions (%input_params or $cgi->param('a', $a)))
is better.
> As much of the
> parameters validation code is now shared between CGI and PATH_INFO,
> although this requires PATH_INFO parsing to be interleaved into the main
> code instead of being factored out into its own routine.
I'm not sure if it is worth it then to unify parameters validation,
with such drawback.
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> gitweb/gitweb.perl | 336 ++++++++++++++++++++++++++++------------------------
> 1 files changed, 183 insertions(+), 153 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f088681..ec4326f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -28,8 +28,10 @@ our $my_url = $cgi->url();
> our $my_uri = $cgi->url(-absolute => 1);
>
> # if we're called with PATH_INFO, we have to strip that
> -# from the URL to find our real URL
> -if (my $path_info = $ENV{"PATH_INFO"}) {
> +# from the URL to find our real URL. PATH_INFO is kept
> +# as it's used later on for parameter extraction
> +my $path_info = $ENV{"PATH_INFO"};
> +if ($path_info) {
> $my_url =~ s,\Q$path_info\E$,,;
> $my_uri =~ s,\Q$path_info\E$,,;
> }
This might be separate patch, if you wanted to increase your commit
count ;-) More seriously I think it should be at least briefly
mentioned in commit message (make $path_info global).
> @@ -390,15 +392,111 @@ $projects_list ||= $projectroot;
>
> # ======================================================================
> # input validation and dispatch
> -our $action = $cgi->param('a');
> -if (defined $action) {
> - if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> - die_error(400, "Invalid action parameter");
> +
> +# input parameters can be collected from a variety of sources (presently, CGI
> +# and PATH_INFO), so we define an %input_params hash that collects them all
> +# together during validation: this allows subsequent uses (e.g. href()) to be
> +# agnostic of the parameter origin
> +
> +my %input_params = ();
> +
> +# input parameters are stored with the long parameter name as key, so we need
> +# the name -> CGI key mapping here. This will also be used in the href
> +# subroutine to convert parameters to their CGI equivalent.
> +#
> +# XXX: Warning: If you touch this, check the search form for updating,
> +# too.
> +
> +my @cgi_param_mapping = (
> + project => "p",
> + action => "a",
> + file_name => "f",
> + file_parent => "fp",
> + hash => "h",
> + hash_parent => "hp",
> + hash_base => "hb",
> + hash_parent_base => "hpb",
> + page => "pg",
> + order => "o",
> + searchtext => "s",
> + searchtype => "st",
> + snapshot_format => "sf",
> + extra_options => "opt",
> + search_use_regexp => "sr",
> +);
> +my %cgi_param_mapping = @cgi_param_mapping;
> +
> +# we will also need to know the possible actions, for validation
> +my %actions = (
> + "blame" => \&git_blame,
> + "blobdiff" => \&git_blobdiff,
> + "blobdiff_plain" => \&git_blobdiff_plain,
> + "blob" => \&git_blob,
> + "blob_plain" => \&git_blob_plain,
> + "commitdiff" => \&git_commitdiff,
> + "commitdiff_plain" => \&git_commitdiff_plain,
> + "commit" => \&git_commit,
> + "forks" => \&git_forks,
> + "heads" => \&git_heads,
> + "history" => \&git_history,
> + "log" => \&git_log,
> + "rss" => \&git_rss,
> + "atom" => \&git_atom,
> + "search" => \&git_search,
> + "search_help" => \&git_search_help,
> + "shortlog" => \&git_shortlog,
> + "summary" => \&git_summary,
> + "tag" => \&git_tag,
> + "tags" => \&git_tags,
> + "tree" => \&git_tree,
> + "snapshot" => \&git_snapshot,
> + "object" => \&git_object,
> + # those below don't need $project
> + "opml" => \&git_opml,
> + "project_list" => \&git_project_list,
> + "project_index" => \&git_project_index,
> +);
Nice, although I'm not sure if [%@]cgi_param_mapping has to global.
If we use long parameters names as keys, I think it has to, somewhat.
See also comment below.
> +
> +# fill %input_params with the CGI parameters. All values except for 'opt'
> +# should be single values, but opt can be an array. We should probably
> +# build an array of parameters that can be multi-valued, but since for the time
> +# being it's only this one, we just single it out
> +while (my ($name, $symbol) = each %cgi_param_mapping) {
> + if ($symbol eq 'opt') {
> + $input_params{$name} = [$cgi->param($symbol)];
> + } else {
> + $input_params{$name} = $cgi->param($symbol);
> }
> }
If it was chosen to use short (CGI query) parameter names, the above
loop could be replaced by simple
%input_params = $cgi->Vars;
or to be more exact, if we want to have multi-valued parameters stored
as array ref
%input_params = map { [ split /\0/ ] if /\0/; } $cgi->Vars;
See CGI(3pm):
When using this [Vars], the thing you must watch out for are multivalued CGI
parameters. Because a hash cannot distinguish between scalar and list con-
text, multivalued parameters will be returned as a packed string, separated
by the "\0" (null) character. You must split this packed string in order
to get at the individual values.
> -# parameters which are pathnames
> -our $project = $cgi->param('p');
> +# next, we want to parse PATH_INFO (which was already stored in $path_info at
> +# the beginning). This is a little hairy because PATH_INFO parsing needs some
> +# form of parameter validation, so we interleave parsing and validation.
I don't think it is a good idea. In my opinion, for my taste, it would
be better to separate evaluating path_info from the rest.
We could instead introduce convention that if variable (like $project)
is set, then it is assumed to be validated; if it is present only in
the %input_params hash, then it has to be validated.
On the other hand this ordering, first by parameter, then by method
of extraction could be seem quite equally valid. Nevertheless I think
that previous flow with separate evaluate_path_info() and what should
be evaluate_CGI_query() has better encapsulation.
> +#
> +# The accepted PATH_INFO syntax is $project/$action/$hash or
> +# $project/$action/$hash_base:$file_name, where $action may be missing (mostly for
> +# backwards compatibility), so we need to parse and validate the parameters in
> +# this same order.
> +
> +# clear $path_info of any leading /
> +$path_info =~ s,^/+,,;
> +
> +our $project = $input_params{'project'};
> +if ($path_info && !defined $project) {
> + # if $project was not defined by CGI, we try to extract it from
> + # $path_info
> + $project = $path_info;
> + $project =~ s,/+$,,;
> + while ($project && !check_head_link("$projectroot/$project")) {
> + $project =~ s,/*[^/]*$,,;
> + }
> + $input_params{'project'} = $project;
> +} else {
> + # otherwise, we suppress $path_info parsing altogether
> + $path_info = undef;
> +}
> +
> +# project name validation
> if (defined $project) {
> if (!validate_pathname($project) ||
> !(-d "$projectroot/$project") ||
Note that this code does less checking if $project is in path_info than
for the case where it is set by CGI query. Perhaps there should be base
fast check in a loop, and more extensive test later.
> @@ -408,16 +506,66 @@ if (defined $project) {
> undef $project;
> die_error(404, "No such project");
> }
> +
> + # we purge the $project name from the $path_info, preparing it for
> + # subsequent parameters extraction
> + $path_info =~ s,^\Q$project\E/*,, if defined $path_info;
> +} else {
> + # we also suppress $path_info parsing if no project was defined
> + $path_info = undef;
> +}
In evaluate_path_info it was simply 'return if...'; here with mentioned
interleaving it is harder and a bit hacky.
[cut]
> @@ -615,43 +676,12 @@ sub href (%) {
> # default is to use -absolute url() i.e. $my_uri
> my $href = $params{-full} ? $my_url : $my_uri;
[cut removed, or rather moved to beginning, @mapping]
> $params{'project'} = $project unless exists $params{'project'};
>
> if ($params{-replay}) {
> - while (my ($name, $symbol) = each %mapping) {
> - if (!exists $params{$name}) {
> - # the parameter we want to recycle may be either part of the
> - # list of CGI parameter, or recovered from PATH_INFO
> - if ($cgi->param($symbol)) {
> - # to allow for multivalued params we use arrayref form
> - $params{$name} = [ $cgi->param($symbol) ];
> - } else {
> - no strict 'refs';
> - $params{$name} = $$name if $$name;
> - }
> - }
> + while (my ($name, $val) = each %input_params) {
> + $params{$name} = $input_params{$name}
> + unless (exists $params{$name});
Very nice, short code. Should be something like that from
the very beginning.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] gitweb: generate project/action/hash URLs
From: Jakub Narebski @ 2008-10-03 1:48 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1222906234-8182-4-git-send-email-giuseppe.bilotta@gmail.com>
On Tue, 2 Oct 2008, Giuseppe Bilotta wrote:
> When generating path info URLs, reduce the number of CGI parameters by
> embedding action and hash_parent:filename or hash in the path.
_Perhaps_ it should be noted that even though gitweb accepted
'project/hash' and 'project/hash_base:file_name' path_info URLs, it
generated links with only 'project/' in path_info.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> gitweb/gitweb.perl | 32 +++++++++++++++++++++++++++++---
> 1 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index ec4326f..2c380ac 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -687,14 +687,40 @@ sub href (%) {
>
> my ($use_pathinfo) = gitweb_check_feature('pathinfo');
> if ($use_pathinfo) {
> - # use PATH_INFO for project name
> + # try to put as many parameters as possible in PATH_INFO:
> + # - project name
> + # - action
> + # - hash or hash_base:filename
> +
> + # Strip any trailing / from $href, or we might get double
> + # slashes when the script is the DirectoryIndex
Perhaps example, like $href='gitweb.example.com/', could be put here.
> + #
I think that we can lose this empty line comment here.
> + $href =~ s,/$,,;
> +
> + # Then add the project name, if present
> $href .= "/".esc_url($params{'project'}) if defined $params{'project'};
> delete $params{'project'};
>
> - # Summary just uses the project path URL
> - if (defined $params{'action'} && $params{'action'} eq 'summary') {
> + # Summary just uses the project path URL, any other action is
> + # added to the URL
> + if (defined $params{'action'}) {
> + $href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
> delete $params{'action'};
> }
Good.
> +
> + # Finally, we put either hash_base:file_name or hash
> + if (defined $params{'hash_base'}) {
> + $href .= "/".esc_url($params{'hash_base'});
> + if (defined $params{'file_name'}) {
> + $href .= ":".esc_url($params{'file_name'});
> + delete $params{'file_name'};
> + }
> + delete $params{'hash'};
> + delete $params{'hash_base'};
> + } elsif (defined $params{'hash'}) {
> + $href .= "/".esc_url($params{'hash'});
> + delete $params{'hash'};
> + }
Hmmmm...
Shouldn't the code first check for $file_name, then add either
"$hash_base:$file_name" (url-escaped), or "$hash" (not "$hash_base")?
> }
>
> # now encode the parameters explicitly
Thank you very much for work on improving gitweb.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] builtin-commit: avoid always using reduce_heads()
From: Shawn O. Pearce @ 2008-10-03 2:35 UTC (permalink / raw)
To: Miklos Vajna; +Cc: SZEDER Gabor, jnareb, Johannes.Schindelin, git
In-Reply-To: <1222457868-9864-1-git-send-email-vmiklos@frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> wrote:
> diff --git a/builtin-commit.c b/builtin-commit.c
> index 55e1087..f546cf7 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -1040,6 +1050,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>
> unlink(git_path("MERGE_HEAD"));
> unlink(git_path("MERGE_MSG"));
> + unlink(git_path("MERGE_MODE"));
> unlink(git_path("SQUASH_MSG"));
>
> if (commit_index_files())
Hmmph. Should branch.c and builtin-reset.c clean this new file
up too?
> diff --git a/builtin-merge.c b/builtin-merge.c
> index 5c65a58..20102a0 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -1210,6 +1210,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> merge_msg.len)
> die("Could not write to %s", git_path("MERGE_MSG"));
> close(fd);
> + fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT, 0666);
> + if (fd < 0)
> + die("Could open %s for writing", git_path("MERGE_MODE"));
> + strbuf_reset(&buf);
> + if (!allow_fast_forward)
> + strbuf_addf(&buf, "no-ff");
> + if (write_in_full(fd, buf.buf, buf.len) != buf.len)
Shouldn't we open this file with O_TRUNC to avoid this scenario:
$ git merge --no-ff --no-commit foo
$ git reset --hard
$ git merge --no-commit foo
... *sigh* MERGE_MODE still has "no-ff" in it ...
This is especially true since some porcelain (e.g. git-gui) just
deletes MERGE_HEAD right now and doesn't know about cleaning up
MERGE_MODE. We'd want to at least reset it correctly on the next
invocation to git-merge.
--
Shawn.
^ permalink raw reply
* alias g to git in .gitconfig?
From: Rob Sanheim @ 2008-10-03 2:51 UTC (permalink / raw)
To: git
This is pretty trivial, but I'm a lazy typist.
Is it possible to alias 'g' to git via git config, instead of via
bash? If I do a plain bash alias then none of the nice autocompletion
from git-contrib work with 'g'.
- Rob
^ permalink raw reply
* Re: alias g to git in .gitconfig?
From: Shawn O. Pearce @ 2008-10-03 3:10 UTC (permalink / raw)
To: Rob Sanheim; +Cc: git
In-Reply-To: <fc113d400810021951hf95ff35qb1ccb4af45a71abe@mail.gmail.com>
Rob Sanheim <rsanheim@gmail.com> wrote:
> This is pretty trivial, but I'm a lazy typist.
>
> Is it possible to alias 'g' to git via git config, instead of via
> bash? If I do a plain bash alias then none of the nice autocompletion
> from git-contrib work with 'g'.
No, you'll need to alias 'g' to git in bash, and then if you still
want completion you'll need to also register the compgen to call
_git completion routine. Its two lines:
alias g=git
complete -o default -o nospace -F _git g
--
Shawn.
^ permalink raw reply
* [PATCH] Use "git_config_string" to simplify "remote.c" code in "handle_config"
From: David Bryson @ 2008-10-03 3:39 UTC (permalink / raw)
To: git
Signed-off-by: David Bryson <david@statichacks.org>
I tried to keep with the naming/coding conventions that I found in
remote.c. Feedback welcome.
---
remote.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/remote.c b/remote.c
index 3f3c789..893a739 100644
--- a/remote.c
+++ b/remote.c
@@ -305,6 +305,7 @@ static int handle_config(const char *key, const char *value, void *cb)
{
const char *name;
const char *subkey;
+ const char *v;
struct remote *remote;
struct branch *branch;
if (!prefixcmp(key, "branch.")) {
@@ -314,15 +315,15 @@ static int handle_config(const char *key, const char *value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, ".remote")) {
- if (!value)
- return config_error_nonbool(key);
- branch->remote_name = xstrdup(value);
+ if (git_config_string(&v, key, value) )
+ return -1;
+ branch->remote_name = v;
if (branch == current_branch)
default_remote_name = branch->remote_name;
} else if (!strcmp(subkey, ".merge")) {
- if (!value)
- return config_error_nonbool(key);
- add_merge(branch, xstrdup(value));
+ if (git_config_string(&v, key, value ))
+ return -1;
+ add_merge(branch, v);
}
return 0;
}
@@ -334,9 +335,9 @@ static int handle_config(const char *key, const char *value, void *cb)
return 0;
rewrite = make_rewrite(name, subkey - name);
if (!strcmp(subkey, ".insteadof")) {
- if (!value)
- return config_error_nonbool(key);
- add_instead_of(rewrite, xstrdup(value));
+ if (git_config_string(&v, key, value ))
+ return -1;
+ add_instead_of(rewrite, v);
}
}
if (prefixcmp(key, "remote."))
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH] git commit: Repaint the output format bikeshed (again)
From: Jeff King @ 2008-10-03 4:24 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Andreas Ericsson, Git Mailing List
In-Reply-To: <20081003001556.GS21310@spearce.org>
On Thu, Oct 02, 2008 at 05:15:56PM -0700, Shawn O. Pearce wrote:
> I think painting is over for now. Time to let the paint dry.
> I applied Jeff's patch:
>
> [jk/bikeshed] created bd8098f: "reformat informational commit message"
Woo! Victory by attrition!
-Peff
^ permalink raw reply
* Re: [PATCH] Use "git_config_string" to simplify "remote.c" code in "handle_config"
From: Andreas Ericsson @ 2008-10-03 5:28 UTC (permalink / raw)
To: David Bryson; +Cc: git
In-Reply-To: <20081003033937.GA11594@eratosthenes.cryptobackpack.org>
David Bryson wrote:
> Signed-off-by: David Bryson <david@statichacks.org>
>
> I tried to keep with the naming/coding conventions that I found in
> remote.c. Feedback welcome.
>
> ---
> remote.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 3f3c789..893a739 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -305,6 +305,7 @@ static int handle_config(const char *key, const char *value, void *cb)
> {
> const char *name;
> const char *subkey;
> + const char *v;
Not very mnemonic. I'm sure you can think up a better name, even if it's
a long one. Git is notoriously sparse when it comes to comments. We rely
instead on self-explanatory code.
> struct remote *remote;
> struct branch *branch;
> if (!prefixcmp(key, "branch.")) {
> @@ -314,15 +315,15 @@ static int handle_config(const char *key, const char *value, void *cb)
> return 0;
> branch = make_branch(name, subkey - name);
> if (!strcmp(subkey, ".remote")) {
> - if (!value)
> - return config_error_nonbool(key);
> - branch->remote_name = xstrdup(value);
> + if (git_config_string(&v, key, value) )
> + return -1;
> + branch->remote_name = v;
> if (branch == current_branch)
> default_remote_name = branch->remote_name;
> } else if (!strcmp(subkey, ".merge")) {
> - if (!value)
> - return config_error_nonbool(key);
> - add_merge(branch, xstrdup(value));
> + if (git_config_string(&v, key, value ))
> + return -1;
> + add_merge(branch, v);
> }
> return 0;
> }
> @@ -334,9 +335,9 @@ static int handle_config(const char *key, const char *value, void *cb)
> return 0;
> rewrite = make_rewrite(name, subkey - name);
> if (!strcmp(subkey, ".insteadof")) {
> - if (!value)
> - return config_error_nonbool(key);
> - add_instead_of(rewrite, xstrdup(value));
> + if (git_config_string(&v, key, value ))
> + return -1;
> + add_instead_of(rewrite, v);
> }
> }
> if (prefixcmp(key, "remote."))
Other than that, the patch looks good.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-03 5:54 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Petr Baudis, git, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <200810030041.54563.jnareb@gmail.com>
On Fri, Oct 3, 2008 at 12:41 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Petr Baudis wrote:
>> On Thu, Oct 02, 2008 at 11:05:18PM +0200, Giuseppe Bilotta wrote:
>> >
>> > In preparing the new patchset, I've put a big comment block explaining
>> > why we need to set both $hash and $hash_base in this code-path:
>> >
>> > # we got "project.git/[action/]branch". in this case
>> > # we set both $hash and $hash_base because different actions
>> > # need one or the other to be set to behave correctly.
>> > #
>> > # For example, if $hash_base is not set then the blob and
>
> 'blob' without $file_name doesn't have sense, but 'tree' does.
> Probably should be s/blob/tree/ above.
>
>> > # history links on the page project.git/tree/somebranch will
>> > # assume a $hash_base of HEAD instead of the correct
>> > # somebranch.
Note: the blob and history links _in a tree page_ are wrong. The page
itself is correct.
What this means is that if you go to project.git/tree/somebranch you
get the correct list of files and directories for the project at
somebranch's HEAD, but all the links IN that page will point to the
wrong version, because they will have no $hash_base set and will thus
default to HEAD.
You can see this effect for example by going to
http://repo.or.cz/w/git.git/v1.4.0?a=tree <- this hand-crafted path
would never be generated by the old pathinfo code, but it show exactly
what would happen with my pahtinfo code on
http://repo.or.cz/w/git.git/tree/v1.4.0 if $hash_base wasn't set.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] rebase -i: proper prepare-commit-msg hook argument when squashing
From: Jeff King @ 2008-10-03 6:01 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Shawn O. Pearce, git
In-Reply-To: <1222992501-943-1-git-send-email-szeder@ira.uka.de>
On Fri, Oct 03, 2008 at 02:08:21AM +0200, SZEDER Gábor wrote:
> + cp -v "$MSG" "$GIT_DIR"/SQUASH_MSG
Sorry, but "cp -v" is not portable. It's not in POSIX, and this breaks
the script for (at least) Solaris.
However, it's not even clear to me why "-v" is used at all, considering
that the "squash" case above does not use it. Is it a debugging
leftover? Am I missing something?
[Aside: My Solaris 8 autobuilder is now running, which was a huge pain.
However, it is very satisfying to catch things like this in "next"
before they hit a wider audience.]
-Peff
^ 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