* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Martin Langhoff @ 2013-01-02 23:44 UTC (permalink / raw)
To: Eric Raymond; +Cc: Jonathan Nieder, Junio C Hamano, Git Mailing List
In-Reply-To: <20130102222849.GA21105@thyrsus.com>
On Wed, Jan 2, 2013 at 5:28 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Martin Langhoff <martin.langhoff@gmail.com>:
>> I dealt with enough CVS repos to see that the branch point could be
>> ambiguous, and that some cases were incurably ugly and ambiguous.
>
> You are quite right, but you have misintepreted the subject of my
> confidence. I am under no illusion that the new cvsimport/cvsps
> pair is a perfect solution to the CVS-lifting problem, nor even that
> such a solution is possible.
Thanks. That is a much more reassuring stance.
m
--
martin.langhoff@gmail.com
martin@laptop.org -- Software Architect - OLPC
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Torsten Bögershausen @ 2013-01-02 23:58 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, git
In-Reply-To: <20130102232239.GA27952@sigill.intra.peff.net>
On 03.01.13 00:22, Jeff King wrote:
> On Thu, Jan 03, 2013 at 12:14:32AM +0100, Torsten Bögershausen wrote:
>
>>> This is wrong if $(PERL_PATH) contains spaces, no? Doing "$(PERL_PATH)"
>>> is also wrong, because the expansion happens in 'make', and a
>>> $(PERL_PATH) with double-quotes would fool the shell. Since we export
>>> $PERL_PATH, I think doing:
>>>
>>> "$$PERL_PATH"" check-non-portable-shell.pl $(T)
>> Thanks, but:
>> - The double "" after PERL_PATH makes the string un-terminated.
>
> Yeah, sorry, typo on my part.
>
>> - Using "$$PERL_PATH" expands from make into "$PERL_PATH" on the command line
>
> Right. That's what I intended.
>
>> - If the Makefile looks like this:
>> PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
>> [snip]
>> $(PERL_PATH) check-non-portable-shell.pl $(T)
>> The command line will look like this:
>> "/Users/tb/projects/git/tb/pe rl" check-non-portable-shell.pl t0000-basic.sh ...
>>
>> So I think that PERL_PATH should be quoted when it is defined in the Makefile.
>
> Does a $PERL_PATH with quotes actually work?
At least on my system the following combination works:
git diff
diff --git a/t/Makefile b/t/Makefile
index f8f8c54..391a5ca 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -8,7 +8,7 @@
#GIT_TEST_OPTS = --verbose --debug
SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
+PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
~/projects/git/tb/t> ls -l "/Users/tb/projects/git/tb/pe rl"
lrwxr-xr-x 1 tb staff 13 Jan 3 00:33 /Users/tb/projects/git/tb/pe rl -> /usr/bin/perl
====================================================
And this works as well, is that what you intended?
Note: "single Dollar"
====================================================
git diff
diff --git a/t/Makefile b/t/Makefile
index f8f8c54..f624f95 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -8,7 +8,7 @@
#GIT_TEST_OPTS = --verbose --debug
SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
+PERL_PATH = /Users/tb/projects/git/tb/pe rl
TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
@@ -57,7 +57,7 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- $(PERL_PATH) check-non-portable-shell.pl $(T)
+ "$(PERL_PATH)" check-non-portable-shell.pl $(T)
> Usually in our runtime environment, commands that are handed to git are
> assumed to be passed directly to the shell, and you need to quote. E.g.,
> setting diff.external to:
>
> [diff]
> external = "foo --bar"
>
> will let the shell split the argument out; if you have a space, you
> would want to set it like:
>
> [diff]
> external = "'command with space'"
>
> This is the most flexible way to do it.
>
> However, for Makefile variables, I think we do not (and cannot) follow
> the same rule. Notice that all of the uses of $PERL_PATH in the test
> suite enclose it in quotes. Having extra quotes would break those
> invocations. And the value of $PERL_PATH will be put on the #!-line,
> which cannot not be quoted.
>
> -Peff
I followed these lines as an example:
test-results/git-smoke.tar.gz: test-results
$(PERL_PATH) ./harness \
--archive="test-results/git-smoke.tar.gz" \
$(T)
(and make smoke did not work, as we don't have ./harness :-(
Do we need some cleanup/improvements here as well?
/Torsten
^ permalink raw reply related
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-03 0:01 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, git
In-Reply-To: <20130102094635.GD9328@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Jan 01, 2013 at 10:40:08PM +0100, Torsten Bögershausen wrote:
>
>> Add the perl script "check-non-portable-shell.pl" to detect non-portable
>> shell syntax
>
> Cool. Thanks for adding more test-lint. But...
>
>> diff --git a/t/Makefile b/t/Makefile
>> index 88e289f..7b0c4dc 100644
>> --- a/t/Makefile
>> +++ b/t/Makefile
>> @@ -23,7 +23,7 @@ TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
>>
>> all: $(DEFAULT_TEST_TARGET)
>>
>> -test: pre-clean $(TEST_LINT)
>> +test: pre-clean test-lint-shell-syntax $(TEST_LINT)
>> $(MAKE) aggregate-results-and-cleanup
>
> I do not think it should be a hard-coded dependency of "test", as then
> there is no way to turn it off. It would make more sense to me to set a
> default TEST_LINT that includes it, but could be overridden by the user.
I would actually not add this to TEST_LINT by default, especially
when "duplicates" and "executable" that are much simpler and less
likely to hit false positives are not on by default.
At least, a change to add this to TEST_LINT by default must wait to
be merged to any integration branch until all the fix-up topics that
this test triggers in the current codebase graduate to the branch.
>> +test-lint-shell-syntax:
>> + $(PERL_PATH) check-non-portable-shell.pl $(T)
>
> This is wrong if $(PERL_PATH) contains spaces, no?
You are correct; "harness" thing in the same Makefile gets this
wrong, too. I think the right invocation is:
@'$(PERL_PATH_SQ)' check-non-portable.shell.pl $(T)
although I do not offhand know if that symbol is already exported by
the top-level Makefile.
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-03 0:08 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vtxqzyw0g.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I would actually not add this to TEST_LINT by default, especially
> when "duplicates" and "executable" that are much simpler and less
> likely to hit false positives are not on by default.
>
> At least, a change to add this to TEST_LINT by default must wait to
> be merged to any integration branch until all the fix-up topics that
> this test triggers in the current codebase graduate to the branch.
>
>>> +test-lint-shell-syntax:
>>> + $(PERL_PATH) check-non-portable-shell.pl $(T)
>>
>> This is wrong if $(PERL_PATH) contains spaces, no?
>
> You are correct; "harness" thing in the same Makefile gets this
> wrong, too. I think the right invocation is:
>
> @'$(PERL_PATH_SQ)' check-non-portable.shell.pl $(T)
>
> although I do not offhand know if that symbol is already exported by
> the top-level Makefile.
I'll tentatively queue this instead. The log message has also been
cleaned up a bit.
-- >8 --
From: Torsten Bögershausen <tboegi@web.de>
Date: Thu, 3 Jan 2013 00:20:19 +0100
Subject: [PATCH] test: Add check-non-portable-shell.pl
Add the perl script "check-non-portable-shell.pl" to detect
non-portable shell syntax.
"echo -n" is an example of a shell command working on Linux, but not
on Mac OS X.
These shell commands are checked and reported as error:
- "echo -n" (printf should be used)
- "sed -i" (GNUism; use a temp file instead)
- "declare" (bashism, often used with arrays)
- "which" (unreliable exit status and output; use type instead)
- "test a == b" (bashism for "test a = b")
"make test-lint-shell-syntax" can be used to run only the check.
Helped-By: Jeff King <peff@peff.net>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/Makefile | 8 ++++++--
t/check-non-portable-shell.pl | 27 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 2 deletions(-)
create mode 100755 t/check-non-portable-shell.pl
diff --git a/t/Makefile b/t/Makefile
index 3025418..a43becb 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -16,6 +16,7 @@ DEFAULT_TEST_TARGET ?= test
# Shell quote;
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
@@ -43,7 +44,7 @@ clean-except-prove-cache:
clean: clean-except-prove-cache
$(RM) .prove
-test-lint: test-lint-duplicates test-lint-executable
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
test-lint-duplicates:
@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -55,6 +56,9 @@ test-lint-executable:
test -z "$$bad" || { \
echo >&2 "non-executable tests:" $$bad; exit 1; }
+test-lint-shell-syntax:
+ @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T)
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean
@@ -87,7 +91,7 @@ test-results:
mkdir -p test-results
test-results/git-smoke.tar.gz: test-results
- $(PERL_PATH) ./harness \
+ '$(PERL_PATH_SQ)' ./harness \
--archive="test-results/git-smoke.tar.gz" \
$(T)
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
new file mode 100755
index 0000000..8b5a71d
--- /dev/null
+++ b/t/check-non-portable-shell.pl
@@ -0,0 +1,27 @@
+#!/usr/bin/perl
+
+# Test t0000..t9999.sh for non portable shell scripts
+# This script can be called with one or more filenames as parameters
+
+use strict;
+use warnings;
+
+my $exit_code=0;
+
+sub err {
+ my $msg = shift;
+ print "$ARGV:$.: error: $msg: $_\n";
+ $exit_code = 1;
+}
+
+while (<>) {
+ chomp;
+ /^\s*sed\s+-i/ and err 'sed -i is not portable';
+ /^\s*echo\s+-n/ and err 'echo -n is not portable (please use printf)';
+ /^\s*declare\s+/ and err 'arrays/declare not portable';
+ /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
+ /test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+ # this resets our $. for each file
+ close ARGV if eof;
+}
+exit $exit_code;
--
1.8.1.203.gc241474
^ permalink raw reply related
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-03 0:16 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jeff King, git
In-Reply-To: <50E4C9B5.8070308@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> At least on my system the following combination works:
>
> git diff
> diff --git a/t/Makefile b/t/Makefile
> index f8f8c54..391a5ca 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -8,7 +8,7 @@
>
> #GIT_TEST_OPTS = --verbose --debug
> SHELL_PATH ?= $(SHELL)
> -PERL_PATH ?= /usr/bin/perl
> +PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
I do not think that will fly. Having that in the main Makefile
where the existing users of the symbol relies on it without any
surrounding quotes, e.g.
$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
sed -e '1{' \
-e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-e ' h' \
-e ' s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
-e ' H' \
-e ' x' \
-e '}' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
$@.perl >$@+ && \
chmod +x $@+ && \
mv $@+ $@
where $(PERL_PATH_SQ) is defined to replace each ' in $(PERL_PATH)
with '\'' so that '$(PERL_PATH_SQ)' becomes a shell-safe way to
quote the value of PERL_PATH without quotes, your definition will
look for a relative path that is inside a directory named '"'
(that's a single double-quote).
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Torsten Bögershausen @ 2013-01-03 0:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Jeff King, git
In-Reply-To: <7vlicbyvc2.fsf@alter.siamese.dyndns.org>
On 03.01.13 01:16, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> At least on my system the following combination works:
>>
>> git diff
>> diff --git a/t/Makefile b/t/Makefile
>> index f8f8c54..391a5ca 100644
>> --- a/t/Makefile
>> +++ b/t/Makefile
>> @@ -8,7 +8,7 @@
>>
>> #GIT_TEST_OPTS = --verbose --debug
>> SHELL_PATH ?= $(SHELL)
>> -PERL_PATH ?= /usr/bin/perl
>> +PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
>
> I do not think that will fly. Having that in the main Makefile
> where the existing users of the symbol relies on it without any
> surrounding quotes, e.g.
>
> $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
> $(QUIET_GEN)$(RM) $@ $@+ && \
> INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
> sed -e '1{' \
> -e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
> -e ' h' \
> -e ' s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
> -e ' H' \
> -e ' x' \
> -e '}' \
> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> $@.perl >$@+ && \
> chmod +x $@+ && \
> mv $@+ $@
>
> where $(PERL_PATH_SQ) is defined to replace each ' in $(PERL_PATH)
> with '\'' so that '$(PERL_PATH_SQ)' becomes a shell-safe way to
> quote the value of PERL_PATH without quotes, your definition will
> look for a relative path that is inside a directory named '"'
> (that's a single double-quote).
Thanks to all for the explanations, fixing up and queing.
And good news:
pu today is "clean",there where no problems found:
commit d69ea46220647c048d332c471a184446cce17627
Merge: e552539 fcf30b3
Author: Junio C Hamano <gitster@pobox.com>
Date: Wed Jan 2 12:44:33 2013 -0800
When the dust has settled, we can either enable the check always, or mention
"make test-lint-shell-syntax" in the Documentation.
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Greg Troxel @ 2013-01-03 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2xn18p5.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> [query about NetBSD-6]
> The "2.7" bit certainly looks fishy, as users should be able to
> choose between "2.6" and "2.7" (and possibly "3.0"), IIUC.
>
> + PYTHON_PATH = /usr/pkg/bin/python2.7
> + PERL_PATH = /usr/pkg/bin/perl
(I am one of the people who maintain the git package in pkgsrc.)
(Strictly, this is not really about NetBSD, but about all systems where
the standard approach to get python is via pkgsrc. So that include
DragonflyBSD as well. (pkgsrc runs on many other systems, but it isn't
the standard approach, so from the git viewpoint that's irrelevant.))
You are entirely right that on e.g. NetBSD 6 the view is that users
should be able to choose the python version.
pkgsrc can install multiple versions of python at the same time, to cope
with python-using packages that need different versions. pkgsrc chooses
not to have a 'python' program, because that would result in installed
packages changing their binding of which python version to use when the
default was changed. The default python version is currently 2.7, so
/usr/pkg/bin/python2.7 is the best guess for finding python on a NetBSD
system, if you're only allowed one guess. A user can set a
PYTHON_VERSION_DEFAULT variable to choose the version they want; each
package expresses which versions will work.
This isn't relevant for git, not being a pure python library, but pkgsrc
supports installing multiple versions of some packages, so one can have
two versions installed at once:
py27-expat-0nb6 Python interface to expat
py26-expat-0nb6 Python interface to expat
The git package just depends on one version; by default the git package
depends on python (but one can tell it not to).
The python.m4 macro that comes with automake seems to find one of the
various pythonX.Y binaries in $PATH just fine.
pkgsrc has an entry for git (at 1.8.0.1).
The key line for handling python is:
MAKE_FLAGS+= PYTHON_PATH=${PYTHONBIN}
and there PYTHONBIN is set up by pkgsrc infrastructure for the right
prefix (99.9% but not always /usr/pkg) and version. After this,
everything seems to come out right:
> head -1 /usr/pkg/libexec/git-core/git-p4
#!/usr/pkg/bin/python2.7
So I'd say that if PYTHON_PATH is set in the environment to configure,
it should behave as it does now. And if not, it would be nice if the
highest pythonX.Y found (that is known to work with git) is used.
> + PYTHON_PATH = /usr/pkg/bin/python2.7
> + PERL_PATH = /usr/pkg/bin/perl
So it would be nice to make these work as ?=, letting an environment
variable win if set.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Greg Troxel @ 2013-01-03 0:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2xn18p5.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> I would appreciate if somebody with more familiarlity with the
> platform can suggest a better alternative than applying the
> following patch to our Makefile. Right now I have an equivalent of
> this change in config.mak locally when building on the said
> platform.
I realized after sending my previous reply that you are probably trying
to have a way to build and run tests on NetBSD-6 from a git checkout as
part of development testing.
One approach I've taken with other programs is to have a README.NetBSD
file which is actually an executable /bin/sh script with comments,
explaining the prereqs in terms of pkgsrc and invoking configure to get
dependencies from pkgsrc (-I/usr/pkg/include plus -L/-R).
So in the git case, this could set PYTHON_PATH in the environment.
I realize a README.foo file for N different systems could be clutter,
but having these checked in would provide the concise help that people
on any of those platforms need.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-03 2:02 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jeff King, git
In-Reply-To: <50E4CF7E.9090302@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> When the dust has settled, we can either enable the check always,
> or mention "make test-lint-shell-syntax" in the Documentation.
In the longer term, I'm pretty much in favor of enabling all the
checks that are cheap by default, as that would help people catch
easy mistakes while preparing their patches. People do not tend to
enable any check if it were optional.
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Junio C Hamano @ 2013-01-03 2:15 UTC (permalink / raw)
To: Greg Troxel; +Cc: git
In-Reply-To: <rmipq1numzj.fsf@fnord.ir.bbn.com>
Greg Troxel <gdt@ir.bbn.com> writes:
> I realize a README.foo file for N different systems could be clutter,
> but having these checked in would provide the concise help that people
> on any of those platforms need.
Our Makefile documents knobs people on various platforms can tweak
(PYTHON_PATH and OLD_ICONV are two examples of them), sets default
values for them based on $(uname -S), then includes config.mak file
the user can optionally create to override these platform defaults.
This infrastructure is used across platforms, not just for NetBSD.
The part shown in the patch was to update the platform default for
NetBSD. The setting we have been shipping in our Makefile seemed to
be different from what I needed on my NetBSD 6 install, and I was
wondering if we have no real users of Git on the platorm (which
would explain why we didn't get any complaints or patches to update
this part). Or there are some optional packages all real NetBSD
users install, but being a NetBSD newbie I didn't, that makes the
values I showed in the patch inappropriate for them (e.g. Perhaps
there is a mechanism other than pkgsrc that installs perl and python
under /usr/bin? Perhaps an optional libi18n package that gives
iconv(3) with new function signature?), in which case I definitely
should not apply that patch to my tree, as it would only be an
improvement for one person and a regression for existing users at
the same time.
^ permalink raw reply
* Re: [PATCH V2] t9810: Do not use sed -i
From: Pete Wyckoff @ 2013-01-03 3:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vhamz18y3.fsf@alter.siamese.dyndns.org>
gitster@pobox.com wrote on Wed, 02 Jan 2013 15:06 -0800:
> Pete Wyckoff <pw@padd.com> writes:
>
> > tboegi@web.de wrote on Wed, 02 Jan 2013 00:20 +0100:
> >> sed -i is not portable on all systems.
> >> Use sed with different input and output files.
> >> Utilize a tmp file whenever needed
> >>
> >> Added missing && at 2 places
> >>
> >> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> >
> > One more teensy fix is needed in this hunk, following Junio's
> > comment about redirections going at the end:
>
> I've already pushed out the previous one on 'pu' after fixing it up
> like this.
>
> Thanks. Anything I missed?
I should have guessed that you would just silently fix it.
Thanks!
-- Pete
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Chris Rorvick @ 2013-01-03 6:34 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130101172645.GA5506@thyrsus.com>
On Tue, Jan 1, 2013 at 11:26 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> diff --git a/git-cvsimport.py b/git-cvsimport.py
> new file mode 100755
> index 0000000..6407e8a
> --- /dev/null
> +++ b/git-cvsimport.py
> @@ -0,0 +1,342 @@
> +#!/usr/bin/env python
> +#
> +# Import CVS history into git
> +#
> +# Intended to be a near-workalike of Matthias Urlichs's Perl implementation.
> +#
> +# By Eric S. Raymond <esr@thyrsus.com>, December 2012
> +# May be redistributed under the license of the git project.
> +
> +import sys
> +
> +if sys.hexversion < 0x02060000:
> + sys.stderr.write("git cvsimport: requires Python 2.6 or later.\n")
> + sys.exit(1)
> +
> +import os, getopt, subprocess, tempfile, shutil
> +
> +DEBUG_COMMANDS = 1
> +
> +class Fatal(Exception):
> + "Unrecoverable error."
> + def __init__(self, msg):
> + Exception.__init__(self)
> + self.msg = msg
> +
> +def do_or_die(dcmd, legend=""):
> + "Either execute a command or raise a fatal exception."
> + if legend:
> + legend = " " + legend
> + if verbose >= DEBUG_COMMANDS:
> + sys.stdout.write("git cvsimport: executing '%s'%s\n" % (dcmd, legend))
> + try:
> + retcode = subprocess.call(dcmd, shell=True)
> + if retcode < 0:
> + raise Fatal("git cvsimport: child was terminated by signal %d." % -retcode)
> + elif retcode != 0:
> + raise Fatal("git cvsimport: child returned %d." % retcode)
> + except (OSError, IOError) as e:
> + raise Fatal("git cvsimport: execution of %s%s failed: %s" % (dcmd, legend, e))
> +
> +def capture_or_die(dcmd, legend=""):
> + "Either execute a command and capture its output or die."
> + if legend:
> + legend = " " + legend
> + if verbose >= DEBUG_COMMANDS:
> + sys.stdout.write("git cvsimport: executing '%s'%s\n" % (dcmd, legend))
> + try:
> + return subprocess.check_output(dcmd, shell=True)
> + except subprocess.CalledProcessError as e:
> + if e.returncode < 0:
> + sys.stderr.write("git cvsimport: child was terminated by signal %d." % -e.returncode)
> + elif e.returncode != 0:
> + sys.stderr.write("git cvsimport: child returned %d." % e.returncode)
> + sys.exit(1)
> +
> +class cvsps:
> + "Method class for cvsps back end."
> + def __init__(self):
> + self.opts = ""
> + self.revmap = None
> + def set_repo(self, val):
> + "Set the repository root option."
> + if not val.startswith(":"):
> + if not val.startswith(os.sep):
> + val = os.path.abspath(val)
> + val = ":local:" + val
> + self.opts += " --root '%s'" % val
> + def set_authormap(self, val):
> + "Set the author-map file."
> + self.opts += " -A '%s'" % val
> + def set_fuzz(self, val):
> + "Set the commit-similarity window."
> + self.opts += " -z %s" % val
> + def set_nokeywords(self):
> + "Suppress CVS keyword expansion."
> + self.opts += " -k"
> + def add_opts(self, val):
> + "Add options to the engine command line."
> + self.opts += " " + val
> + def set_exclusion(self, val):
> + "Set a file exclusion regexp."
> + self.opts += " -n -f '%s'" % val
> + def set_after(self, val):
> + "Set a date threshold for incremental import."
> + self.opts += " -d '%s'" % val
> + def set_revmap(self, val):
> + "Set the file to which the engine should dump a reference map."
> + self.revmap = val
> + self.opts += " -R '%s'" % self.revmap
> + def set_module(self, val):
> + "Set the module to query."
> + self.opts += " " + val
> + def command(self):
> + "Emit the command implied by all previous options."
> + return "cvsps --fast-export " + self.opts
> +
> +class cvs2git:
> + "Method class for cvs2git back end."
> + def __init__(self):
> + self.opts = ""
> + self.modulepath = "."
> + def set_authormap(self, _val):
> + "Set the author-map file."
> + sys.stderr.write("git cvsimport: author maping is not supported with cvs2git.\n")
> + sys.exit(1)
> + def set_repo(self, _val):
> + "Set the repository root option."
> + sys.stderr.write("git cvsimport: cvs2git must run within a repository checkout directory.\n")
> + sys.exit(1)
> + def set_fuzz(self, _val):
> + "Set the commit-similarity window."
> + sys.stderr.write("git cvsimport: fuzz setting is not supported with cvs2git.\n")
> + sys.exit(1)
> + def set_nokeywords(self):
> + "Suppress CVS keyword expansion."
> + self.opts += " --keywords-off"
> + def add_opts(self, val):
> + "Add options to the engine command line."
> + self.opts += " " + val
> + def set_exclusion(self, val):
> + "Set a file exclusion regexp."
> + self.opts += " --exclude='%s'" % val
> + def set_after(self, _val):
> + "Set a date threshold for incremental import."
> + sys.stderr.write("git cvsimport: incremental import is not supported with cvs2git.\n")
> + sys.exit(1)
> + def set_revmap(self, _val):
> + "Set the file to which the engine should dump a reference map."
> + sys.stderr.write("git cvsimport: can't get a reference map from cvs2git.\n")
> + sys.exit(1)
> + def set_module(self, val):
> + "Set the module to query."
> + self.modulepath = " " + val
> + def command(self):
> + "Emit the command implied by all previous options."
> + return "(cvs2git --username=git-cvsimport --quiet --quiet --blobfile={0} --dumpfile={1} {2} {3} && cat {0} {1} && rm {0} {1})".format(tempfile.mkstemp()[1], tempfile.mkstemp()[1], self.opts, self.modulepath)
> +
> +class filesource:
> + "Method class for file-source back end."
> + def __init__(self, filename):
> + self.filename = filename
> + def __complain(self, legend):
> + sys.stderr.write("git cvsimport: %s with file source.\n" % legend)
> + sys.exit(1)
> + def set_repo(self, _val):
> + "Set the repository root option."
> + self.__complain("repository can't be set")
> + def set_authormap(self, _val):
> + "Set the author-map file."
> + sys.stderr.write("git cvsimport: author maping is not supported with filesource.\n")
> + sys.exit(1)
> + def set_fuzz(self, _val):
> + "Set the commit-similarity window."
> + self.__complain("fuzz can't be set")
> + def set_nokeywords(self, _val):
> + "Suppress CVS keyword expansion."
> + self.__complain("keyword suppression can't be set")
> + def add_opts(self, _val):
> + "Add options to the engine command line."
> + self.__complain("other options can't be set")
> + def set_exclusion(self, _val):
> + "Set a file exclusion regexp."
> + self.__complain("exclusions can't be set")
> + def set_after(self, _val):
> + "Set a date threshold for incremental import."
> + pass
> + def set_revmap(self, _val):
> + "Set the file to which the engine should dump a reference map."
> + sys.stderr.write("git cvsimport: can't get a reference map from cvs2git.\n")
> + sys.exit(1)
> + def set_module(self, _val):
> + "Set the module to query."
> + self.__complain("module can't be set")
> + def command(self):
> + "Emit the command implied by all previous options."
> + return "cat " + self.filename
> +
> +if __name__ == '__main__':
> + if sys.hexversion < 0x02060000:
> + sys.stderr.write("git cvsimport: requires Python 2.6 or later.\n")
> + sys.exit(1)
> + (options, arguments) = getopt.getopt(sys.argv[1:], "vbe:d:C:r:o:ikus:p:z:P:S:aL:A:Rh")
> + verbose = 0
> + bare = False
> + root = None
> + outdir = os.getcwd()
> + remotize = False
> + import_only = False
> + underscore_to_dot = False
> + slashsubst = None
> + authormap = None
> + revisionmap = False
> + backend = cvsps()
> + for (opt, val) in options:
> + if opt == '-v':
> + verbose += 1
> + elif opt == '-b':
> + bare = True
> + elif opt == '-e':
> + for cls in (cvsps, cvs2git):
> + if cls.__name__ == val:
> + backend = cls()
> + break
> + else:
> + sys.stderr.write("git cvsimport: unknown engine %s.\n" % val)
> + sys.exit(1)
> + elif opt == '-d':
> + backend.set_repo(val)
> + elif opt == '-C':
> + outdir = val
> + elif opt == '-r':
> + remotize = True
> + elif opt == '-o':
> + sys.stderr.write("git cvsimport: -o is no longer supported.\n")
> + sys.exit(1)
> + elif opt == '-i':
> + import_only = True
> + elif opt == '-k':
> + backend.set_nokeywords()
> + elif opt == '-u':
> + underscore_to_dot = True
> + elif opt == '-s':
> + slashsubst = val
> + elif opt == '-p':
> + backend.add_opts(val.replace(",", " "))
> + elif opt == '-z':
> + backend.set_fuzz(val)
> + elif opt == '-P':
> + backend = filesource(val)
> + sys.exit(1)
> + elif opt in ('-m', '-M'):
> + sys.stderr.write("git cvsimport: -m and -M are no longer supported: use reposurgeon instead.\n")
> + sys.exit(1)
> + elif opt == '-S':
> + backend.set_exclusion(val)
> + elif opt == '-a':
> + sys.stderr.write("git cvsimport: -a is no longer supported.\n")
> + sys.exit(1)
> + elif opt == '-L':
> + sys.stderr.write("git cvsimport: -L is no longer supported.\n")
> + sys.exit(1)
> + elif opt == '-A':
> + authormap = os.path.abspath(val)
> + elif opt == '-R':
> + revisionmap = True
> + else:
> + print """\
> +git cvsimport [-A <author-conv-file>] [-C <git_repository>] [-b] [-d <CVSROOT>]
> + [-e engine] [-h] [-i] [-k] [-p <options-for-cvsps>] [-P <source-file>]
> + [-r <remote>] [-R] [-s <subst>] [-S <regex>] [-u] [-v] [-z <fuzz>]
> + [<CVS_module>]
> +"""
> +
> + def metadata(fn):
> + if bare:
> + return fn
> + else:
> + return os.path.join(".git", fn)
> + try:
> + if outdir:
> + try:
> + # If the output directory does not exist, create it
> + # and initialize it as a git repository.
> + os.mkdir(outdir)
> + do_or_die("git init --quiet " + outdir)
> + except OSError:
> + # Otherwise, assume user wants incremental import.
> + if not bare and not os.path.exists(os.path.join(outdir, ".git")):
> + raise Fatal("output directory is not a git repository")
> + threshold = capture_or_die("git log -1 --format=%ct").strip()
> + backend.set_after(threshold)
> + if revisionmap:
> + backend.set_revmap(tempfile.mkstemp()[1])
> + markmap = tempfile.mkstemp()[1]
> + if arguments:
> + backend.set_module(arguments[0])
> + gitopts = ""
> + if bare:
> + gitopts += " --bare"
> + if revisionmap:
> + gitopts += " --export-marks='%s'" % markmap
> + if authormap:
> + shutil.copyfile(authormap, metadata("cvs_authors"))
> + if os.path.exists(metadata("cvs-authors")):
> + backend.set_authormap(metadata("cvs-authors"))
> + do_or_die("%s | (cd %s >/dev/null; git fast-import --quiet %s)" \
> + % (backend.command(), outdir, gitopts))
outdir needs to be quoted in the formatted string, i.e.:
"%s | (cd '%s' >/dev/null ..."
Also, I noticed the generated cvs-revisions file now maps cvs
revisions to blobs instead of commits. Was this change intentional?
Thanks,
Chris
^ permalink raw reply
* [RFD] annnotating a pair of commit objects?
From: Junio C Hamano @ 2013-01-03 7:03 UTC (permalink / raw)
To: git
I'd like a datastore that maps a pair of commit object names to
another object name, such that:
* When looking at two commits A and B, efficiently query all data
associated with a pair of commits <X,Y> where X is contained in
the range A..B and not in B..A, and Y is contained in the range
B..A and not in A..B.
* When <X,Y> is registered in the datastore, and X is rewritten to
X' and/or Y is rewritten to Y', the mapping is updated so that it
can be queried with <X',Y'> as a new key, similar to the way a
notes tree that maps object X can be updated to map object X'
when such a rewrite happens.
The intended use case is to "go beyond rerere". Given a history of
this shape:
o---o---o---I mainline
/
O---o---X---o---A topic A
\
o---Y---o---o---B topic B
Suppose in the original O we had a function "distimmed_doshes()" to
tell if doshes are already distimmed, with some call sites. On the
branch leading to A, at commit X, this function was renamed to
"doshes_are_distimmed()", and all existing call sites were adjusted.
On the side branch leading to B, however, at commit Y, a new call
site to the old function was added in a file that was not touched
between O..A at all.
When merging either the topic A or the topic B (but not both) to the
integration branch that did not touch this function or use of it, no
special care needs to be taken, but when merging the second topic
after merging the other one, we need to resolve a semantic conflict.
Namely, the callsite to "distimmed_doshes()" introduced by commit Y
needs to be adjusted to call "doshes_are_distimmed()" instead.
The first step is to recognize the potential issue. When queuing
the topic that contains X and the other topic that contains Y,
suppose I could register <X,Y> to the datastore I am dreaming. When
I merge A to the integration branch, I can notice that there is no
such pair <M,N> in the datastore that:
* M is in A..I and not in I..A
* N is in I..A and not in A..I
and can create a merge J without semantic adjustment.
o---o---o---I---J mainline
/ /
O---o---X---o---A topic A
\
o---Y---o---o---B topic B
When I later merge topic B to the integration branch, however, there
is <X,Y> in the datastore such that:
* X is in B..J and not in J..B
* Y is in J..B and not in B..J
to notice that we need to be careful when creating the merge K:
o---o---o---I---J---K mainline
/ / /
O---o---X---o---A / topic A
\ /
o---Y---o---o---B topic B
Of course, the next step is to store not just one bit "<X,Y> exists
in the datastore--be careful", but what semantic adjustment needs to
be applied [*1*]
Obviously, with O(cnt(A..B))*O(cnt(B..A)) complexity, this can be
trivially coded, by trying all pairs in symmetric difference.
But I am hoping we can do better than that.
Ideas?
[Footnote]
*1* We could do it in multiple ways and the details are not
interesting. A blob object that records a patch that can be applied
with "git apply -3", or a commit object that represents necessary
"evil" change that can be cherry-pick'ed, would be two possible
implementations.
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Junio C Hamano @ 2013-01-03 7:08 UTC (permalink / raw)
To: Chris Rorvick; +Cc: esr, git
In-Reply-To: <CAEUsAPYwinmbDkSVu71WJRgUjLfBeNdKDFt6O1f8-Ti9evn6Hw@mail.gmail.com>
Chris Rorvick <chris@rorvick.com> writes:
> outdir needs to be quoted in the formatted string, i.e.:
>
> "%s | (cd '%s' >/dev/null ..."
The issue is real, but I am afraid that the above is not sufficient
because outdir can contain single quotes. I think other places that
call out to external processes share the same issue of being careless
about quoting in general.
Doesn't Python come with a standard subprocess module that lets you
spawn external programs safely, similar to the way Perl's list form
open(), e.g. "open($fh, "-|", 'git', @args)", works?
^ permalink raw reply
* [PATCH] tests: turn on test-lint by default
From: Jeff King @ 2013-01-03 7:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vhamzyqev.fsf@alter.siamese.dyndns.org>
On Wed, Jan 02, 2013 at 06:02:48PM -0800, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
> > When the dust has settled, we can either enable the check always,
> > or mention "make test-lint-shell-syntax" in the Documentation.
>
> In the longer term, I'm pretty much in favor of enabling all the
> checks that are cheap by default, as that would help people catch
> easy mistakes while preparing their patches. People do not tend to
> enable any check if it were optional.
That is fine with me, and I always intended that we turn the lint on by
default at some point (I'm not really sure why we didn't -- looking at
the list archives, I think I did not push it because it seemed like
nobody was really that interested).
Certainly the two existing checks are cheap and do not produce false
positives, and should be safe to turn on. Like this:
-- >8 --
Subject: [PATCH] tests: turn on test-lint by default
The test Makefile knows about a few "lint" checks for common
errors. However, they are not enabled as part of "make test"
by default, which means that many people do not bother
running them. Since they are both quick to run and accurate
(i.e., no false positives), there should be no harm in
turning them on and helping submitters catch errors earlier.
We could just set:
TEST_LINT = test-lint
to enable all tests. But that would be unnecessarily
annoying later on if we add slower or less accurate tests
that should not be part of the default. Instead, we name the
tests individually.
Signed-off-by: Jeff King <peff@peff.net>
---
t/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/Makefile b/t/Makefile
index 3025418..5c6de81 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -13,6 +13,7 @@ DEFAULT_TEST_TARGET ?= test
RM ?= rm -f
PROVE ?= prove
DEFAULT_TEST_TARGET ?= test
+TEST_LINT ?= test-lint-duplicates test-lint-executable
# Shell quote;
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
--
1.8.1.rc3.4.gf3a2f57
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Antoine Pelisse @ 2013-01-03 7:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Chris Rorvick, Eric Raymond, git
In-Reply-To: <7vmwwqyc8w.fsf@alter.siamese.dyndns.org>
> Doesn't Python come with a standard subprocess module that lets you
> spawn external programs safely, similar to the way Perl's list form
> open(), e.g. "open($fh, "-|", 'git', @args)", works?
You mean something like this:
p1 = subprocess.Popen([backend.command()], stdout=subprocess.PIPE)
subprocess.Popen(["git", "fast-import", "--quiet"] + gitopts,
cwd=outdir, stdin=p1.stdout)
Assuming gitopts is a list rather than a string. (care must be taken
with backend.command() also)
^ permalink raw reply
* Re: [RFD] annnotating a pair of commit objects?
From: Jeff King @ 2013-01-03 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4m2ycij.fsf@alter.siamese.dyndns.org>
On Wed, Jan 02, 2013 at 11:03:00PM -0800, Junio C Hamano wrote:
> I'd like a datastore that maps a pair of commit object names to
> another object name, such that:
>
> * When looking at two commits A and B, efficiently query all data
> associated with a pair of commits <X,Y> where X is contained in
> the range A..B and not in B..A, and Y is contained in the range
> B..A and not in A..B.
> [...]
> Obviously, with O(cnt(A..B))*O(cnt(B..A)) complexity, this can be
> trivially coded, by trying all pairs in symmetric difference.
>
> But I am hoping we can do better than that.
>
> Ideas?
Just thinking out loud, the problem can be generalized in math terms as:
- you have a universe of elements, `U` (i.e., all commits)
- you have two sets, `X` and `Y`, such that each is a subset of `U`
(these correspond to the two sides of the range, but we can think of
them just as sets of commits). We happen to know that the sets are
disjoint, but I don't know if that is helpful here.
- you have a set of sets `M` that is a subset of the cartesian product
`U x U` (i.e., its elements are "{x,y}" pairs, and membership in
that set is your "bit"; you could also think of it as a mapping if
you wanted more than a bit).
- you want to know the intersection of `X x Y` and `M` (which of your
pairs are in the mapping set).
Without doing any careful analysis, my gut says that in the worst case,
you are going to be stuck with `O(|X|*|Y|)` (i.e., what you are trying
to do better than above). But if we assume that `M` is relatively sparse
(which it should be; you only create entries when you do a merge between
two commits, and even then, only when it is tricky), we can probably do
better in practice.
For example, consider this naive way of doing it. Store `M` as a mapping
of commits to sets of commits, with fast lookup (a hash, or sorted
list). For each element of `X`, look it up in `M`; call its value `V`
(which, remember, is a set itself). For each element of `Y`, look it up
in `V`. The complexity would be:
O(|X| * lg(|M|) * |Y| * lg(V_avg))
where "V_avg" is the average cardinality of each of the value sets we
map in the first step. But imagine we pre-sort `Y`, and then in the
second step, rather than looking up each `Y` in `V`, we instead look up
each `V` in `Y`. Then we have:
O(|X| * lg(|M|) * V_avg * lg(|Y|))
IOW, we get to apply the log to |Y|. This is a win if we expect that
V_avg is going to be much smaller than |Y|. Which I think it would be,
since we would only have entries for merges we've done before[1].
That's just off the top of my head. This seems like it should be a
classic databases problem (since the cartesian product here is really
just a binary relation), but I couldn't find anything obvious online
(and I never paid attention in class, either).
-Peff
[1] You can do the same inversion trick for looking up elements of `M`
in `X` instead of vice versa. It would probably buy you less, as you
have a lot of commits that have merges at all (i.e., `M` is big),
but only a few matching partners for each entry (i.e., `V` is
small).
^ permalink raw reply
* Re: [BUG] two-way read-tree can write null sha1s into index
From: Jeff King @ 2013-01-03 8:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git
In-Reply-To: <7vvcbg7d8x.fsf@alter.siamese.dyndns.org>
On Tue, Jan 01, 2013 at 02:24:46PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > So I think we need to update twoway_merge to recognize unmerged entries,
> > which gives us two options:
> >
> > 1. Reject the merge.
> >
> > 2. Throw away the current unmerged entry in favor of the "new" entry
> > (when old and new are the same, of course; otherwise we would
> > reject).
> >
> > I think (2) is the right thing.
>
> As "--reset" in "read-tree --reset -u A B" is a way to say "we know
> we are based on A and we want to go to B no matter what", I agree we
> should not reject the merge.
>
> With -m instead of --reset, I am not sure what the right semantics
> should be, though.
Good point; I was just thinking about the --reset case.
With "-m", though, we could in theory carry over the unmerged entries
(again, assuming that "old" and "new" are the same; otherwise it is an
obvious reject). But those entries would be confused with any new
unmerged entries we create. It seems we already protect against this,
though: "read-tree -m" will not run at all if you have unmerged entries.
Likewise, "checkout" seems to have similar protections.
So I think it may be a non-issue.
-Peff
^ permalink raw reply
* Re: git filter-branch doesn't dereference annotated tags
From: Johannes Sixt @ 2013-01-03 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Grégory Pakosz, git
In-Reply-To: <7v623f18ci.fsf@alter.siamese.dyndns.org>
Am 03.01.2013 00:19, schrieb Junio C Hamano:
> Grégory Pakosz <gpakosz@visionobjects.com> writes:
>
>> So we have an annotated tag that points to a commit that is rewritten
>> to nothing as the result of the filtering. What should happen?
>
> If the user asked to filter that tag itself, it may make sense to
> remove it, rather than keeping it pointing at the original commit,
> because the commit it used to point at no longer exists in the
> alternate history being created by filter-branch.
IOW, if the command was something like
git filter-branch ...filter options... -- v1.0 master ...
and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
deleted if the commit it points to goes away. But if the commit did not
go away, but was rewritten, then it is equally reasonable to expect that
the tag is also rewritten. But I don't think that we currently do the
latter.
Therefore, IMO, a change that implements the former behavior should also
implement the latter behavior.
-- Hannes
^ permalink raw reply
* Re: git filter-branch doesn't dereference annotated tags
From: Grégory Pakosz @ 2013-01-03 9:50 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <50E55198.5080202@kdbg.org>
>
> IOW, if the command was something like
>
> git filter-branch ...filter options... -- v1.0 master ...
>
> and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
> deleted if the commit it points to goes away. But if the commit did not
> go away, but was rewritten, then it is equally reasonable to expect that
> the tag is also rewritten. But I don't think that we currently do the
> latter.
>
When the commit doesn't go away, the tag is currently being rewritten properly.
> Therefore, IMO, a change that implements the former behavior should also
> implement the latter behavior.
>
The patch in my latest email does both. (yet lacks unit tests for now)
Gregory
^ permalink raw reply
* Re: [RFD] annnotating a pair of commit objects?
From: Michael Haggerty @ 2013-01-03 9:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vr4m2ycij.fsf@alter.siamese.dyndns.org>
On 01/03/2013 08:03 AM, Junio C Hamano wrote:
> I'd like a datastore that maps a pair of commit object names to
> another object name, such that:
>
> * When looking at two commits A and B, efficiently query all data
> associated with a pair of commits <X,Y> where X is contained in
> the range A..B and not in B..A, and Y is contained in the range
> B..A and not in A..B.
>
> * When <X,Y> is registered in the datastore, and X is rewritten to
> X' and/or Y is rewritten to Y', the mapping is updated so that it
> can be queried with <X',Y'> as a new key, similar to the way a
> notes tree that maps object X can be updated to map object X'
> when such a rewrite happens.
>
> The intended use case is to "go beyond rerere". Given a history of
> this shape:
>
> o---o---o---I mainline
> /
> O---o---X---o---A topic A
> \
> o---Y---o---o---B topic B
If we ignore rewriting for a moment, the information that you want to
record is essentially the merge M of X and Y, no? Namely, X and Y
conflict logically with each other (though perhaps not textually) and
you, the human, want to record how to reconcile them:
o---o---o---I mainline
/
O---o---X---o---A topic A
\ \
\ M
\ /
o---Y---o---o---B topic B
However, you don't necessarily want to go to the trouble to make a
branch to point at M, nor to do the bookkeeping manually that would be
required to take the information stored in M into account when merging A
and B later.
Suppose we had M; how could we make use of it in future merges?
> [...] and can create a merge J without semantic adjustment.
>
> o---o---o---I---J mainline
> / /
> O---o---X---o---A topic A
> \
> o---Y---o---o---B topic B
That would become
o---o---o---I---J mainline
/ /
O---o---X---o---A topic A
\ \
\ M
\ /
o---Y---o---o---B topic B
> When I later merge topic B to the integration branch, however, [...]
> to notice that we need to be careful when creating the merge K:
>
> o---o---o---I---J---K mainline
> / / /
> O---o---X---o---A / topic A
> \ /
> o---Y---o---o---B topic B
When doing this merge, I think your goal is equivalent to discovering
that M includes part of the merge of J and B, and adding M as an
(implicit or explicit) third parent to the merge:
o---o---o---I---J-------K mainline
/ / . /
O---o---X---o---A . / topic A
\ \ . /
\ M......... /
\ / /
o---Y---o---o---B topic B
How could M be stored? Assuming that these type of premerge merges are
sparse, then Jeff's analysis seems good. Concretely, one could simply
store pointers to M from both X and Y; e.g.,
* Add a note to X with the information "when merging this commit with Y,
use premerge M"
* Add a note to Y with the information "when merging this commit with X,
use premerge M"
Then, when merging, create the set J..B, scan all of the commits on B..J
for these "premerge" notes (O(|B..J|)), and for each one, look in the
set J..B to see if it is present. The effort should scale like
O( |J..B| + |B..J| * lg(|J..B|) )
where, of course J and B could be exchanged for either aesthetic or
performance reasons. (One would also need a mechanism for preventing M
from being garbage-collected.)
Incidentally, this is just the sort of thing I have been thinking about
using to implement a kind of "incremental merge"; I've started writing
up my thoughts on my blog [1,2,3] (including how to make pretty pictures
of merge conflicts).
Michael
[1]
http://softwareswirl.blogspot.de/2012/12/the-conflict-frontier-of-nightmare-merge.html
[2]
http://softwareswirl.blogspot.de/2012/12/mapping-merge-conflict-frontier.html
[3]
http://softwareswirl.blogspot.de/2012/12/real-world-conflict-diagrams.html
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [RFD] annnotating a pair of commit objects?
From: Johannes Sixt @ 2013-01-03 10:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4m2ycij.fsf@alter.siamese.dyndns.org>
Am 03.01.2013 08:03, schrieb Junio C Hamano:
> The intended use case is to "go beyond rerere". Given a history of
> this shape:
>
> o---o---o---I mainline
> /
> O---o---X---o---A topic A
> \
> o---Y---o---o---B topic B
>
> Suppose in the original O we had a function "distimmed_doshes()" to
> tell if doshes are already distimmed, with some call sites. On the
> branch leading to A, at commit X, this function was renamed to
> "doshes_are_distimmed()", and all existing call sites were adjusted.
> On the side branch leading to B, however, at commit Y, a new call
> site to the old function was added in a file that was not touched
> between O..A at all.
>
> When merging either the topic A or the topic B (but not both) to the
> integration branch that did not touch this function or use of it, no
> special care needs to be taken, but when merging the second topic
> after merging the other one, we need to resolve a semantic conflict.
> Namely, the callsite to "distimmed_doshes()" introduced by commit Y
> needs to be adjusted to call "doshes_are_distimmed()" instead.
I guess this issue comes up when you rebuild pu. Perhaps you (and other
integrators with a similar workflow) are sufficiently served with
something that resembles rebase -p --first-parent, as proposed here:
http://thread.gmane.org/gmane.comp.version-control.git/198125/focus=198483
(A proposal of the same idea appeared already years earlier:
http://thread.gmane.org/gmane.comp.version-control.git/62782/focus=62794
but its implementation only re-did the merge, which would not help your
case.)
-- Hannes
^ permalink raw reply
* Re: git filter-branch doesn't dereference annotated tags
From: Johannes Sixt @ 2013-01-03 10:33 UTC (permalink / raw)
To: Grégory Pakosz; +Cc: Junio C Hamano, git
In-Reply-To: <CAC_01E2HXSnXBgDm=Cbwgi5PbiuHp_qPpoaqT_=pdDWDMnC5jA@mail.gmail.com>
Am 03.01.2013 10:50, schrieb Grégory Pakosz:
>>
>> IOW, if the command was something like
>>
>> git filter-branch ...filter options... -- v1.0 master ...
>>
>> and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
>> deleted if the commit it points to goes away. But if the commit did not
>> go away, but was rewritten, then it is equally reasonable to expect that
>> the tag is also rewritten. But I don't think that we currently do the
>> latter.
>>
> When the commit doesn't go away, the tag is currently being rewritten properly.
Indeed, but only if a --tag-name-filter was specified.
>> Therefore, IMO, a change that implements the former behavior should also
>> implement the latter behavior.
>>
> The patch in my latest email does both. (yet lacks unit tests for now)
If it deletes a tag only when --tag-name-filter was specified, than that
should be fine.
-- Hannes
^ permalink raw reply
* Fwd: git-gui / Warning: "No newline at end of file”
From: Tobias Preuss @ 2013-01-03 12:26 UTC (permalink / raw)
To: git
In-Reply-To: <CADEaiE-J4nEwyK4WSiH=zzaH6cb85mw15O3wxrWrTXJtZfJixQ@mail.gmail.com>
Hello. I never got a response. Did my email pass the distribution
list? Best, Tobias
---------- Forwarded message ----------
From: Tobias Preuss <tobias.preuss@googlemail.com>
Date: Tue, Nov 13, 2012 at 9:26 PM
Subject: git-gui / Warning: "No newline at end of file”
To: git <git@vger.kernel.org>
Hello.
I noticed a problem when working with git-gui which might be a bug.
The issue only affects you when you are visually trying to stage
changes line by line. Here are the steps to reproduce the problem:
1. Initialize a new repository.
2. Create a file with three lines of content each with the word
"Hello". Do not put a new line at the end of the file.
3. Add and commit the file.
4. Edit the same file putting words inbetween the three lines.
5. Open git-gui and try to stage the changes line by line.
The editor will append the warning "No newline at end of file” to the
end of the diff. When you are trying to stage a line an error occurs.
The problem is also illustrated in a question on Stackoverflow [1].
Please let me know if you need more information or if I should send
this problem to some other mailing list.
Thank you, Tobias
____________
[1] http://stackoverflow.com/questions/13223868/how-to-stage-line-by-line-in-git-gui-although-no-newline-at-end-of-file-warnin
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Greg Troxel @ 2013-01-03 13:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2xnypt6.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> Greg Troxel <gdt@ir.bbn.com> writes:
>
>> I realize a README.foo file for N different systems could be clutter,
>> but having these checked in would provide the concise help that people
>> on any of those platforms need.
>
> Our Makefile documents knobs people on various platforms can tweak
> (PYTHON_PATH and OLD_ICONV are two examples of them), sets default
> values for them based on $(uname -S), then includes config.mak file
> the user can optionally create to override these platform defaults.
> This infrastructure is used across platforms, not just for NetBSD.
If you have to choose a single PYTHON_PATH, the one you picked is right
(for now, and likely will be right for a long time).
But as a general rule, I think configure tests are preferable to
OS-specific variables.
> The part shown in the patch was to update the platform default for
> NetBSD. The setting we have been shipping in our Makefile seemed to
> be different from what I needed on my NetBSD 6 install, and I was
> wondering if we have no real users of Git on the platorm (which
> would explain why we didn't get any complaints or patches to update
> this part). Or there are some optional packages all real NetBSD
> users install, but being a NetBSD newbie I didn't, that makes the
> values I showed in the patch inappropriate for them (e.g. Perhaps
> there is a mechanism other than pkgsrc that installs perl and python
> under /usr/bin? Perhaps an optional libi18n package that gives
> iconv(3) with new function signature?), in which case I definitely
> should not apply that patch to my tree, as it would only be an
> improvement for one person and a regression for existing users at
> the same time.
A large number of people on NetBSD use git, but almost all of them get
it via pkgsrc (which is also where they get perl, emacs, svn, and
everything else you didn't find in /usr/bin). The exception would be
people that want to hack on git itself.
People who want gnu libiconv can install the libiconv package from
pkgsrc. (I'm guessing OLD_ICONV means "POSIX iconv", without GNU
extensions (iconvctl?).) The git package doesn't depend on GNU iconv,
though (perhaps it should but we tend to avoid dependencies that arren't
really needed).
There are no mechanisms to install python/perl in /usr/bin, and doing so
would be viewed as bizarre. /usr/bin belongs to the base system,
/usr/pkg to pkgsrc and /usr/local to the user.
So, it doesn't matter too much for pkgsrc what you change, because it
can be patched anyway (once, for all users). It's probably better to
make a straightforward build from source come out right.
But, if the build respects the PYTHON_PATH environment variable, that's
easier (and more robust against changes) than patching.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ 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