* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Peter Oberndorfer @ 2011-10-11 21:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipnvfk70.fsf@alter.siamese.dyndns.org>
On Dienstag, 11. Oktober 2011, Junio C Hamano wrote:
> Peter Oberndorfer <kumbayo84@arcor.de> writes:
>
> > Using $GIT_EDITOR or core.editor config var for this is not possible
> > since it is also used to start the commit message editor for reword action.
>
> Your tool _could_ be smart about this issue and inspect the contents to
> launch a real editor when it is fed a material not for sequencing, but
> that feels hacky.
I already tried this, but my first version did not redirect stdin/stdout
so vi stayed in background and the whole thing just hung.
I did not try further because i assumed more problems would appear
when redirecting stdin/stdout...
> > * GIT_EDITOR env var is not honored anymore after this change.
>
> Care to explain? "git var" knows magic about a few variables like
> GIT_EDITOR and GIT_PAGER.
>
> $ git config core.editor vim
> $ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
> vi
> $ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
> emacs
Sorry i was wrong, i missed that git var looks at $GIT_EDITOR.
So the sequence for choosing the sequencer editor is:
$GIT_SEQUENCE_EDITOR
config sequence.editor
var GIT_EDITOR
Which looks OK to me.
> > * Should git_rebase_editor be in git-rebase--interactive.sh instead
>
> Probably yes.
OK, will do.
>
> > * How should the config be called?
>
> Given that in the longer term we would be using a unified sequencer
> machinery for not just rebase-i but for am and cherry-pick, I would advise
> against calling this anything "rebase". How does "sequence.edit" sound?
>
I do not really care very much, but how about sequence.editor?
Sounds more similar to core.editor
> You need to be prepared to adjust your code to deal with new kinds of
> sequencing insns in the insn sheet and possibly a format change of the
> insn sheet itself.
I assume instruction sheet is the commented out part that looks like:
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
Currently all lines starting with # are ignored.
(They are also not written to the output when finished
which is a point I might have to change...)
Also the instructions are currently not taken from this instruction sheet.
They are all hardcoded.
Thanks for the feedback
Greetings Peter
^ permalink raw reply
* Re: [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Phil Hord @ 2011-10-11 20:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vy5wre0n7.fsf@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 4:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
>> return 0;
>> if (!S_ISREG(st.st_mode))
>> return 0;
>> - if (st.st_size < 10 || st.st_size > PATH_MAX)
>> - return 1;
>> + if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
>> + return 0;
>
> We are asked if the file is likely to be a single-liner "gitfile: <path>",
> and were answering yes when it is a very short file (less than 10 bytes)
> that cannot possibly even contain "gitfile: " prefix.
>
> I suspect that we can and should get rid of the "cannot be very long"
> check altogether---we do open and check the file, and after all it is not
> like we are throwing different strings as "url" argument to this function
> at random and this function needs heuristics to reject bogus input
> early. The argument is an input from the user.
>
> Quite an embarrasing bug. Thanks for spotting.
Yes, and it's _my_ embarrassing bug. I've caught this and have it in
a re-roll, but I got mired up and haven't submitted it again. I'll
try to do so today.
Phil
^ permalink raw reply
* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-11 20:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <7v39ezffq5.fsf_-_@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 01:14:26PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> I think we've discussed tightening it a few years ago already.
> >>
> >> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
> >> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
> >
> > Perhaps like this? Only compile tested...
>
> Not quite. There are at least three bugs in the patch.
>
> - Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would
> not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed;
Yeah, I found one of these also. I thought at first we could rename
things like NOTES_MERGE_PARTIAL, as it's more about internal
communication within a specific version of git than it is about letting
an external program peek at our state. But there really are several of
them. And I think it makes sense to keep this safety valve conservative,
and try to document existing practice rather than dictating it. I'm
worried that some porcelain or other tool is using their own all-caps
name, and that tightening it too much would be breaking that.
Your relaxed rule does match things like COMMIT_EDITMSG and NOTES_MSG,
which are obviously bogus. At the same time, I'm not sure it's a big
deal. The point of this is to restrict the lookup to a class of names
which are likely magical to git, and users should probably avoid the
magical namespace (i.e., it's still not a good idea to call your branch
"HEAD"). Something like "config" is an easy branch name to unknowingly
use. Something like "COMMIT_EDITMSG" is not.
Your rule does disallow RENAMED-REF, which is used in branch renaming.
However, I'm having trouble figuring out what it's really for. It's not
mentioned in the documentation at all, and c976d41, its origin, says
only:
This also indroduces $GIT_DIR/RENAME_REF which is a "metabranch"
used when renaming branches. It will always hold the original
sha1 for the latest renamed branch.
Additionally, if $GIT_DIR/logs/RENAME_REF exists,
all branch rename events are logged there.
But in the code, it is spelled RENAMED-REF (with a dash). And as far as
I can tell, does not actually create a reflog. And it's not documented
anywhere, so I suspect nobody is using it. Maybe it is worth switching
that name.
> - dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
> the master branch.
It looks like your code will allow any subdirectory. I had thought to
limit it to "refs/". Otherwise, my "config" example could be
"objects/pack", or "lost-found/commits", "remotes/foo", or something.
Obviously the longer the name, the smaller the possibility of an
accidental collision. But I couldn't think of any other subdirectory
into which refs should go.
-Peff
^ permalink raw reply
* Re: [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Junio C Hamano @ 2011-10-11 20:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.1110111424010.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The logic to check whether a file is a gitfile used the heuristics that
> the file cannot be larger than PATH_MAX. But in that case it returned the
> wrong value. Our test cases do not cover this, as the bundle files
> produced are smaller than PATH_MAX. Except on Windows.
>
> While at it, fix the faulty logic that the path stored in a gitfile cannot
> be larger than PATH_MAX-sizeof("gitfile: ").
>
> Problem identified by running the test suite in msysGit, offending commit
> identified by Jörg Rosenkranz.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> This patch should apply cleanly to 'next', which we track in
> msysgit/git.
>
> The task of adding a test case is something I leave to someone who
> wants to get involved with Git development and needs an easy way
> in.
>
> transport.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index f3195c0..57138d9 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
> return 0;
> if (!S_ISREG(st.st_mode))
> return 0;
> - if (st.st_size < 10 || st.st_size > PATH_MAX)
> - return 1;
> + if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
> + return 0;
We are asked if the file is likely to be a single-liner "gitfile: <path>",
and were answering yes when it is a very short file (less than 10 bytes)
that cannot possibly even contain "gitfile: " prefix.
I suspect that we can and should get rid of the "cannot be very long"
check altogether---we do open and check the file, and after all it is not
like we are throwing different strings as "url" argument to this function
at random and this function needs heuristics to reject bogus input
early. The argument is an input from the user.
Quite an embarrasing bug. Thanks for spotting.
^ permalink raw reply
* Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 20:14 UTC (permalink / raw)
To: Jeff King, Michael Haggerty
Cc: git, cmn, A Large Angry SCM, Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <7vmxd7flkw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> I think we've discussed tightening it a few years ago already.
>>
>> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
>> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
>
> Perhaps like this? Only compile tested...
Not quite. There are at least three bugs in the patch.
- Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would
not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed;
- dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
the master branch.
- These codepaths get pointer+length so that it can be told to parse only
the first 4 bytes in "HEAD:$path".
-- >8 --
Subject: [PATCH] Restrict ref-like names immediately below $GIT_DIR
We have always dwimmed the user input $string into a ref by first looking
directly inside $GIT_DIR, and then in $GIT_DIR/refs, $GIT_DIR/refs/tags,
etc., and that is what made
git log HEAD..MERGE_HEAD
work correctly. This however means that
git rev-parse config
git log index
would look at $GIT_DIR/config and $GIT_DIR/index and see if they are valid
refs.
To reduce confusion, let's not dwim a path immediately below $GIT_DIR that
is not all-caps.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
sha1_name.c | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 143fd97..5eb19c2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -261,6 +261,25 @@ static char *substitute_branch_name(const char **string, int *len)
return NULL;
}
+static int ok_at_root_level(const char *str, int len)
+{
+ int seen_non_root_char = 0;
+
+ while (len--) {
+ char ch = *str++;
+
+ if (ch == '/')
+ return 1;
+ /*
+ * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+ * the root level as a ref.
+ */
+ if (ch != '_' && (ch < 'A' || 'Z' < ch))
+ seen_non_root_char = 1;
+ }
+ return !seen_non_root_char;
+}
+
int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
{
char *last_branch = substitute_branch_name(&str, &len);
@@ -274,6 +293,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
unsigned char *this_result;
int flag;
+ if (p == ref_rev_parse_rules && !ok_at_root_level(str, len))
+ continue;
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
r = resolve_ref(fullref, this_result, 1, &flag);
@@ -302,6 +323,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
char path[PATH_MAX];
const char *ref, *it;
+ if (p == ref_rev_parse_rules && !ok_at_root_level(str, len))
+ continue;
mksnpath(path, sizeof(path), *p, len, str);
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
^ permalink raw reply related
* Re: Symmetric of rebase / more intelligent cherry-pick
From: Junio C Hamano @ 2011-10-11 19:36 UTC (permalink / raw)
To: Lionel Elie Mamane; +Cc: git
In-Reply-To: <20111011155444.GB14417@capsaicin.mamane.lu>
Lionel Elie Mamane <lionel@mamane.lu> writes:
> git cherry-pick ..UPSTREAM
> *nearly* does what I want, except that it lacks rebase's intelligence
> of skipping commits that do the same textual changes as a commit
> already in the current branch.
I think in the longer term "--ignore-if-in-upstream" that is known only to
format-patch, which is the true source the intelligence of rebase you
observed comes from, should be factored out into a helper function that
can be used to filter output from get_revision() in other commands, or
perhaps get_revision() itself might want to learn it.
I say "or perhaps might" above, because I do not think the general
revision traversal machinery used by the log family (which cherry-pick's
multi-pick option relies on) has enough information to decide what the
caller means by "upstream" at the point setup_revisions() is called.
^ permalink raw reply
* [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Johannes Schindelin @ 2011-10-11 19:25 UTC (permalink / raw)
To: git, gitster
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1276 bytes --]
The logic to check whether a file is a gitfile used the heuristics that
the file cannot be larger than PATH_MAX. But in that case it returned the
wrong value. Our test cases do not cover this, as the bundle files
produced are smaller than PATH_MAX. Except on Windows.
While at it, fix the faulty logic that the path stored in a gitfile cannot
be larger than PATH_MAX-sizeof("gitfile: ").
Problem identified by running the test suite in msysGit, offending commit
identified by Jörg Rosenkranz.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
This patch should apply cleanly to 'next', which we track in
msysgit/git.
The task of adding a test case is something I leave to someone who
wants to get involved with Git development and needs an easy way
in.
transport.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index f3195c0..57138d9 100644
--- a/transport.c
+++ b/transport.c
@@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
return 0;
if (!S_ISREG(st.st_mode))
return 0;
- if (st.st_size < 10 || st.st_size > PATH_MAX)
- return 1;
+ if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
+ return 0;
fd = open(url, O_RDONLY);
if (fd < 0)
--
1.7.6.msysgit.0.584.g2cbf
^ permalink raw reply related
* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Junio C Hamano @ 2011-10-11 18:37 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git
In-Reply-To: <201110111956.08829.kumbayo84@arcor.de>
Peter Oberndorfer <kumbayo84@arcor.de> writes:
> Using $GIT_EDITOR or core.editor config var for this is not possible
> since it is also used to start the commit message editor for reword action.
Your tool _could_ be smart about this issue and inspect the contents to
launch a real editor when it is fed a material not for sequencing, but
that feels hacky.
> * GIT_EDITOR env var is not honored anymore after this change.
Care to explain? "git var" knows magic about a few variables like
GIT_EDITOR and GIT_PAGER.
$ git config core.editor vim
$ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
vi
$ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
emacs
> * Should git_rebase_editor be in git-rebase--interactive.sh instead
Probably yes.
> * How should the config be called?
Given that in the longer term we would be using a unified sequencer
machinery for not just rebase-i but for am and cherry-pick, I would advise
against calling this anything "rebase". How does "sequence.edit" sound?
You need to be prepared to adjust your code to deal with new kinds of
sequencing insns in the insn sheet and possibly a format change of the
insn sheet itself.
^ permalink raw reply
* [RESEND PATCH v3] Configurable hyperlinking in gitk
From: Jeff Epler @ 2011-10-11 18:37 UTC (permalink / raw)
To: git, Marc Branchaud, Chris Packham, Jakub Narebski,
Junio C Hamano, Paul
In-Reply-To: <20110922013101.GB26880@unpythonic.net>
Many projects use project-specific notations in changelogs to refer
to bug trackers and the like. One example is the "Closes: #12345"
notation used in Debian.
Make gitk configurable so that arbitrary strings can be turned into
clickable links that are opened in a web browser.
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
This v3 patch didn't generate any discussion last time around (~3 weeks
ago), so I've taken the liberty of reposting it.
I'm aware of no problems with this patch, and a number of people have
commented that it is useful to them. For URLs that contain "&" and
other shell metacharacters, it *does* depend on r480f062c
"git-web--browse: avoid the use of eval" which is in next but not maint.
Since the V2 patch, I
* Renamed configuration variables to get rid of the "gitk" prefix
to encourage other git-related programs to adopt the same
functionality.
* Renamed configuration variables from cryptic "re", "sub" to less
cryptic "regexp" and "subst"
* Changed the example RE to be an ERE (no \d or \M)
* Documented that these are POSIX EREs; hopefully that's OK. I see
in CodingGuidelines that in git itself "a subset of BREs" are used,
so maybe even this is too much power. And hopefully tcl's
re_syntax really is close enough to an ERE superset that this isn't
a terrible lie about the initial implementation either.
* Added a Signed-Off-By, since I've had a number of positive feedbacks
and the only problems I've heard of (since patch v2) are the ones
related to 'eval' in git-web--browse.
In v2 of the patch, I had fixed a problem with %-signs in URLs and
changed the documentation example.
Documentation/config.txt | 30 +++++++++++++++++-
gitk-git/gitk | 75 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae9913b..ffc9ccf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1064,6 +1064,10 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and
is one of "ext" and "pserver") to make them apply only for the given
access method.
+gitk.browser::
+ Specify the browser that will be used to open links generated by
+ 'linkify' configuration options.
+
grep.lineNumber::
If set to true, enable '-n' option by default.
@@ -1317,6 +1321,28 @@ interactive.singlekey::
setting is silently ignored if portable keystroke input
is not available.
+linkify.<name>.regexp::
+ Specify a regular expression in the POSIX Extended Regular Expression
+ syntax defining a class of strings to automatically convert to
+ hyperlinks. This regular expression many not span multiple lines.
+ You must also specify 'linkify.<name>.subst'.
+
+linkify.<name>.subst::
+ Specify a substitution that results in the target URL for the
+ related regular expression. Back-references like '\1' refer
+ to capturing groups in the associated regular expression.
+ You must also specify 'linkify.<name>.regexp'.
++
+For example, to automatically link from Debian-style "Closes: #nnnn"
+message to the Debian BTS,
++
+--------
+ git config linkify.debian-bts.regexp '#([1-9][0-9]*)'
+ git config linkify.debian-bts.subst 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\1'
+--------
++
+Currently, only linkgit:gitk[1] converts strings to links in this fashion.
+
log.abbrevCommit::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
linkgit:git-whatchanged[1] assume `\--abbrev-commit`. You may
@@ -1870,5 +1896,5 @@ user.signingkey::
web.browser::
Specify a web browser that may be used by some commands.
- Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
- may use it.
+ Currently only linkgit:git-instaweb[1], linkgit:gitk[1],
+ and linkgit:git-help[1] may use it.
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..9db5525 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {
# append some text to the ctext widget, and make any SHA1 ID
# that we know about be a clickable link.
proc appendwithlinks {text tags} {
- global ctext linknum curview
+ global ctext linknum curview linkmakers
set start [$ctext index "end - 1c"]
$ctext insert end $text $tags
@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {
setlink $linkid link$linknum
incr linknum
}
+
+ if {$linkmakers == {}} return
+
+ set link_re {}
+ foreach {re rep} $linkmakers { lappend link_re $re }
+ set link_re "([join $link_re {)|(}])"
+
+ set ee 0
+ while {[regexp -indices -start $ee -- $link_re $text l]} {
+ set s [lindex $l 0]
+ set e [lindex $l 1]
+ set linktext [string range $text $s $e]
+ incr e
+ set ee $e
+
+ foreach {re rep} $linkmakers {
+ if {![regsub $re $linktext $rep linkurl]} continue
+ $ctext tag delete link$linknum
+ $ctext tag add link$linknum "$start + $s c" "$start + $e c"
+ seturllink $linkurl link$linknum
+ incr linknum
+ break
+ }
+ }
}
proc setlink {id lk} {
@@ -6726,6 +6750,53 @@ proc setlink {id lk} {
}
}
+proc get_link_config {} {
+ if {[catch {exec git config -z --get-regexp {^linkify\.}} linkers]} {
+ return {}
+ }
+
+ set linktypes [list]
+ foreach item [split $linkers "\0"] {
+ if {$item == ""} continue
+ if {![regexp {linkify\.(\S+)\.(regexp|subst)\s(.*)} $item _ k t v]} {
+ continue
+ }
+ set linkconfig($t,$k) $v
+ if {$t == "regexp"} { lappend linktypes $k }
+ }
+
+ set linkmakers [list]
+ foreach k $linktypes {
+ if {![info exists linkconfig(subst,$k)]} {
+ puts stderr "Warning: link `$k' is missing a substitution string"
+ } elseif {[catch {regexp -inline -- $linkconfig(regexp,$k) ""} err]} {
+ puts stderr "Warning: link `$k': $err"
+ } else {
+ lappend linkmakers $linkconfig(regexp,$k) $linkconfig(subst,$k)
+ }
+ unset linkconfig(regexp,$k)
+ unset -nocomplain linkconfig(subst,$k)
+ }
+ foreach k [array names linkconfig] {
+ regexp "subst,(.*)" $k _ k
+ puts stderr "Warning: link `$k' is missing a regular expression"
+ }
+ set linkmakers
+}
+
+proc openlink {url} {
+ exec git web--browse --config=gitk.browser $url &
+}
+
+proc seturllink {url lk} {
+ set qurl [string map {% %%} $url]
+ global ctext
+ $ctext tag conf $lk -foreground blue -underline 1
+ $ctext tag bind $lk <1> [list openlink $qurl]
+ $ctext tag bind $lk <Enter> {linkcursor %W 1}
+ $ctext tag bind $lk <Leave> {linkcursor %W -1}
+}
+
proc appendshortlink {id {pre {}} {post {}}} {
global ctext linknum
@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq "win32"} {
focus -force .
}
+set linkmakers [get_link_config]
+
getcommits {}
# Local variables:
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Jakub Narebski @ 2011-10-11 18:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <7vvcrvfmg4.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Tue, 11 Oct 2011, Junio C Hamano wrote:
>>
>>> I probably do not have time to look into this, but just FYI my trial merge
>>> to 'pu' of this topic is failing like this:
>>>
>>> asciidoc: ERROR: gitweb.conf.txt: line 484: illegal style name: Default: ()
>>> asciidoc: ERROR: gitweb.conf.txt: line 494: illegal style name: Default: 300
>>
>> Damn, I thought I have fixed that. This probably depends on AsciiDoc
>> version ("make doc" on 'master' generates a few _warnings_ for me related
>> to similar situation), but the problem is with
>>
>> [Default: <value>]
>>
>> that was copied from gitweb/README. But [<sth>] is an attribute list
>> (style name in simplest form), used more often in newer AsciiDoc.
>>
>> So either we have to escape '[' and ']', i.e. use {startsb} and {endsb},
>> which would reduce human-friendliness, or move to different way of marking
>> default values, e.g. _italic_.
>>
>> What do you think?
>
> What do the other documents in the directory this file lives say? I think
> we explain what the variables does, and add "defaults to false" or
> somesuch in the text, without any funny mark-up.
O.K., will do.
Now that reminds me that in a few situations gitweb.conf.txt uses literary
description "defaults to sth"... I'll make the rest consistent with this.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Phil Hord @ 2011-10-11 18:15 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git
In-Reply-To: <201110111956.08829.kumbayo84@arcor.de>
On Tue, Oct 11, 2011 at 1:56 PM, Peter Oberndorfer <kumbayo84@arcor.de> wrote:
> i wrote a (not yet released) git rebase -i helper that allows to order commits
> by drag/drop and allows to select the action from a combo box.
> (written in Qt)
> See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
> No more typos, no more lost commit by cutting without pasting...
[+1]
>
> To integrate this properly into git i need something like this patch.
>
> Open questions/problems:
> * GIT_EDITOR env var is not honored anymore after this change.
> Help from somebody with more bash knowledge is highly appreciated!
>
> * Should git_rebase_editor be in git-rebase--interactive.sh instead
> (since it is only used there)
>
> * How should the config be called?
> It is not directly used during rebase, only during rebase -i
> that might not be fully clear from the config name.
>
> * Better config.txt description?
>
> Thanks,
> Greetings Peter
>
> PS: My tool will hopefully be released soon.
> Cleanup code, test(lin/ win), write some doc (how to use with git),
> choose name :-), choose license...
>
> Documentation/config.txt | 6 ++++++
> git-rebase--interactive.sh | 2 +-
> git-sh-setup.sh | 13 +++++++++++++
> 3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 03296b7..1d9ae79 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1591,6 +1591,12 @@ rebase.stat::
> Whether to show a diffstat of what changed upstream since the last
> rebase. False by default.
>
> +rebase.editor::
> + Text editor used by git rebase -i for editing the rebasse todo file.
s/rebasse/rebase/
> cp "$todo" "$todo".backup
> -git_editor "$todo" ||
> +git_rebase_editor "$todo" ||
> die_abort "Could not execute editor"
Maybe something like this would work:
git_rebase_editor "$todo" ||
git_editor "$todo" ||
die_abort "Could not execute editor"
If git_rebase_editor call returns an error (non-zero exit code), then
git_editor will be invoked. If that also returns an error, then the
die_abort is called.
I think this will allow your env:GIT_EDITOR to work as expected.
Phil
^ permalink raw reply
* Re: [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 18:07 UTC (permalink / raw)
To: Jeff King, Michael Haggerty
Cc: git, cmn, A Large Angry SCM, Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <7vr52jfm8i.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> I wonder if the right solution is for us to be more picky about what can
>> be found in $GIT_DIR. Maybe matching all-uppercase, or starting with
>> "refs/", which I think would match existing convention?
>
> I think we've discussed tightening it a few years ago already.
>
> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
Perhaps like this? Only compile tested...
sha1_name.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index ba976b4..5effb1a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -261,6 +261,23 @@ static char *substitute_branch_name(const char **string, int *len)
return NULL;
}
+static int is_kind_of_head(const char *str)
+{
+ int len = strlen(str) - 4;
+ if (len < 0 || strcmp(str + len, "HEAD"))
+ return 0;
+ if (!len)
+ return 1;
+ if (str[--len] != '_' || !len)
+ return 0;
+ while (len) {
+ char ch = str[--len];
+ if (ch < 'A' || 'Z' < ch)
+ return 0;
+ }
+ return 1;
+}
+
int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
{
char *last_branch = substitute_branch_name(&str, &len);
@@ -274,6 +291,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
unsigned char *this_result;
int flag;
+ if (p == ref_rev_parse_rules && !is_kind_of_head(str))
+ continue;
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
r = resolve_ref(fullref, this_result, 1, &flag);
@@ -302,6 +321,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
char path[PATH_MAX];
const char *ref, *it;
+ if (p == ref_rev_parse_rules && !is_kind_of_head(str))
+ continue;
mksnpath(path, sizeof(path), *p, len, str);
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
^ permalink raw reply related
* [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Peter Oberndorfer @ 2011-10-11 17:56 UTC (permalink / raw)
To: git
If rebase.editor is not set interactive rebase falls back
to the default editor.
With this change is it possible to have a separate
(possibly graphical) editor that helps the user
during a interactive rebase.
Using $GIT_EDITOR or core.editor config var for this is not possible
since it is also used to start the commit message editor for reword action.
Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---
Hi,
i wrote a (not yet released) git rebase -i helper that allows to order commits
by drag/drop and allows to select the action from a combo box.
(written in Qt)
See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
No more typos, no more lost commit by cutting without pasting...
To integrate this properly into git i need something like this patch.
Open questions/problems:
* GIT_EDITOR env var is not honored anymore after this change.
Help from somebody with more bash knowledge is highly appreciated!
* Should git_rebase_editor be in git-rebase--interactive.sh instead
(since it is only used there)
* How should the config be called?
It is not directly used during rebase, only during rebase -i
that might not be fully clear from the config name.
* Better config.txt description?
Thanks,
Greetings Peter
PS: My tool will hopefully be released soon.
Cleanup code, test(lin/ win), write some doc (how to use with git),
choose name :-), choose license...
Documentation/config.txt | 6 ++++++
git-rebase--interactive.sh | 2 +-
git-sh-setup.sh | 13 +++++++++++++
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 03296b7..1d9ae79 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1591,6 +1591,12 @@ rebase.stat::
Whether to show a diffstat of what changed upstream since the last
rebase. False by default.
+rebase.editor::
+ Text editor used by git rebase -i for editing the rebasse todo file.
+ The value is meant to be interpreted by the shell when it is used.
+ When not configured the default commit message editor is used instead.
+ See "core.editor"
+
rebase.autosquash::
If set to true enable '--autosquash' option by default.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94f36c2..0f3b569 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -832,7 +832,7 @@ has_action "$todo" ||
die_abort "Nothing to do"
cp "$todo" "$todo".backup
-git_editor "$todo" ||
+git_rebase_editor "$todo" ||
die_abort "Could not execute editor"
has_action "$todo" ||
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 8e427da..303fb96 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -113,6 +113,19 @@ git_editor() {
eval "$GIT_EDITOR" '"$@"'
}
+git_rebase_editor() {
+ if test -z "${GIT_REBASEI_EDITOR:+set}"
+ then
+ GIT_REBASEI_EDITOR="$(git config rebase.editor)"
+ if [ -z "$GIT_REBASEI_EDITOR" ]
+ then
+ GIT_REBASEI_EDITOR="$(git var GIT_EDITOR)" || return $?
+ fi
+ fi
+
+ eval "$GIT_REBASEI_EDITOR" '"$@"'
+}
+
git_pager() {
if test -t 1
then
--
1.7.7.215.gfef80
^ permalink raw reply related
* Re: [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 17:53 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <20111011161652.GA15629@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I wonder if the right solution is for us to be more picky about what can
> be found in $GIT_DIR. Maybe matching all-uppercase, or starting with
> "refs/", which I think would match existing convention?
I think we've discussed tightening it a few years ago already.
HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
^ permalink raw reply
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Junio C Hamano @ 2011-10-11 17:49 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Drew Northup, Jonathan Nieder
In-Reply-To: <201110111739.49967.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> On Tue, 11 Oct 2011, Junio C Hamano wrote:
>
>> I probably do not have time to look into this, but just FYI my trial merge
>> to 'pu' of this topic is failing like this:
>>
>> asciidoc: ERROR: gitweb.conf.txt: line 484: illegal style name: Default: ()
>> asciidoc: ERROR: gitweb.conf.txt: line 494: illegal style name: Default: 300
>
> Damn, I thought I have fixed that. This probably depends on AsciiDoc
> version ("make doc" on 'master' generates a few _warnings_ for me related
> to similar situation), but the problem is with
>
> [Default: <value>]
>
> that was copied from gitweb/README. But [<sth>] is an attribute list
> (style name in simplest form), used more often in newer AsciiDoc.
>
> So either we have to escape '[' and ']', i.e. use {startsb} and {endsb},
> which would reduce human-friendliness, or move to different way of marking
> default values, e.g. _italic_.
>
> What do you think?
What do the other documents in the directory this file lives say? I think
we explain what the variables does, and add "defaults to false" or
somesuch in the text, without any funny mark-up.
^ permalink raw reply
* Re: Re: [PATCH 6/6] Retain caches of submodule refs
From: Heiko Voigt @ 2011-10-11 17:41 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski
In-Reply-To: <4E93C232.9090400@alum.mit.edu>
Hi,
On Tue, Oct 11, 2011 at 06:12:34AM +0200, Michael Haggerty wrote:
> On 10/10/2011 09:53 PM, Heiko Voigt wrote:
> > On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote:
> > Since the setup_revision() api can currently not be used to safely
> > iterate twice over the same submodule my patch
> >
> > allow multiple calls to submodule merge search for the same path
> >
> > rewrites the search into using a child process. AFAIK the submodule ref
> > iteration api would then even be unused.
>
> If your patch is accepted, then we should check whether anything should
> be ripped out.
I would rather like to extend the setup_revision() api so that we can
iterate multiple times. Additionally I have a feeling that this API
might be useful for further extensions of the recursive-push support of
submodules which is currently under development.
So in summary: I would like to wait with ripping anything out until we
get to a final state with submodule support.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()
From: Jens Lehmann @ 2011-10-11 17:38 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git, Junio C Hamano, David Aguilar
In-Reply-To: <CALUzUxo6YEEpL_MhT=O9sJSUdwcpKBpeM2O8GkbbyxvqmWCFLQ@mail.gmail.com>
Am 11.10.2011 10:44, schrieb Tay Ray Chuan:
> On Tue, Oct 11, 2011 at 3:34 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> When cmd_foreach() passes an empty "name" variable to the
>> spawned command that might still work (and even make sense), but using the
>> empty name in cmd_sync() to access the config is looking like an error to
>> me. It might make sense to add an "|| exit" at least to the callsite in
>> cmd_sync(). Or am I missing something here?
>
> Cc-ed David, who authored cmd_sync().
>
> David, what do you think of Jens' analysis?
>
> In the meantime, I'll probably reword the second paragraph to say that
> future work will be needed to analyze non- || exit callsites.
Yeah, me too thinks the missing "|| exit" should be subject of another
patch.
^ permalink raw reply
* Re: Submodule confusion when checking out branch
From: Jens Lehmann @ 2011-10-11 17:35 UTC (permalink / raw)
To: Howard Miller; +Cc: git
In-Reply-To: <CAHVO_90rHbqqUx6HCh7tLWO9aP9PyPMpYnZGszCDB2bfNzUXAQ@mail.gmail.com>
Am 11.10.2011 11:38, schrieb Howard Miller:
> I added a submodule to my project like this (all from the root of the project)
>
> git submodule add git@..... path/to/submodule
> git submodule init
> git add path/to/submodule
> git commit -m 'I added a submodule!'
> git push
>
> All looks good and 'git status' reports 'nothing to commit'
>
> However, I now cannot change branches. On checkout, I get...
>
> "error: The following untracked working tree files would be
> overwritten by checkout:"
> (followed by a big list of all the files in the submodule)
>
> Where did I go wrong and what can I do to sort it?
Hmm, as I don't know for what checkout you see this problem (do you
switch from a branch containing the submodule to one that doesn't
have it or the other way round?) and assuming you had some files
committed in the directory where the submodule lives I can take a
guess what happened:
Could it be the case that you converted an existing directory into a
submodule and then get this error when you want to switch back to a
branch where this directory is still filled with the original files?
Then this is a known shortcoming of submodules at the moment. I have
experimental code to make Git work in that case but it is not ready
for inclusion yet.
^ permalink raw reply
* Re: [PATCH v2 0/7] Provide API to invalidate refs cache
From: Junio C Hamano @ 2011-10-11 17:26 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Martin Fick, Christian Couder,
Christian Couder, Thomas Rast
In-Reply-To: <4E93D932.6020001@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 10/11/2011 02:02 AM, Junio C Hamano wrote:
> ...
>> I could rebase your series, but it always is more error prone to have
>> somebody who is not the original author rebase a series than the original
>> author build for the intended base tree from the beginning.
>
> I don't mind rebasing this little series on jp/get-ref-dir-unsorted.
> ...
> Rebasing 78 patches is going to be a morass of clerical work.
I do not think it is "clerical" in the first place.
Realistically, I expect that a 50+ patch series that touch fairly an
important part of the system to take 2 cycles and a half before it hits a
released version, judging from our recent experience with the recursive
merge fix-up series.
When we already have a patch that has been discussed well enough on the
list to fix somebody's real world problem, can we afford to block it and
give an exclusive write lock to part of the codebase for 2 cycles to your
series, or anybody's for that matter?
> Is there any alternative?
I think an alternative is not to hold on to a series before it gets so
large to make you feel adjusting to the needs to other changes in the
codebase is "clerical". Commit often and early while developing the
initial pass, re-read often and throughout the whole process looking for
things you regret you would have done in early in the series that you
didn't (aka "oops, here is a fixup for the thinko in the early patch in
the series), and clean-up early while preparing to publish. Reorder the
parts that you are more confident that they do not need to change to come
early in the series, and unleash these early parts when you reach certain
confidence level.
I think your iterate-refs series was an example of good execution. It made
the codeflow a lot clearer by reducing the special casing of the submodule
parameter. In your grand scheme of things (e.g. read only parts of the ref
namespace as needed) you might consider it a mere side effect, but the
series by itself was a good thing to have.
Sometimes you may feel that a part of your series when taken out of
context would not justify itself like the iterate-refs series did, until
later parts of the series start taking advantage of the change. But that
is what commit log messages are for: e.g. "this change to encapsulate
these global variables into a single structure does not make a difference
in the current codebase, but in a later patch this and that callers will
need additional pieces of information passed aruond in the callchain, and
will add new members to the structure".
> ... So maybe
> I brought this whole mess down on my own head :-(
No, it is not anybody's fault in particular. That's life and open source.
^ permalink raw reply
* "trying out" gitolite with minimum impact on a system
From: Sitaram Chamarty @ 2011-10-11 17:19 UTC (permalink / raw)
To: Git Mailing List
(This is kind of a record for me, 2 gitolite related emails to the git
ML in less than a month)
If you were in the position of vaguely thinking gitolite may be useful
to you but lacked the time to actually install it to try things out,
this may help.
----
Some folks want to be able to try gitolite's access control features,
play with the configuration language, and so on, but to actually
*install* it seems like a "commitment" of some kind.
Gitolite has always had the ability to be installed *entirely* within
one Unix userid. Plus, the homegrown test suite I've been using for a
while now has always simulated different "gitolite users" with a bit
of ~/.ssh/config magic.
So I just combined the two, and made the client also be on the same
Unix userid, resulting in this:
http://sitaramc.github.com/gitolite/t/README.html#_playing_with_gitolite
Briefly, it boils down to this: create a user, log in as that user,
clone gitolite, run *one* command. Done.
After that, entirely within that user, you have an admin user and six
normal users, to do with as you please. You emulate different users
simply by using a different username in the URL, like "git clone
u1:reponame" versus "git clone u2:reponame".
This gives you a chance to play with the myriad access control
features gitolite has.
When done, delete that user and your system is back where it was.
--
Sitaram
^ permalink raw reply
* Re: [PATCHv5/RFC 3/6] gitweb: Add manpage for gitweb (APPLICATION!!!)
From: Drew Northup @ 2011-10-11 16:56 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Jonathan Nieder
In-Reply-To: <201110111620.12788.jnareb@gmail.com>
On Tue, 2011-10-11 at 16:20 +0200, Jakub Narebski wrote:
> On Tue, 11 Oct 2011, Drew Northup wrote:
> > On Tue, 2011-10-11 at 15:51 +0200, Jakub Narebski wrote:
> > > On Tue, 11 Oct 2011, Drew Northup wrote:
> >
> > > > This would be why I included a synopsis with my original submission. As
> > > > this was supposed to be a description of the configuration files of said
> > > > application it does not make much sense to put the executable in the
> > > > synopsis. Please forgive me for attempting to make sense!
> > >
> > > But this manpage is about _gitweb itself_, not about gitweb config file(s).
> > > Gitweb itself is application, though it is not runnable directly (yet).
> > >
> > > Web apps either don't use manpages as documentation, and those that do
> > > that I found (SVN::Web for example) include runnable server-starting script.
> >
> > Hmm, Couldn't tell from the mail header _which_ we were talking about in
> > this subthread. I'll have to read the _whole_ patch apparently next
> > time.
>
> I'm sorry. I guess better subjects would be:
>
> gitweb: Add gitweb.conf(5) manpage
> gitweb: Add gitweb(1) manpage
>
> instead of current
>
> gitweb: Add manpage for gitweb configuration files
> gitweb: Add manpage for gitweb
>
> Or perhaps:
>
> gitweb: Add manpage for gitweb configuration files
> gitweb: Add manpage for gitweb itself
Jakub,
For the sake of clarity I'd go with redundancy (but I've noticed that's
not always enormously popular around here)
gitweb: Add manpage for gitweb configuration files--gitweb.conf(5)
gitweb: Add manpage for gitweb itself--gitweb(1)
Perhaps the first one is too long, but I think that it leaves little
question as to what the patch contains. Otherwise, your first or final
pair of updated subjects would probably be clear enough.
Granted, just as soon as you fix that somebody else will manage to top
my misreading of the subject line ;-).
--
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: [PATCH v4] attr.c: respect core.ignorecase when matching attribute patterns
From: Junio C Hamano @ 2011-10-11 16:54 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, mhagger, Brandon Casey
In-Reply-To: <HBn1XX5GLcGG9WPqS0RC9Uscll_6Kbd741mHOR7uc_IFxdOpGSDGF7qEBPF66SbtO3keG4GcnkbtEvKDQ5D3bCDNiV9EPgEh-CMLKgbfFJcpVD5Gcb69-QoqTpHG_J9pDQG844LtNZU@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> ... Currently, git builds the attr stack
> based on the path supplied by the user, so we don't have to do anything
> special (like use strcmp_icase) to handle the parts of that path that don't
> match the filesystem with respect to case. If git instead built the attr
> stack by scanning the repository, then the paths in the origin field would
> not necessarily match the paths supplied by the user.
I find this description somewhat misleading. "check-attr" at the plumbing
level does take full path from the end user, but a common thing Git does
is to ask the system to learn the prefix to the current directory with
getcwd(3) append what fill_directory() enumerates as matching a pathspec
given by the user with readdir(3) to the prefix to form the full path, and
then feed that full path to git_check_attr().
Without anybody changing anything, we already do build the attr stack by
"scanning the repository" in that case, no?
^ permalink raw reply
* Re: [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-11 16:16 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <1316121043-29367-20-git-send-email-mhagger@alum.mit.edu>
On Thu, Sep 15, 2011 at 11:10:40PM +0200, Michael Haggerty wrote:
> While resolving references, if a reference is found that is in an
> unrecognized format, emit a warning (and then fail, as before).
> Wouldn't *you* want to know?
Not necessarily. :)
I noticed this today, and bisected it to this patch:
$ git.v1.7.7 show config
fatal: ambiguous argument 'config': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
$ git.mh.check-ref-format-3 show config
warning: reference in .git/config is formatted incorrectly
warning: reference in .git/config is formatted incorrectly
fatal: ambiguous argument 'config': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
which is arguably not a big deal, as we eventually report failure
anyway. But it's even worse with:
$ git branch config v1.7.7
$ git show --oneline config
warning: reference in .git/config is formatted incorrectly
703f05a Git 1.7.7
As you probably guessed, this is due to the ref resolution rules first
looking directly in .git for refs, which catches things like "HEAD" and
fully-qualified refs like "refs/heads/foo".
Which means we have been considering ".git/config" as a possible ref for
a long time, which is arguably wrong. Your patch only makes it more
obvious that this is the case.
I wonder if the right solution is for us to be more picky about what can
be found in $GIT_DIR. Maybe matching all-uppercase, or starting with
"refs/", which I think would match existing convention?
-Peff
^ permalink raw reply
* Symmetric of rebase / more intelligent cherry-pick
From: Lionel Elie Mamane @ 2011-10-11 15:54 UTC (permalink / raw)
To: git
Hi,
I was thinking about a sort of symmetric to rebase, a kind of
"intelligent" mass cherry-pick. For the sake of example let's call it
"basere", although a better name would have to be found.
While
git rebase UPSTREAM
moves the *current* branch (and any commit in it, but not in UPSTREAM)
on top of UPSTREAM,
git basere UPSTREAM
would "cherry-pick" all commits in UPSTREAM that are not in HEAD to
the current branch.
git cherry-pick ..UPSTREAM
*nearly* does what I want, except that it lacks rebase's intelligence
of skipping commits that do the same textual changes as a commit
already in the current branch. So maybe it should be an option to
cherry-pick rather than a new command? E.g. --skip-same? What about
making it cherry-picks *default* behaviour?
Why do I think it is useful? Take the case of a developer following
two different origins A and B, and B uses rebase (not merge) to
integrate changes from A, but not as often as one would like. A could
for example be the "official" tree (e.g. linux Linus tree), and B a
feature branch of a developer that is meant to eventually go into A
"cleanly" at some future time and A's policy is that one has to rebase
before sending a merge request. Then another developer wants to keep
up with changes from *both* A and B. So, the situation could look
like:
A---A---A remotes/A/master
/
X---X---B0---B1---L master
\
B0---B1---B2--B3 remotes/B/master
So one does "git rebase remotes/A/master master" and one gets
A---A---A---B0---B1---L master
/
X---X
\
B0---B1---B2--B3 remotes/B/master
I'd like to go to the situation:
A---A---A---B0---B1---L---B2--B3 master
/
X---X
\
B0---B1---B2--B3 remotes/B/master
Without having to manually find the cutoff point "B2"
(or worse, if the history is more messy than that, as in:
A---A---A---B1---L master
/
X---X
\
B0---B1---B2--B3 remotes/B/master
)
The best way I have found to do that is:
git fetch B
git checkout --no-track -b TMP remotes/B/master
git rebase master
git checkout master
git merge --ff-only TMP
git branch -d TMP
or less commands but more messy/fragile (and more work for git):
git fetch B
git tag TMP
git rebase remotes/B/master
git rebase TMP
git tag -d TMP
but that is rather onerous for something that should be just one
command.
So what are your thoughts on that?
- Good idea? Would you take a patch that does something like that?
- Should it be cherry-picks's default behaviour?
- An option to cherry-pick?
- A new command? What name? Maybe "multi-cherry-pick"?
- An option to rebase, e.g. "--pull" or "--incoming" or "--here"?
("--pull" is probably too confusing wrt to "git pull"; people will
have trouble remembering which is which between "git pull
--rebase" and "git rebase --pull"...)
--
Lionel
^ permalink raw reply
* Release notes link on homepage 404
From: Lionel Elie Mamane @ 2011-10-11 15:58 UTC (permalink / raw)
To: git
The "Release Notes" link on http://git-scm.com/ is a 404
--
Lionel
^ 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