* git-svn breakage on repository rename
From: Guido Stevens @ 2009-12-04 20:26 UTC (permalink / raw)
To: normalperson, git; +Cc: George Kuk
Hi Eric e.a.,
I have a weird git-svn corner case that might interest you (or not at
all). I'd appreciate any help or hints for moving beyond this problem.
I'm using git-svn to do a full commit history analysis of the Zope +
Plone CMS code bases as part of a research project with the University
of Nottingham into open source knowledge dynamics.
One of the repositories I'm importing breaks with a "Checksum mismatch",
indicating a corruption. However, this error message occurs exactly at
the point where the repository was renamed: from "Products.CMFPlone" to
"Plone" (22715->22716). (Yes, it's the Plone core itself that resists
analysis :-()
The git-svn url for the later commits would be:
http://svn-mirror.plone.org/svn/plone/Plone/trunk
Whereas an older part of the code base lives at:
http://svn-mirror.plone.org/svn/plone/Products.CMFPlone/trunk
Funny thing is, git-svn detects this rename upfront but then breaks
anyway. Which raises the questions:
- is this breakage caused by the rename?
- or does git-svn handle the rename, and there is an actual corruption?
- is there any way I can work around this and get a valid or semi-valid
git history for this project?
I don't mind missing a few commits, since I'm not doing code development
on this repository, only statistical analysis.
Solving this would also be helpful for anyone who wants to work on Plone
development through git rather than through raw svn.
:*CU#
----------------------------------------------------
To reconstruct this error:
----------------------------------------------------
$ git svn init https://svn-mirror.plone.org/svn/plone/Plone/trunk Plone
$ cd Plone
$ git svn fetch
... Error message: (reformatted to wrap 78 cols):
Found possible branch point:
https://svn.plone.org/svn/plone/Plone/branches/4.0 =>
https://svn.plone.org/svn/plone/Plone/trunk, 30966
Initializing parent: git-svn@30966
Found possible branch point:
https://svn.plone.org/svn/plone/Plone/branches/3.3 =>
https://svn.plone.org/svn/plone/Plone/branches/4.0, 27288
Initializing parent: git-svn@27288
Found possible branch point:
https://svn.plone.org/svn/plone/Plone/branches/3.2 =>
https://svn.plone.org/svn/plone/Plone/branches/3.3, 25119
Initializing parent: git-svn@25119
Found possible branch point:
https://svn.plone.org/svn/plone/Plone/branches/3.1 =>
https://svn.plone.org/svn/plone/Plone/branches/3.2, 22725
Initializing parent: git-svn@22725
branch_from: /Products.CMFPlone => /Products.CMFPlone/branches/3.1
Found possible branch point:
https://svn.plone.org/svn/plone/Products.CMFPlone/branches/3.1 =>
https://svn.plone.org/svn/plone/Plone/branches/3.1, 22715
Initializing parent: git-svn@22715
Found branch parent: (git-svn@22725)
e477345f83a0f2cc7e27348e01493a841c9cd587
Following parent with do_switch
Checksum mismatch: Products/CMFPlone/HISTORY.txt
expected: 69106809d879e7370dd133c7ba338670
got: 7b1a0641d429f0c567acf7a3a4be5a45
--
*** Guido A.J. Stevens *** tel: +31.43.3618933 ***
*** guido.stevens@cosent.net *** Postbus 619 ***
*** http://www.cosent.nl *** 6200 AP Maastricht ***
s h a r i n g m a k e s s e n s e
^ permalink raw reply
* Re: Wrong damage counting in diffcore_count_changes?
From: Junio C Hamano @ 2009-12-04 20:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0912041200120.24579@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> This changes it so that
>
> - whenever it bypasses a destination hash (because it doesn't match a
> source), it counts the bytes associated with that as "literal added"
>
> - at the end (once we have used up all the source hashes), we do the same
> thing with the remaining destination hashes.
>
> - when hashes do match, and we use the difference in counts as a value,
> we also use up that destination hash entry (the 'd++'
>
> This _seems_ to make --dirstat output more sensible, and I'd hope that
> that in turn should mean that file rename detection should also be more
> sensible. But I haven't actually verified it in any way. Maybe I just
> screwed up file rename detection entirely.
>
> Did I miss something?
Well, the current loop structure largely comes from your eb4d0e3 (optimize
diffcore-delta by sorting hash entries., 2007-10-02) so you would be the
best judge of the change ;-), even though it seems that the current code
inherited the "skipping of dst when src does not exist" bug from c06c796
(diffcore-rename: somewhat optimized., 2006-03-12).
Earlier code, e.g. as of ba23bbc (diffcore-delta: make change counter to
byte oriented again., 2006-03-04), used to be very simple minded and
inefficient and iterated over src_count[] and dst_count[] arrays for the
entire hash value namespace and there was no such "missing, skipped,
happened to have the next value" issue.
I think you understand the original intention of the function correctly
and from a cursory look of the patch I think it fixes the bug in the
current code, and any changes to renames/breaks should be improvements.
But my lunchbreak is over, and my evening is booked, so I unfortunately
cannot spend more time thinking about any possible fallouts from this
change until tomorrow.
Sorry, and thanks.
> Linus
> ---
> diffcore-delta.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/diffcore-delta.c b/diffcore-delta.c
> index e670f85..7cf431d 100644
> --- a/diffcore-delta.c
> +++ b/diffcore-delta.c
> @@ -201,10 +201,15 @@ int diffcore_count_changes(struct diff_filespec *src,
> while (d->cnt) {
> if (d->hashval >= s->hashval)
> break;
> + la += d->cnt;
> d++;
> }
> src_cnt = s->cnt;
> - dst_cnt = d->hashval == s->hashval ? d->cnt : 0;
> + dst_cnt = 0;
> + if (d->cnt && d->hashval == s->hashval) {
> + dst_cnt = d->cnt;
> + d++;
> + }
> if (src_cnt < dst_cnt) {
> la += dst_cnt - src_cnt;
> sc += src_cnt;
> @@ -213,6 +218,10 @@ int diffcore_count_changes(struct diff_filespec *src,
> sc += dst_cnt;
> s++;
> }
> + while (d->cnt) {
> + la += d->cnt;
> + d++;
> + }
>
> if (!src_count_p)
> free(src_count);
^ permalink raw reply
* Re: Endianness bug in git-svn or called component?
From: Nathaniel W Filardo @ 2009-12-04 20:29 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Nathaniel W Filardo, git
In-Reply-To: <m23a3qa40n.fsf@igel.home>
[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]
On Fri, Dec 04, 2009 at 09:16:40PM +0100, Andreas Schwab wrote:
> Nathaniel W Filardo <nwf@cs.jhu.edu> writes:
>
> > On this machine,
> >
> > mirrors hydra:/tank0/mirrors/misc% uname -a
> > FreeBSD hydra.priv.oc.ietfng.org 9.0-CURRENT FreeBSD 9.0-CURRENT #13: Sat Nov 14 19:40:25 EST 2009 root@hydra.priv.oc.ietfng.org:/systank/obj/systank/src/sys/NWFKERN sparc64
> >
> > attempting to fetch from an svn source yields the following error:
> >
> > rs hydra:/tank0/mirrors/misc% git svn init -s https://joshua.svn.sourceforge.net/svnroot/joshua test-joshua
> > Initialized empty Git repository in /tank0/mirrors/misc/test-joshua/.git/
> > mirrors hydra:/tank0/mirrors/misc% cd test-joshua
> > mirrors hydra:/tank0/mirrors/misc/test-joshua% git svn fetch
> > A scripts/distributedLM/config.template
> > [...]
> > A build.xml
> > r1 = fe84a7d821ec6d92da75133ac7ad1deb476b6484 (refs/remotes/trunk)
> > error: index uses extension, which we do not understand
> > fatal: index file corrupt
> > write-tree: command returned error: 128
>
> I could not reproduce that on powerpc (both 32bit and 64bit).
>
> Andreas.
Hm. I seem to have forgotten to mention that I am running
% git --version
git version 1.6.5.3
built from the FreeBSD ports tree, in case that matters, using
% gcc --version
gcc (GCC) 4.2.1 20070719 [FreeBSD]
Knowing nothing of git's internals, how should I start to investigate what's
going on here?
Thanks again.
--nwf;
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Endianness bug in git-svn or called component?
From: Andreas Schwab @ 2009-12-04 20:16 UTC (permalink / raw)
To: Nathaniel W Filardo; +Cc: git
In-Reply-To: <20091204174458.GV17192@gradx.cs.jhu.edu>
Nathaniel W Filardo <nwf@cs.jhu.edu> writes:
> On this machine,
>
> mirrors hydra:/tank0/mirrors/misc% uname -a
> FreeBSD hydra.priv.oc.ietfng.org 9.0-CURRENT FreeBSD 9.0-CURRENT #13: Sat Nov 14 19:40:25 EST 2009 root@hydra.priv.oc.ietfng.org:/systank/obj/systank/src/sys/NWFKERN sparc64
>
> attempting to fetch from an svn source yields the following error:
>
> rs hydra:/tank0/mirrors/misc% git svn init -s https://joshua.svn.sourceforge.net/svnroot/joshua test-joshua
> Initialized empty Git repository in /tank0/mirrors/misc/test-joshua/.git/
> mirrors hydra:/tank0/mirrors/misc% cd test-joshua
> mirrors hydra:/tank0/mirrors/misc/test-joshua% git svn fetch
> A scripts/distributedLM/config.template
> [...]
> A build.xml
> r1 = fe84a7d821ec6d92da75133ac7ad1deb476b6484 (refs/remotes/trunk)
> error: index uses extension, which we do not understand
> fatal: index file corrupt
> write-tree: command returned error: 128
I could not reproduce that on powerpc (both 32bit and 64bit).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Wrong damage counting in diffcore_count_changes?
From: Linus Torvalds @ 2009-12-04 20:07 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
Ok, so I had somebody actually ask me about '--dirstat', and as a result I
ended up looking at how well the numbers it calculates really reflect the
damage done to a file.
And to my horror, it doesn't necessarily reflect the damage well at all!
Now, dirstat just takes the same damage numbers that git uses to estimate
similarity for renames, so if dirstat gets odd numbers, then that implies
that file similarity will also be somewhat odd.
So looking at _why_ the dirstat numbers were odd, I came up with this
patch that seems to make much sense. What used to happen is that
diffcore_count_changes() simply ignored any hashes in the destination that
didn't match hashes in the source. EXCEPT if the source hash didn't exist
at all, in which case it would count _one_ destination hash that happened
to have the "next" hash value.
This changes it so that
- whenever it bypasses a destination hash (because it doesn't match a
source), it counts the bytes associated with that as "literal added"
- at the end (once we have used up all the source hashes), we do the same
thing with the remaining destination hashes.
- when hashes do match, and we use the difference in counts as a value,
we also use up that destination hash entry (the 'd++'
This _seems_ to make --dirstat output more sensible, and I'd hope that
that in turn should mean that file rename detection should also be more
sensible. But I haven't actually verified it in any way. Maybe I just
screwed up file rename detection entirely.
Did I miss something?
Linus
---
diffcore-delta.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/diffcore-delta.c b/diffcore-delta.c
index e670f85..7cf431d 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -201,10 +201,15 @@ int diffcore_count_changes(struct diff_filespec *src,
while (d->cnt) {
if (d->hashval >= s->hashval)
break;
+ la += d->cnt;
d++;
}
src_cnt = s->cnt;
- dst_cnt = d->hashval == s->hashval ? d->cnt : 0;
+ dst_cnt = 0;
+ if (d->cnt && d->hashval == s->hashval) {
+ dst_cnt = d->cnt;
+ d++;
+ }
if (src_cnt < dst_cnt) {
la += dst_cnt - src_cnt;
sc += src_cnt;
@@ -213,6 +218,10 @@ int diffcore_count_changes(struct diff_filespec *src,
sc += dst_cnt;
s++;
}
+ while (d->cnt) {
+ la += d->cnt;
+ d++;
+ }
if (!src_count_p)
free(src_count);
^ permalink raw reply related
* Re: [ANNOUNCE] Git 1.6.5.4
From: Todd Zullinger @ 2009-12-04 19:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andreas Schwab, Michael J Gruber, git
In-Reply-To: <7vzl5ysm11.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> I think it depends on the likelihood that a distro has xmlto so old that
>>> it does not understand --stringparam yet it uses stylesheet so new that
>>> setting the parameter makes a positive difference (either it gives the
>>> full URL or at least squelches the "You should define the parameter"
>>> noise) in the output.
>>
>> openSUSE 11.2, for example. Its xmlto has a non-standard --xsltopts
>> option that passes its argument down to xsltproc.
>
> Ok, as I said that I've been wrong before in this area ;-)
>
> I don't think I will have much time for git today, and it would be
> appreciated if somebody can work on this and send a tested patch that
> applies cleanly on top of 'maint' to implement the @@MAN_BASE_URL@@
> replacement from manpage-base.xsl.in to manpage-base.xsl as Todd suggested
> earlier.
Something like this perhaps? I tested it with 1.6.5.4 on Fedora 10,
12 and CentOS 5.4 but it could surely use more eyes and testing to
ensure it works as it should and doesn't have unintended negative
effects on make clean and such.
This does set MAN_BASE_URL unconditionally, pointing to kernel.org.
That way anyone building with recent DocBook and taking no new action
will have something useful in the man page links.
I noticed on Fedora 12 that email addresses get added to the NOTES
section. For example, git-branch(1) has:
Documentation by Junio C Hamano and the git-list <git@vger.kernel.org[4]>.
...
NOTES
...
4. git@vger.kernel.org
mailto:git@vger.kernel.org
That must be something new in DocBook, as it doesn't occur in the
Fedora 10 builds. It's a little extraneous, but not harmful I guess.
--- >8 ---
From: Todd Zullinger <tmz@pobox.com>
Date: Fri, 4 Dec 2009 12:53:21 -0500
Subject: [PATCH] Documentation: Avoid use of xmlto --stringparam
The --stringparam option is not available on older xmlto versions.
Instead, set man.base.url.for.relative.links via a .xsl file. Older
docbook versions will ignore this without causing grief to users of
older xmlto versions.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
Documentation/.gitignore | 1 +
Documentation/Makefile | 23 ++++++++++++-----------
Documentation/manpage-base-url.xsl.in | 10 ++++++++++
3 files changed, 23 insertions(+), 11 deletions(-)
create mode 100644 Documentation/manpage-base-url.xsl.in
diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index d8edd90..1c3a9fe 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -8,3 +8,4 @@ gitman.info
howto-index.txt
doc.dep
cmds-*.txt
+manpage-base-url.xsl
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 1c9dfce..1c867fa 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -104,18 +104,15 @@ XMLTO_EXTRA += -m manpage-suppress-sp.xsl
endif
# Newer DocBook stylesheet emits warning cruft in the output when
-# this is not set, and if set it shows an absolute link. We can
-# use MAN_BASE_URL=http://www.kernel.org/pub/software/scm/git/docs/
-# but distros may want to set it to /usr/share/doc/git-core/docs/ or
-# something like that.
+# this is not set, and if set it shows an absolute link. Older
+# stylesheets simply ignore this parameter.
#
-# As older stylesheets simply ignore this parameter, it ought to be
-# safe to set it to empty string when the base URL is not specified,
-# but unfortunately we cannot do so unconditionally because at least
-# xmlto 0.0.18 is reported to lack --stringparam option.
-ifdef MAN_BASE_URL
-XMLTO_EXTRA += --stringparam man.base.url.for.relative.links=$(MAN_BASE_URL)
+# Distros may want to use MAN_BASE_URL=file:///path/to/git/docs/
+# or similar.
+ifndef MAN_BASE_URL
+MAN_BASE_URL = http://www.kernel.org/pub/software/scm/git/docs/
endif
+XMLTO_EXTRA += -m manpage-base-url.xsl
# If your target system uses GNU groff, it may try to render
# apostrophes as a "pretty" apostrophe using unicode. This breaks
@@ -244,6 +241,7 @@ clean:
$(RM) howto-index.txt howto/*.html doc.dep
$(RM) technical/api-*.html technical/api-index.txt
$(RM) $(cmds_txt) *.made
+ $(RM) manpage-base-url.xsl
$(MAN_HTML): %.html : %.txt
$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
@@ -251,7 +249,10 @@ $(MAN_HTML): %.html : %.txt
$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) -o $@+ $< && \
mv $@+ $@
-%.1 %.5 %.7 : %.xml
+manpage-base-url.xsl: manpage-base-url.xsl.in
+ sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
+
+%.1 %.5 %.7 : %.xml manpage-base-url.xsl
$(QUIET_XMLTO)$(RM) $@ && \
xmlto -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
diff --git a/Documentation/manpage-base-url.xsl.in b/Documentation/manpage-base-url.xsl.in
new file mode 100644
index 0000000..e800904
--- /dev/null
+++ b/Documentation/manpage-base-url.xsl.in
@@ -0,0 +1,10 @@
+<!-- manpage-base-url.xsl:
+ special settings for manpages rendered from newer docbook -->
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+ version="1.0">
+
+<!-- set a base URL for relative links -->
+<xsl:param name="man.base.url.for.relative.links"
+ >@@MAN_BASE_URL@@</xsl:param>
+
+</xsl:stylesheet>
--
1.6.6.rc1
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The world keeps ending but new people too dumb to know it keep showing
up as if the fun's just started.
^ permalink raw reply related
* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Johannes Schindelin @ 2009-12-04 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, Michael J Gruber, Michael Haggerty, git
In-Reply-To: <7vws12r5v2.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, 4 Dec 2009, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
> > Michael J Gruber <git@drmicha.warpmail.net> writes:
> >
> >>> If the idea of a "fix" command is acceptable, then I would like to
> >>> implement a further convenience: if a group of commits to be folded
> >>> together includes *only* "fix" commits, then the first log message
> >>> should be used without even opening an editor. But I would like to
> >>> get a reaction to the "fix" command in general before doing so.
> >>
> >> I'd say that would make a useful command ("fix") even more useful, being
> >> just the right counterpart to "reword" for trivial commit message fixes.
> >
> > +1 for fix, and +1 for the "don't even launch the editor" too.
>
> I like it, too. Also I vaguely recall that there was a series that died
> that would have allowed you to give hints to help this behaviour at the
> time you make "fix-up" commits; we may want to resurrect it on top of this
> feature.
I'll just repeat this exactly one more time: it is not always possible to
know whether you make a fix-up commit, and it is not always possible to be
sure that you want to amend the next time you do a rebase.
So: Commit time is definitely a bad time to decide on the action in some
future rebase event.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Johannes Schindelin @ 2009-12-04 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git
In-Reply-To: <7v638mskmx.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, 4 Dec 2009, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 0bd3bf7..539413d 100755
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -302,7 +302,7 @@ nth_string () {
> >
> > make_squash_message () {
> > if test -f "$SQUASH_MSG"; then
> > - COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> > + COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
> > < "$SQUASH_MSG" | sed -ne '$p')+1))
>
> This sed replacement worries me. I don't have a time to check myself
> today but do we use \(this\|or\|that\) alternates with our sed script
> already elsewhere in the codebase (test scripts do not count)?
>
> Otherwise this may suddenly be breaking a platform that has an
> implementation of sed that may be substandard but so far has been
> sufficient to work with git.
IIRC "|" was not correctly handled by BSD sed (used e.g. in MacOSX).
So maybe it would be best to just look for "commit message"? I agree with
Michael that the regex should not be too loose.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH v2 4/8] Support remote helpers implementing smart transports
From: Shawn O. Pearce @ 2009-12-04 18:37 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1259942168-24869-7-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
>
> +'connect' <service>::
> + Connects to given service. Stdin and stdout of helper are
> + connected to specified service (git prefix is included in service
> + name so e.g. fetching uses 'git-upload-pack' as service) on
> + remote side. Valid replies to this command are empty line
> + (connection established), 'FALLBACK' (no smart transport support,
Why not 'fallback' to remain consistent with this protocol and many
others in git where we stick to lowercase ASCII?
> + fall back to dumb transports) and just exiting with error message
> + printed (can't connect, don't bother trying to fall back). After
> + line feed terminating the positive (empty) response, the output
> + of service starts. After the connection ends, the remote
> + helper exits. Note that to prevent deadlocking, all read data
> + should be immediately flushed to outgoing connection (excepting
> + remote initial advertisments, which should be flushed on first
> + flush packet (0000 as length) encountered.
Why is the initial advertisement special? If the helper always
flushes both sides, it shouldn't ever deadlock the protocol. Also,
note that a helper should be able to implement a tiny delay like
Nagle's algorithm does in TCP. It just can't sit on a byte forever.
> @@ -72,7 +73,10 @@ static struct child_process *get_helper(struct transport *transport)
> helper->argv = xcalloc(4, sizeof(*helper->argv));
> strbuf_addf(&buf, "remote-%s", data->name);
> helper->argv[0] = strbuf_detach(&buf, NULL);
> - helper->argv[1] = transport->remote->name;
> + if(transport->remote)
> + helper->argv[1] = transport->remote->name;
> + else
> + helper->argv[1] = "";
This hunk appears to be unrelated. And actually, if transport has
no remote, shouldn't the arg here be NULL so the helper gets only
1 argument and not 2 arguments?
> +static int _process_connect(struct transport *transport,
> + const char *name, const char *exec)
> +{
> + struct helper_data *data = transport->data;
> + struct strbuf cmdbuf = STRBUF_INIT;
> + struct child_process *helper;
> + int r;
> +
> + helper = get_helper(transport);
> +
> + /* Handle --upload-pack and friends. This is fire and forget...
> + just warn if it fails. */
> + if(exec && strcmp(name, exec)) {
> + r = set_helper_option(transport, "servpath", exec);
> + if(r > 0)
> + fprintf(stderr, "Warning: Setting remote service path "
> + "not supported by protocol.\n");
> + else if(r < 0)
> + fprintf(stderr, "Warning: Invalid remote service "
> + "path.\n");
> + }
I think exec winds up defaulting to name if --upload-pack was not
used on the command line, and remote.$name.uploadpack was not set.
See transport.c where you initialize the git options struct, these
fields were defaulted in.
My point is, we shouldn't send option servpath to the helper if
name is equal to servpath, because the helper might not support
servpath and the option command will issue a warning above for no
reason at all.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v2 3/8] Support taking over transports
From: Shawn O. Pearce @ 2009-12-04 18:27 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1259942168-24869-6-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> Add support for taking over transports that turn out to be smart.
I really don't like this disown strategy and its magic ref return
value from fetch.
> @@ -1020,7 +1089,13 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
> heads[nr_heads++] = rm;
> }
>
> +retry:
> rc = transport->fetch(transport, nr_heads, heads);
> + if(rc == TRANSPORT_LAYER6_READY) {
> + git_take_over_transport(transport);
> + goto retry;
> + }
Why can't you expose git_take_over_transport as a public function
and then the transport-helper.c code can instead do:
... setup connect with helper ...
transport_takeover(transport, child);
return transport->fetch(....);
Would this make the code simpler?
--
Shawn.
^ permalink raw reply
* Re: git-blame.el: what is format-spec?
From: Matthieu Moy @ 2009-12-04 18:18 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Sergei Organov, git
In-Reply-To: <m2bpieab67.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Sergei Organov <osv@javad.com> writes:
>
>> However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
>> is not installed on our server where I've encountered the problem.
>
> Gnus has been part of Emacs since more than 10 years.
... but Linux distros often cut it out by default, so if you just ask
for Emacs as a package, you don't get the Gnus package.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: git-blame.el: what is format-spec?
From: David Kågedal @ 2009-12-04 17:36 UTC (permalink / raw)
To: Sergei Organov; +Cc: git, Andreas Schwab
In-Reply-To: <87ljhi3cao.fsf@osv.gnss.ru>
Sergei Organov <osv@javad.com> writes:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Sergei Organov <osv@javad.com> writes:
>>
>>> What is format-spec function in current git-blame.el? Neither my GNU
>>> Emacs 22.2.1 nor Google knows anything about it.
>>
>> It's part of Emacs since more than 9 years, imported from Gnus.
>>
>
> Thanks, I now see it in Gnus on my own computer, in
> lisp/gnus/format-spec.el.gz.
>
> GNU Emacs 22.2.1 (i486-pc-linux-gnu, GTK+ Version 2.12.11) of 2008-11-10
> on raven, modified by Debian
>
> However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
> is not installed on our server where I've encountered the problem. Was
> format-spec actually moved to core emacs recently?
That was not my intention when I posted the patch. I seem to recall that
I asked for testing, in particular from users with older Emacsen than
23. But I got no response, and only recently discovered that the patch
hade been accepted.
format-spec is included in Emacs 23, and is a useful function. But we
can rewrite git-blame.el to do the formatting manuall instead.
--
David Kågedal
^ permalink raw reply
* Re: [PATCH v8 7/7] fast-import: add (non-)relative-marks feature
From: Daniel Barkalow @ 2009-12-04 18:09 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
vcs-fast-import-devs
In-Reply-To: <1259946420-8845-8-git-send-email-srabbelier@gmail.com>
On Fri, 4 Dec 2009, Sverre Rabbelier wrote:
> After specifying 'feature relative-marks' the paths specified with
> 'feature import-marks' and 'feature export-marks' are relative to an
> internal directory in the current repository.
>
> In git-fast-import this means that the paths are relative to the
> '.git/info/fast-import' directory. However, other importers may use a
> different location.
>
> Add 'feature non-relative-marks' to disable this behavior, this way
> it is possible to, for example, specify the import-marks location as
> relative, and the export-marks location as non-relative.
>
> Also add tests to verify this behavior.
>
> Cc: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> As requested by Daniel, it is now possible to have the marks be
> relative to a constant directory. We might want to consider making
> this the default at some point.
>
> This patch opens the way for remote-helpers to use the marks feature
> without poluting the work tree, which I think is very important.
I think it would be better to make relative paths be the only available
method, in part because we don't want to polute the work tree, but more
because otherwise scripts aren't transferrable. That is, if you have an
absolute path, you can't send the same script to two different importers
(because they'd have to write their marks to the same location). And
there's no way for the program constructing a script to determine a good
absolute location if the script may be run on a different host than it's
generated on (think of getting a Linux user getting a script attached to a
bug report from a native Windows user; there are no absolute paths that
are valid on both of these).
(However, it probably does make sense to permit absolute paths in the
command line, since whoever's writing the command line is presumably aware
of any local conventions)
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Endianness bug in git-svn or called component?
From: Nathaniel W Filardo @ 2009-12-04 17:44 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]
On this machine,
mirrors hydra:/tank0/mirrors/misc% uname -a
FreeBSD hydra.priv.oc.ietfng.org 9.0-CURRENT FreeBSD 9.0-CURRENT #13: Sat Nov 14 19:40:25 EST 2009 root@hydra.priv.oc.ietfng.org:/systank/obj/systank/src/sys/NWFKERN sparc64
attempting to fetch from an svn source yields the following error:
rs hydra:/tank0/mirrors/misc% git svn init -s https://joshua.svn.sourceforge.net/svnroot/joshua test-joshua
Initialized empty Git repository in /tank0/mirrors/misc/test-joshua/.git/
mirrors hydra:/tank0/mirrors/misc% cd test-joshua
mirrors hydra:/tank0/mirrors/misc/test-joshua% git svn fetch
A scripts/distributedLM/config.template
[...]
A build.xml
r1 = fe84a7d821ec6d92da75133ac7ad1deb476b6484 (refs/remotes/trunk)
error: index uses extension, which we do not understand
fatal: index file corrupt
write-tree: command returned error: 128
Even on previously initialized and fetched git-svn repos, "git svn fetch"
produces the same response.
This is the same symptom as an older bug, but I have not attempted to track
down what's going wrong this time in hopes that somebody more familiar can
fix it faster; see
http://kerneltrap.org/mailarchive/git/2006/5/28/205750/thread#mid-205750 .
If there's anything more that would be helpful to know, please just ask.
Thanks.
--nwf;
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers
From: Shawn O. Pearce @ 2009-12-04 17:55 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1259942168-24869-4-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> Change URL handling to allow external protocol handlers to implement
> new protocols without the '::' syntax if helper name does not conflict
> with any built-in protocol.
>
> foo:// now invokes git-remote-foo with foo:// URL.
...
Uh, great, but...
> @@ -30,6 +50,7 @@ static struct child_process *get_helper(struct transport *transport)
> const char **refspecs = NULL;
> int refspec_nr = 0;
> int refspec_alloc = 0;
> + int duped;
>
> if (data->helper)
> return data->helper;
...
> + /* Open the output as FILE* so strbuf_getline() can be used.
> + Do this with duped fd because fclose() will close the fd,
> + and stuff like disowning will require the fd to remain.
> +
> + Set the stream to unbuffered because some reads are critical
> + in sense that any overreading will cause deadlocks.
> + */
> + if((duped = dup(helper->out)) < 0)
> + die_errno("Can't dup helper output fd");
> + data->out = xfdopen(duped, "r");
> + setvbuf(data->out, NULL, _IONBF, 0);
> +
This is an entirely unrelated change. Please split it into its
own commit so its easier to review, test, blah blah blah.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Matthieu Moy @ 2009-12-04 17:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Johannes.Schindelin
In-Reply-To: <7v638mskmx.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 0bd3bf7..539413d 100755
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -302,7 +302,7 @@ nth_string () {
>>
>> make_squash_message () {
>> if test -f "$SQUASH_MSG"; then
>> - COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>> + COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>> < "$SQUASH_MSG" | sed -ne '$p')+1))
>
> This sed replacement worries me. I don't have a time to check myself
> today but do we use \(this\|or\|that\) alternates with our sed script
> already elsewhere in the codebase (test scripts do not count)?
It seems we don't:
git$ git grep '\\|' *.sh
git-rebase--interactive.sh: COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\
git$
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Junio C Hamano @ 2009-12-04 17:44 UTC (permalink / raw)
To: Matthieu Moy
Cc: Michael J Gruber, Michael Haggerty, git, gitster,
Johannes.Schindelin
In-Reply-To: <vpqfx7qocwl.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>>> If the idea of a "fix" command is acceptable, then I would like to
>>> implement a further convenience: if a group of commits to be folded
>>> together includes *only* "fix" commits, then the first log message
>>> should be used without even opening an editor. But I would like to
>>> get a reaction to the "fix" command in general before doing so.
>>
>> I'd say that would make a useful command ("fix") even more useful, being
>> just the right counterpart to "reword" for trivial commit message fixes.
>
> +1 for fix, and +1 for the "don't even launch the editor" too.
I like it, too. Also I vaguely recall that there was a series that died
that would have allowed you to give hints to help this behaviour at the
time you make "fix-up" commits; we may want to resurrect it on top of this
feature.
^ permalink raw reply
* Re: git-blame.el: what is format-spec?
From: Andreas Schwab @ 2009-12-04 17:42 UTC (permalink / raw)
To: Sergei Organov; +Cc: git
In-Reply-To: <87ljhi3cao.fsf@osv.gnss.ru>
Sergei Organov <osv@javad.com> writes:
> However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
> is not installed on our server where I've encountered the problem.
Gnus has been part of Emacs since more than 10 years.
> Was format-spec actually moved to core emacs recently?
Yes, in 23.1.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Matthieu Moy @ 2009-12-04 17:40 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Michael Haggerty, git, gitster, Johannes.Schindelin
In-Reply-To: <4B192701.4000308@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>> If the idea of a "fix" command is acceptable, then I would like to
>> implement a further convenience: if a group of commits to be folded
>> together includes *only* "fix" commits, then the first log message
>> should be used without even opening an editor. But I would like to
>> get a reaction to the "fix" command in general before doing so.
>
> I'd say that would make a useful command ("fix") even more useful, being
> just the right counterpart to "reword" for trivial commit message fixes.
+1 for fix, and +1 for the "don't even launch the editor" too.
> OTOH, it would not be possible any more to squash in a few fixes and
> then edit the message. Maybe having to quit the editor is not that much
> work after all.
Well, it's still possible, by using "squash" and deleting the extra
part. In both cases, there's one scenario where one has a little more
to do than strictly required (either delete a bit of texte or close
the editor), but I think the senario "just want to fix the content
with another patch, but the commit message was good" is so common that
it deserves being optimized.
In particular, that would make a rebase -i with only reordering and
fixes non-interactive. With 3 fixes, you just wait for Git around a
second and it's done, smoothly, while it requires launching/closing
the editor 3 times ...
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Junio C Hamano @ 2009-12-04 17:40 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Johannes.Schindelin
In-Reply-To: <6d779d2c244bf5d5b7924cdc5daf66a8186e4bc7.1259934977.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..539413d 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,7 @@ nth_string () {
>
> make_squash_message () {
> if test -f "$SQUASH_MSG"; then
> - COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> + COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
> < "$SQUASH_MSG" | sed -ne '$p')+1))
This sed replacement worries me. I don't have a time to check myself
today but do we use \(this\|or\|that\) alternates with our sed script
already elsewhere in the codebase (test scripts do not count)?
Otherwise this may suddenly be breaking a platform that has an
implementation of sed that may be substandard but so far has been
sufficient to work with git.
^ permalink raw reply
* Re: rev-parse --show-cdup in .git
From: Junio C Hamano @ 2009-12-04 17:32 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4B192EAE.8000806@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> I'm sure this was discussed somewhere, but I can't find it:
>
> When called from within .git, git rev-parse --show-cdup returns nothing
> rather than "..", not even an error code. Is this intended?
I do not think --show-cdup should be linked to --is-inside-work-tree in
any way, if that is what you are getting at by mentioning "error code".
> This is all the more disturbing since the cwd of hooks seems to be
> GIT_DIR. Is that something one can rely upon? In that case one can
> simply use ".." for cdup.
I think you will see something that is different from and is more sensible
than ".." when you use GIT_WORK_TREE environment variable (or its
corresponding configuration variable).
^ permalink raw reply
* Re: git reset --hard in .git causes a checkout in that directory
From: Junio C Hamano @ 2009-12-04 17:25 UTC (permalink / raw)
To: Jeff King; +Cc: Maarten Lankhorst, Junio C Hamano, git
In-Reply-To: <20091204111158.GE27495@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Junio, I think the following should go to maint (I didn't bother
> splitting the --merge and --hard code; --merge is in v1.6.2. I assumed
> we don't care about maint releases that far back).
Thanks. The test already checks that the change won't break soft reset,
which is good, but it does not seem to check/specify what should happen in
the mixed reset in this case (I think it should be allowed). Could you
add one while at it?
> diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
> index 42bf518..3ddf0ac 100755
> --- a/t/t7103-reset-bare.sh
> +++ b/t/t7103-reset-bare.sh
> @@ -11,6 +11,16 @@ test_expect_success 'setup non-bare' '
> git commit -a -m two
> '
>
> +test_expect_success 'hard reset requires a worktree' '
> + (cd .git &&
> + test_must_fail git reset --hard)
> +'
> +
> +test_expect_success 'merge reset requires a worktree' '
> + (cd .git &&
> + test_must_fail git reset --merge)
> +'
> +
> test_expect_success 'setup bare' '
> git clone --bare . bare.git &&
> cd bare.git
> @@ -20,6 +30,10 @@ test_expect_success 'hard reset is not allowed' '
> test_must_fail git reset --hard HEAD^
> '
>
> +test_expect_success 'merge reset is not allowed' '
> + test_must_fail git reset --merge HEAD^
> +'
> +
> test_expect_success 'soft reset is allowed' '
> git reset --soft HEAD^ &&
> test "`git show --pretty=format:%s | head -n 1`" = "one"
> --
> 1.6.6.rc1.18.ga777f.dirty
^ permalink raw reply
* Re: Running commands in wrong environment
From: Junio C Hamano @ 2009-12-04 17:16 UTC (permalink / raw)
To: Jeff King; +Cc: Marinescu Paul dan, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20091204104441.GD27495@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But the consequences, as you mention, could include data loss, which
> argues for being on the safe side. In that case, we would probably want
> an "xsetenv" to die() if we fail to avoid cluttering the code
> everywhere.
>
> I dunno. If we're going to do it, it is probably maint-worthy. Junio?
My gut feeling is xsetenv/xputenv would be sufficient. I do not think we
make any setenv/putenv that we do not care about failing, perhaps other
than the "if LESS is not there set it to this default value, as it would
give users nicer experience if they use 'less'."
^ permalink raw reply
* Re: [BUG] crash on make test
From: Junio C Hamano @ 2009-12-04 17:12 UTC (permalink / raw)
To: Jeff King; +Cc: Marinescu Paul dan, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20091204103557.GC27495@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think we should apply the patch below to maint, as this is something
> that can come up due to permissions problems. But I fear it won't
> actually fix the test failure you are seeing; you will just see it die()
> instead of segfaulting. However, the value of errno should give us a
> clue about what is happening, so please try running the test again with
> this patch.
Thanks, and I agree with the whole of the above paragraph, not just the
first sentence.
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-04 17:10 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Todd Zullinger, Michael J Gruber, git
In-Reply-To: <m2r5rb9hes.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I think it depends on the likelihood that a distro has xmlto so old that
>> it does not understand --stringparam yet it uses stylesheet so new that
>> setting the parameter makes a positive difference (either it gives the
>> full URL or at least squelches the "You should define the parameter"
>> noise) in the output.
>
> openSUSE 11.2, for example. Its xmlto has a non-standard --xsltopts
> option that passes its argument down to xsltproc.
Ok, as I said that I've been wrong before in this area ;-)
I don't think I will have much time for git today, and it would be
appreciated if somebody can work on this and send a tested patch that
applies cleanly on top of 'maint' to implement the @@MAN_BASE_URL@@
replacement from manpage-base.xsl.in to manpage-base.xsl as Todd suggested
earlier.
^ 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