* [JGIT PATCH 1/3] Check object connectivity during fetch if fsck is enabled
From: Shawn O. Pearce @ 2008-10-30 17:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1225388785-26818-1-git-send-email-spearce@spearce.org>
If we are fetching over a pack oriented connection and we are doing
object-level fsck validation we need to also verify the graph is
fully connected after the fetch is complete. This additional check
is necessary to ensure the peer didn't omit objects that we don't
have, but which are listed as needing to be present.
On the walk style fetch connection we can bypass this check, as the
connectivity was implicitly verified by the walker as it downloaded
objects and built its queue of things to fetch. Native pack and
bundle transports however do not have this check built into them,
and require that we execute the work ourselves.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../jgit/transport/BasePackFetchConnection.java | 4 +++
.../spearce/jgit/transport/FetchConnection.java | 22 ++++++++++++++++++++
.../org/spearce/jgit/transport/FetchProcess.java | 13 ++++++++++-
.../spearce/jgit/transport/TransportBundle.java | 4 +++
.../jgit/transport/WalkFetchConnection.java | 4 +++
5 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index a542eb7..542a8a9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -146,6 +146,10 @@ public boolean didFetchIncludeTags() {
return false;
}
+ public boolean didFetchTestConnectivity() {
+ return false;
+ }
+
protected void doFetch(final ProgressMonitor monitor,
final Collection<Ref> want) throws TransportException {
try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
index 9d25b0d..d93972d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
@@ -111,4 +111,26 @@ public void fetch(final ProgressMonitor monitor, final Collection<Ref> want)
* false if tags were not implicitly obtained.
*/
public boolean didFetchIncludeTags();
+
+ /**
+ * Did the last {@link #fetch(ProgressMonitor, Collection)} validate graph?
+ * <p>
+ * Some transports walk the object graph on the client side, with the client
+ * looking for what objects it is missing and requesting them individually
+ * from the remote peer. By virtue of completing the fetch call the client
+ * implicitly tested the object connectivity, as every object in the graph
+ * was either already local or was requested successfully from the peer. In
+ * such transports this method returns true.
+ * <p>
+ * Some transports assume the remote peer knows the Git object graph and is
+ * able to supply a fully connected graph to the client (although it may
+ * only be transferring the parts the client does not yet have). Its faster
+ * to assume such remote peers are well behaved and send the correct
+ * response to the client. In such tranports this method returns false.
+ *
+ * @return true if the last fetch had to perform a connectivity check on the
+ * client side in order to succeed; false if the last fetch assumed
+ * the remote peer supplied a complete graph.
+ */
+ public boolean didFetchTestConnectivity();
}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
index 654572d..bb2d051 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
@@ -118,7 +118,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
final boolean includedTags;
if (!askFor.isEmpty() && !askForIsComplete()) {
- conn.fetch(monitor, askFor.values());
+ fetchObjects(monitor);
includedTags = conn.didFetchIncludeTags();
// Connection was used for object transfer. If we
@@ -143,7 +143,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
if (!askFor.isEmpty() && (!includedTags || !askForIsComplete())) {
reopenConnection();
if (!askFor.isEmpty())
- conn.fetch(monitor, askFor.values());
+ fetchObjects(monitor);
}
}
} finally {
@@ -171,6 +171,15 @@ else if (tagopt == TagOpt.FETCH_TAGS)
}
}
+ private void fetchObjects(final ProgressMonitor monitor)
+ throws TransportException {
+ conn.fetch(monitor, askFor.values());
+ if (transport.isCheckFetchedObjects()
+ && !conn.didFetchTestConnectivity() && !askForIsComplete())
+ throw new TransportException(transport.getURI(),
+ "peer did not supply a complete object graph");
+ }
+
private void closeConnection() {
if (conn != null) {
conn.close();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
index 5b321a0..7d38b02 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
@@ -165,6 +165,10 @@ private String readLine(final byte[] hdrbuf) throws IOException {
return RawParseUtils.decode(Constants.CHARSET, hdrbuf, 0, lf);
}
+ public boolean didFetchTestConnectivity() {
+ return false;
+ }
+
@Override
protected void doFetch(final ProgressMonitor monitor,
final Collection<Ref> want) throws TransportException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 5638454..d089f7b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -189,6 +189,10 @@ WalkFetchConnection(final WalkTransport wt, final WalkRemoteObjectDatabase w) {
workQueue = new LinkedList<ObjectId>();
}
+ public boolean didFetchTestConnectivity() {
+ return true;
+ }
+
@Override
protected void doFetch(final ProgressMonitor monitor,
final Collection<Ref> want) throws TransportException {
--
1.6.0.3.756.gb776d
^ permalink raw reply related
* [JGIT PATCH 2/3] Add --[no-]thin and --[no-]fsck optiosn to fetch command line tool
From: Shawn O. Pearce @ 2008-10-30 17:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1225388785-26818-2-git-send-email-spearce@spearce.org>
This way users can force verification on the fly, such as when
fetching from an untrusted URL pasted on the command line.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/pgm/Fetch.java | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
index e14e213..ad7e08f 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
@@ -54,6 +54,22 @@
@Option(name = "--verbose", aliases = { "-v" }, usage = "be more verbose")
private boolean verbose;
+ @Option(name = "--fsck", usage = "perform fsck style checks on receive")
+ private Boolean fsck;
+
+ @Option(name = "--no-fsck")
+ void nofsck(final boolean ignored) {
+ fsck = Boolean.FALSE;
+ }
+
+ @Option(name = "--thin", usage = "fetch thin pack")
+ private Boolean thin;
+
+ @Option(name = "--no-thin")
+ void nothin(final boolean ignored) {
+ thin = Boolean.FALSE;
+ }
+
@Argument(index = 0, metaVar = "uri-ish")
private String remote = "origin";
@@ -63,6 +79,10 @@
@Override
protected void run() throws Exception {
final Transport tn = Transport.open(db, remote);
+ if (fsck != null)
+ tn.setCheckFetchedObjects(fsck.booleanValue());
+ if (thin != null)
+ tn.setFetchThin(thin.booleanValue());
final FetchResult r;
try {
r = tn.fetch(new TextProgressMonitor(), toget);
--
1.6.0.3.756.gb776d
^ permalink raw reply related
* [JGIT PATCH 3/3] Don't permit '.' or '..' in tree entries
From: Shawn O. Pearce @ 2008-10-30 17:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1225388785-26818-3-git-send-email-spearce@spearce.org>
A Git tree must not have '.' or '..' within the structure as these
names are reserved in every directory by the client operating system.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../org/spearce/jgit/lib/ObjectCheckerTest.java | 31 ++++++++++++++++++++
.../src/org/spearce/jgit/lib/ObjectChecker.java | 7 ++++
2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
index fa37fb5..7befde8 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
@@ -980,6 +980,13 @@ public void testValidTree5() throws CorruptObjectException {
checker.checkTree(data);
}
+ public void testValidTree6() throws CorruptObjectException {
+ final StringBuilder b = new StringBuilder();
+ entry(b, "100644 .a");
+ final byte[] data = Constants.encodeASCII(b.toString());
+ checker.checkTree(data);
+ }
+
public void testValidTreeSorting1() throws CorruptObjectException {
final StringBuilder b = new StringBuilder();
entry(b, "100644 fooaaa");
@@ -1166,6 +1173,30 @@ public void testInvalidTreeNameIsEmpty() {
}
}
+ public void testInvalidTreeNameIsDot() {
+ final StringBuilder b = new StringBuilder();
+ entry(b, "100644 .");
+ final byte[] data = Constants.encodeASCII(b.toString());
+ try {
+ checker.checkTree(data);
+ fail("incorrectly accepted an invalid tree");
+ } catch (CorruptObjectException e) {
+ assertEquals("invalid name '.'", e.getMessage());
+ }
+ }
+
+ public void testInvalidTreeNameIsDotDot() {
+ final StringBuilder b = new StringBuilder();
+ entry(b, "100644 ..");
+ final byte[] data = Constants.encodeASCII(b.toString());
+ try {
+ checker.checkTree(data);
+ fail("incorrectly accepted an invalid tree");
+ } catch (CorruptObjectException e) {
+ assertEquals("invalid name '..'", e.getMessage());
+ }
+ }
+
public void testInvalidTreeTruncatedInName() {
final StringBuilder b = new StringBuilder();
b.append("100644 b");
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
index d403119..b303d6f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
@@ -318,6 +318,13 @@ public void checkTree(final byte[] raw) throws CorruptObjectException {
}
if (thisNameB + 1 == ptr)
throw new CorruptObjectException("zero length name");
+ if (raw[thisNameB] == '.') {
+ final int nameLen = (ptr - 1) - thisNameB;
+ if (nameLen == 1)
+ throw new CorruptObjectException("invalid name '.'");
+ if (nameLen == 2 && raw[thisNameB + 1] == '.')
+ throw new CorruptObjectException("invalid name '..'");
+ }
if (duplicateName(raw, thisNameB, ptr - 1))
throw new CorruptObjectException("duplicate entry names");
--
1.6.0.3.756.gb776d
^ permalink raw reply related
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Sam Vilain @ 2008-10-30 17:51 UTC (permalink / raw)
To: Theodore Tso; +Cc: git, Sam Vilain
In-Reply-To: <20081030143918.GB14744@mit.edu>
On Thu, 2008-10-30 at 10:39 -0400, Theodore Tso wrote:
> * Add the command "git revert-file <files>" which is syntactic sugar for:
>
> git checkout HEAD -- <files>
>
> Rationale: Many other SCM's have a way of undoing local edits to a
> file very simply, i.e."hg revert <file>" or "svn revert <file>", and
> for many developers's workflow, it's useful to be able to undo local
> edits to a single file, but not to everything else in the working
> directory. And "git checkout HEAD -- <file>" is rather cumbersome
> to type, and many beginning users don't find it intuitive to look in
> the "git-checkout" man page for instructions on how to revert a
> local file.
Well, I don't have strong feelings on the exact command name used; I
suggested "undo", probably also ambiguous. But still, a significant
number of users are surprised when they type 'git revert' and they get a
backed out patch. It's such an uncommon operation, it doesn't deserve
to be triggered so easily. And reverting files to the state in the
index and/or HEAD is a common operation that deserves being short to
type.
Making it plain "revert" would violate expectations of existing users;
it seems a better idea to just deprecate it, and point the users to the
new method - cherry-pick --revert - or the command they might have meant
- whatever that becomes.
Sam.
^ permalink raw reply
* Re: Using the --track option when creating a branch
From: Sam Vilain @ 2008-10-30 17:57 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Samuel Tardieu, Bill Lear, git
In-Reply-To: <4909CABD.1040708@op5.se>
On Thu, 2008-10-30 at 15:54 +0100, Andreas Ericsson wrote:
> > I am curious of what other people workflows are. Do you often push
> > multiple branches at the same time?
>
> Quite often, yes.
>
> > More often than one at a time?
>
> No.
>
> > Many times a day?
> >
>
> Define "many". Perhaps as often as 2-3 times per day. Not very often,
> but frequent enough that I definitely want some short sweet way of
> doing it.
I think your use case is the unusual one, not the common one. Most
users will want the moral equivalent of "push = HEAD" by default, and
those who prefer the existing behaviour can configure it.
Sam.
^ permalink raw reply
* Re: Using the --track option when creating a branch
From: Sam Vilain @ 2008-10-30 18:00 UTC (permalink / raw)
To: Samuel Tardieu; +Cc: Pierre Habouzit, Andreas Ericsson, Bill Lear, git
In-Reply-To: <2008-10-30-15-56-34+trackit+sam@rfc1149.net>
On Thu, 2008-10-30 at 15:56 +0100, Samuel Tardieu wrote:
> * Pierre Habouzit <madcoder@debian.org> [2008-10-30 15:41:07 +0100]
>
> | On Thu, Oct 30, 2008 at 02:23:16PM +0000, Samuel Tardieu wrote:
> | > I think it would be better to have :
> | >
> | > git push <= push the current branch
> | > git push --all <= push all matching refs
> | > git push --all --create <= push all matching refs, create if needed
> | >
> | > The latest command is probably used so rarely (compared to the others)
> | > that it wouldn't be a problem to make it longer. Of course, if a
> | > refspec is given explicitely, it should be honored and remote refs
> | > created if needed.
> |
> | Fwiw I'm in favor of that, and it was what I advocated at the time.
> |
> | Though I think than as soon as you add an explicit remote name, like:
> | git push origin, pushing all matched references makes sense. Which is
> | also what I advocated at the time.
>
> Indeed, it makes sense. We could then have:
>
> git push <= push the current branch on default remote
> (which is, at least in my case, the most
> frequent use I want to make of "git push",
> on all the projects [work or volunteer]
> I work on)
> git push remote <= push all matching refs on named remote
I think that 'git push origin' should be the same as 'git push'; so,
'git push remote' would then just push the current head to the tracking
branch of that remote. This exposes another issue with the current
method of configuring the tracking branch, which is that only one remote
and branch may be configured for each local branch. In reality, someone
might be pushing and pulling from multiple remotes; expecting them to
keep naming the current branch all the time seems arduous.
I think if you want matching refs to be pushed, say so:
git push remote --matching
> git push --all [remote] <= push and create all refs on remote (or default)
^ permalink raw reply
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Sam Vilain @ 2008-10-30 18:06 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Pierre Habouzit, Matthieu Moy, Theodore Tso, git
In-Reply-To: <alpine.LFD.2.00.0810301316220.13034@xanadu.home>
On Thu, 2008-10-30 at 13:17 -0400, Nicolas Pitre wrote:
> > errrrm, git reset <file> resets the index notion of the file to its status
> > in HEAD... which I'm sure is *somehow* useful to "some" people ;P
>
> Too bad...
The changes need to not unnecessarily break scripts or let down people's
expectations. I'd be happy to deprecate the use of reset with file
arguments, to keep 'reset' focused on resetting the current HEAD and not
concerned with files; but changing its behaviour on a subtle level like
this is sure to annoy...
Sam.
^ permalink raw reply
* Re: [PATCH] Documented --no-checkout option in git-svn
From: Deskin Miller @ 2008-10-30 18:07 UTC (permalink / raw)
To: _vi; +Cc: git, gitster, Vitaly _Vi Shukela
In-Reply-To: <1225382900-22482-1-git-send-email-_vi@list.ru>
On Thu, Oct 30, 2008 at 06:08:20PM +0200, _vi@list.ru wrote:
> From: Vitaly "_Vi" Shukela <public_vi@tut.by>
>
> Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
> ---
> Documentation/git-svn.txt | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 84c8f3c..90784a5 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -91,6 +91,9 @@ COMMANDS
> tracking. The name of the [svn-remote "..."] section in the
> .git/config file may be specified as an optional command-line
> argument.
> +
> +--no-checkout
> + Do not checkout latest revision after fetching.
This isn't quite how the other options are listed in the source; for one, this
ends up formatted in the final manpage like
--no-checkout Do not checkout latest revision after fetching.
Instead of
--no-checkout
Do not checkout latest revision after fetching.
Also, the wording seems slightly imprecise; in fact, if the repository already
has a checkout, git svn fetch would not attempt to check anything out in its
place, nor will it check anything out if there is a local master branch
already. With clone this is not typically a problem, but in fact it is
possible to clone into a preexisting git repository, so the same concerns
exist.
Deskin Miller
^ permalink raw reply
* [PATCH] Introduce receive.denyDeletes
From: Jan Krüger @ 2008-10-30 18:11 UTC (permalink / raw)
To: git; +Cc: gitster
Occasionally, it may be useful to prevent branches from getting deleted from
a centralized repository, particularly when no administrative access to the
server is available to undo it via reflog. It also makes
receive.denyNonFastForwards more useful if it is used for access control, since
it prevents force-updating refs by deleting and re-creating a ref.
Signed-off-by: Jan Krüger <jk@jk.gs>
---
Fairly low invasiveness. Includes documentation and test case. I have run all
parts of the test suite that use receive-pack, send-pack and friends.
Documentation/config.txt | 4 ++++
builtin-receive-pack.c | 12 ++++++++++++
t/t5400-send-pack.sh | 11 +++++++++++
3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 29369d0..965ed74 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1188,6 +1188,10 @@ receive.unpackLimit::
especially on slow filesystems. If not set, the value of
`transfer.unpackLimit` is used instead.
+receive.denyDeletes::
+ If set to true, git-receive-pack will deny a ref update that deletes
+ the ref. Use this to prevent such a ref deletion via a push.
+
receive.denyNonFastForwards::
If set to true, git-receive-pack will deny a ref update which is
not a fast forward. Use this to prevent such an update via a push,
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 9f60f31..2c0225c 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -11,6 +11,7 @@
static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
+static int deny_deletes = 0;
static int deny_non_fast_forwards = 0;
static int receive_fsck_objects;
static int receive_unpack_limit = -1;
@@ -23,6 +24,11 @@ static int capabilities_sent;
static int receive_pack_config(const char *var, const char *value, void *cb)
{
+ if (strcmp(var, "receive.denydeletes") == 0) {
+ deny_deletes = git_config_bool(var, value);
+ return 0;
+ }
+
if (strcmp(var, "receive.denynonfastforwards") == 0) {
deny_non_fast_forwards = git_config_bool(var, value);
return 0;
@@ -185,6 +191,12 @@ static const char *update(struct command *cmd)
"but I can't find it!", sha1_to_hex(new_sha1));
return "bad pack";
}
+ if (deny_deletes && is_null_sha1(new_sha1) &&
+ !is_null_sha1(old_sha1) &&
+ !prefixcmp(name, "refs/heads/")) {
+ error("denying ref deletion for %s", name);
+ return "deletion prohibited";
+ }
if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
!is_null_sha1(old_sha1) &&
!prefixcmp(name, "refs/heads/")) {
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 544771d..6db9e18 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -104,6 +104,17 @@ HOME=`pwd`/no-such-directory
export HOME ;# this way we force the victim/.git/config to be used.
test_expect_success \
+ 'pushing a delete should be denied with denyDeletes' '
+ cd victim &&
+ git config receive.denyDeletes true &&
+ git branch extra master &&
+ cd .. &&
+ test -f victim/.git/refs/heads/extra &&
+ git send-pack ./victim/.git/ :extra master && return 1
+ rm -f victim/.git/refs/heads/extra
+'
+
+test_expect_success \
'pushing with --force should be denied with denyNonFastforwards' '
cd victim &&
git config receive.denyNonFastforwards true &&
--
1.6.0.3.523.g304d0.dirty
^ permalink raw reply related
* Re: [PATCH] Documented --no-checkout option in git-svn
From: Sam Vilain @ 2008-10-30 18:20 UTC (permalink / raw)
To: git, Deskin Miller
Cc: Sam Vilain,
_vi@list.ru, git@vger.kernel.org, gitster@pobox.com, Vitaly _Vi Shukela,
Vitaly "_Vi" Shukela
In-Reply-To: <20081030180736.GA20322@euler>
On Thu, 2008-10-30 at 14:07 -0400, Deskin Miller wrote:
> > +--no-checkout
> > + Do not checkout latest revision after fetching.
>
> This isn't quite how the other options are listed in the source; for one, this
> ends up formatted in the final manpage like
>
> --no-checkout Do not checkout latest revision after fetching.
>
> Instead of
>
> --no-checkout
> Do not checkout latest revision after fetching.
>
> Also, the wording seems slightly imprecise; in fact, if the repository already
> has a checkout, git svn fetch would not attempt to check anything out in its
> place, nor will it check anything out if there is a local master branch
> already. With clone this is not typically a problem, but in fact it is
> possible to clone into a preexisting git repository, so the same concerns
> exist.
I think the wording is close enough; here's a version which looks good
to me and fixes the asciidoc differences.
Subject: git-svn: document --no-checkout option
From: Vitaly "_Vi" Shukela <public_vi@tut.by>
Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
Signed-off-by: Sam Vilain <sam@vilain.net>
---
Documentation/git-svn.txt | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 84c8f3c..2298512 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -91,6 +91,9 @@ COMMANDS
tracking. The name of the [svn-remote "..."] section in the
.git/config file may be specified as an optional command-line
argument.
+
+--no-checkout;;
+ Do not checkout the latest revision after fetching.
'clone'::
Runs 'init' and 'fetch'. It will automatically create a
@@ -103,6 +106,9 @@ COMMANDS
the working tree; and the 'rebase' command will be able
to update the working tree with the latest changes.
+--no-checkout;;
+ Do not checkout the latest revision after cloning.
+
'rebase'::
This fetches revisions from the SVN parent of the current HEAD
and rebases the current (uncommitted to SVN) work against it.
--
debian.1.5.6.1
^ permalink raw reply related
* [PATCH] Documentation: clarify information about 'ident' attribute
From: Jan Krüger @ 2008-10-30 18:14 UTC (permalink / raw)
To: git; +Cc: gitster
The documentation spoke of the attribute being set "to" a path; this can
mistakenly be interpreted as "the attribute needs to have its value set to
some kind of path". This clarifies things (and also calls the object ID a
hash rather than a name because that might be confusing too).
Signed-off-by: Jan Krüger <jk@jk.gs>
---
Confused me and a few other people on IRC the other day.
Documentation/gitattributes.txt | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2694559..b39db6b 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -163,9 +163,9 @@ few exceptions. Even though...
`ident`
^^^^^^^
-When the attribute `ident` is set to a path, git replaces
-`$Id$` in the blob object with `$Id:`, followed by
-40-character hexadecimal blob object name, followed by a dollar
+When the attribute `ident` is set for a path, git replaces
+`$Id$` in the blob object with `$Id:`, followed by the
+40-character hexadecimal blob object hash, followed by a dollar
sign `$` upon checkout. Any byte sequence that begins with
`$Id:` and ends with `$` in the worktree file is replaced
with `$Id$` upon check-in.
--
1.6.0.3.523.g304d0.dirty
^ permalink raw reply related
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Nicolas Pitre @ 2008-10-30 18:28 UTC (permalink / raw)
To: Sam Vilain; +Cc: Pierre Habouzit, Mike Hommey, Shawn O. Pearce, git
In-Reply-To: <1225387882.19891.9.camel@maia.lan>
On Thu, 30 Oct 2008, Sam Vilain wrote:
> On Thu, 2008-10-30 at 12:53 -0400, Nicolas Pitre wrote:
> > > Seconded.
> > >
> > > Having git-checkout $foo being a shorthand for git checkout -b $foo
> > > origin/$foo when origin/$foo exists and $foo doesn't is definitely handy.
> >
> > No. This is only the first step towards insanity.
> >
> > In many cases origin/$foo == origin/master so this can't work in that
> > case which is, after all, the common case.
>
> I don't understand that argument at all, can you explain further?
By default, git creates a branch called "master. Hence, by default, if
you clone that repository, this branch will be called origin/master. So
by default $foo is already ambiguous.
> > Therefore I think this is
> > wrong to add magic operations which are not useful for the common case
> > and actively _hide_ how git actually works. Not only will you have to
> > explain how git works anyway for that common origin/master case, but
> > you'll also have to explain why sometimes the magic works and sometimes
> > not. Please keep such convenience shortcuts for your own scripts and/or
> > aliases.
>
> It's not about magic, it's about sensible defaults. Currently this use
> case is an error, and the resultant command is very long to type, and
> involves typing the branch name twice. I end up writing things like:
>
> git checkout -b {,origin/}wr34251-do-something
>
> For the user who doesn't know to use the ksh-style {} blocks this is
> voodoo. The longer form is cumbersome.
This is no excuse for promoting semantics only useful in such special
cases.
> For the case where the thing you type is a resolvable reference, it
> would just check it out, as now.
As long as it checks it out with a detached head if it is a remote
branch then I have no issue.
Nicolas
^ permalink raw reply
* Re: [PATCH] Introduce receive.denyDeletes
From: Shawn O. Pearce @ 2008-10-30 18:32 UTC (permalink / raw)
To: Jan Krrrger; +Cc: git, gitster
In-Reply-To: <20081030191134.62455c24@perceptron>
Jan Krrrger <jk@jk.gs> wrote:
> Occasionally, it may be useful to prevent branches from getting deleted from
> a centralized repository, particularly when no administrative access to the
> server is available to undo it via reflog. It also makes
> receive.denyNonFastForwards more useful if it is used for access control, since
> it prevents force-updating refs by deleting and re-creating a ref.
...
> test_expect_success \
> + 'pushing a delete should be denied with denyDeletes' '
> + cd victim &&
> + git config receive.denyDeletes true &&
> + git branch extra master &&
> + cd .. &&
> + test -f victim/.git/refs/heads/extra &&
> + git send-pack ./victim/.git/ :extra master && return 1
Hmm, that should be:
test_must_fail git send-pack ./victim/.git/ :extra master
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Introduce receive.denyDeletes
From: Jan Krüger @ 2008-10-30 18:45 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20081030183210.GO14786@spearce.org>
On Thu, 30 Oct 2008 11:32:10 -0700, "Shawn O. Pearce"
<spearce@spearce.org> wrote:
> > test_expect_success \
> > [snip]
> > + git send-pack ./victim/.git/ :extra master && return 1
>
> Hmm, that should be:
>
> test_must_fail git send-pack ./victim/.git/ :extra master
Can I then delete the branch afterwards without lots of juggling (in
case the test fails due to a random other reason that the branch
accidentally getting deleted by receive-pack)? I'd expect I'd have to
save the exit code to a temporary variable and that's just as ugly.
Disclaimer: I sort of just hacked the test case together by stringing
together pieces of existing test code, so I don't presume full
understanding of the test framework.
By the way, I love what your mail client did to my name.
-Jan
^ permalink raw reply
* Re: Using the --track option when creating a branch
From: Marc Branchaud @ 2008-10-30 19:13 UTC (permalink / raw)
To: Bill Lear; +Cc: Andreas Ericsson, Samuel Tardieu, git
In-Reply-To: <18697.54743.601331.133842@lisa.zopyra.com>
Bill Lear wrote:
>
> the reason being that every manual our users read says "use git push",
> use "git pull", the examples being written for 'master' branch usage,
> and people just assume that 'git push'/'git pull' are smart enough to
> know which branch you are on and do the same logical thing as a bare
> 'git push'/'git pull' does when on master.
I agree that this is a 'gotcha' for git-push. I'm a new git user, and
I've been experimenting with git and reading the documentation for the
last few weeks. But I would not have known about this behavior if it
weren't for this thread.
Yes, push's man page is clear about what happens if you push with no
refspec, and the fetch & pull man pages both have an obscure note to
"never do your own development on branches that appear
on the right hand side of a <refspec> colon on 'Pull:' lines". But
still the behavior is not what I expected. Before I read this thread, I
missed the implications of what those parts of the man pages were saying.
One could call this a failure of the documentation (man pages and
beyond). Personally, though, I tend to expect minimal commands to do
minimal things. So a plain "git push" would do the minimum amount of
pushing, and if I want it to do more I'd add extra parameters to the
command.
The current behavior seems fairly harmless if you always follow the
pattern of creating topic branches for all your work. But git (rightly)
doesn't enforce that pattern, and so I think push shouldn't default to
doing something potentially harmful just because you forgot to create a
topic branch one day. (Or maybe you decided to be clever and give one
of your local branches the same name as a remote's branch...)
Marc
^ permalink raw reply
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Junio C Hamano @ 2008-10-30 19:24 UTC (permalink / raw)
To: sverre; +Cc: Git Mailinglist
In-Reply-To: <bd6139dc0810291606o2efe4254me378335b76861340@mail.gmail.com>
I have a mixed feeling about this. From a longer-term perspective, do you
really want this to be a part of git.git repository?
I do not mind having notes to endorse and advocate "stats" as one of the
"Third party packages that may make your git life more pleasuable", just
like tig, stgit, guilt and topgit, but I cannot convince myself that
merging it as a subtree is the right thing to do at this point.
The "stats" tool, at least at the conceptual level, shares one important
property with tools like gitk and gitweb: it could be useful to people
whose sources are not in git repositories but in say Hg or Bzr, with some
effort. The code may need refactoring to make it easier to plug in
different backends and writing actual backends (aka "porting"), but that
is something you can expect people with different backends to help you
with.
However, it would be awkward for the contrib/ area in git.git to carry a
lot of code that are only needed to produce stat data from non-git
repositories, once such a porting effort begins.
It's perfectly fine if you are not interested in any of the other
backends, and tell the people that they are welcome to fork it never to
merge back. But if this were my brainchild, I'd imagine I'd be wishing to
be able to buy back the improvements to the "core stats" parts that are
done by people with other backends. I would imagine binding the current
code as part of git.git would make such improvements harder to manage,
both for you (who wants to buy back the changes made by others on
different backends) and for others on different backends (who want to
merge the changes made by you to their forks).
Perhaps pointing at your tree as a submodule would be the right thing to
do; then git.git proper will be just one of the users of "stats" tool.
How about making that as a mid-to-longer term goal? When we eject git-gui
and gitk from git.git and make them a submodule (wasn't that supposed to
happen in 1.8 or 2.0 timeframe?), we may also add "stats" as a submodule?
^ permalink raw reply
* Re: [PATCH] Documentation: clarify information about 'ident' attribute
From: Junio C Hamano @ 2008-10-30 19:30 UTC (permalink / raw)
To: Jan Krüger; +Cc: git, gitster
In-Reply-To: <20081030191433.710aff11@perceptron>
"Jan Krüger" <jk@jk.gs> writes:
> The documentation spoke of the attribute being set "to" a path; this can
> mistakenly be interpreted as "the attribute needs to have its value set to
> some kind of path". This clarifies things (and also calls the object ID a
> hash rather than a name because that might be confusing too).
I'd agree with the first change, but not with the second. "object name"
has been the official name of these hexadecimal thingy for a long time
(see the glossary).
^ permalink raw reply
* Re: [PATCH] git show <tree>: show mode and hash, and handle -r
From: Junio C Hamano @ 2008-10-30 21:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, schacon
In-Reply-To: <alpine.DEB.1.00.0810290207330.22125@pacific.mpi-cbg.de.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Now, git show <tree> shows some more information, and with the -r option,
> it recurses into subdirectories.
>
> Requested by Scott Chacon.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> The only quirk is the command line parsing for -r: we cannot use
> DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched
> on not only by cmd_log_init(), but implicitly by
> diff_setup_done(), because FORMAT_PATCH is selected by git-show.
That's a rather large quirk with an ugly workaround if you ask me.
I also notice that there is:
int cmd_log_reflog(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
...
/*
* This means that we override whatever commit format the user gave
* on the cmd line. Sad, but cmd_log_init() currently doesn't
* allow us to set a different default.
*/
I wonder if it would help breaking down cmd_log_init() a bit like this.
builtin-log.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git c/builtin-log.c w/builtin-log.c
index 2efe593..0fcc28a 100644
--- c/builtin-log.c
+++ w/builtin-log.c
@@ -50,18 +50,23 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
return 0;
}
-static void cmd_log_init(int argc, const char **argv, const char *prefix,
- struct rev_info *rev)
+static void cmd_log_init_0(int argc, const char **argv, const char *prefix,
+ struct rev_info *rev,
+ int default_abbrev,
+ int default_commit_format,
+ int default_verbose_header,
+ int default_recursive)
{
int i;
int decorate = 0;
- rev->abbrev = DEFAULT_ABBREV;
- rev->commit_format = CMIT_FMT_DEFAULT;
+ rev->abbrev = default_abbrev;
+ rev->commit_format = default_commit_format;
if (fmt_pretty)
get_commit_format(fmt_pretty, rev);
- rev->verbose_header = 1;
- DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
+ rev->verbose_header = default_verbose_header;
+ if (default_recursive)
+ DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
@@ -88,6 +93,16 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
}
}
+static void cmd_log_init(int argc, const char **argv, const char *prefix,
+ struct rev_info *rev)
+{
+ cmd_log_init_0(argc, argv, prefix, rev,
+ DEFAULT_ABBREV,
+ CMIT_FMT_DEFAULT,
+ 1,
+ 1);
+}
+
/*
* This gives a rough estimate for how many commits we
* will print out in the list.
^ permalink raw reply related
* RE: [RFC] gitweb: add 'historyfollow' view that follows renames
From: Moore, Robert @ 2008-10-30 22:00 UTC (permalink / raw)
To: Blucher, Guy, Lin, Ming M, jnareb@gmail.com; +Cc: git@vger.kernel.org
In-Reply-To: <054F21930D24A0428E5B4588462C7AED0149B4B8@ednex512.dsto.defence.gov.au>
Thanks. This sounds like what we need.
We will take a look at it.
Bob
>-----Original Message-----
>From: Blucher, Guy [mailto:Guy.Blucher@dsto.defence.gov.au]
>Sent: Sunday, October 26, 2008 8:18 PM
>To: Lin, Ming M; jnareb@gmail.com; Moore, Robert
>Cc: git@vger.kernel.org
>Subject: [RFC] gitweb: add 'historyfollow' view that follows renames
>
>
>Hi Folks,
>
>>>
>>> What should we add to automatically get all file history?
>
>> While the 'commitdiff' view would, in default gitweb configuration,
>> contain information about file renames, currently 'history' view does
>> not support '--follow' option to git-log. It wouldn't be too hard to
>> add it, but it just wasn't done (well, add to this the fact that
>> --follow works only for simple cases).
>
>We also ran up against this issue because renaming of files in our
>project is a useful bit of information while browsing file history.
>
>I hacked gitweb to add a 'historyfollow' viewing option in addition to
>the 'history' option. Yes, '--follow' is expensive so I didn't just
>make it the default 'history' behaviour.
>
>The only problem with doing it was that parse_commits in gitweb.perl
>uses git rev-list which doesn't support the '--follow' option like
>git-log does. A bit of hacking to use 'git log --pretty=raw -z' to make
>the output look close to that from rev-list means only minor
>shoe-horning is required (perhaps it would be better to make rev-list
>understand --follow though?).
>
>I wasn't originally prepared to publish the work here because I don't
>really think it's the best solution. But considering it's come up... I
>include a patch inline against gitweb.perl from v1.6.0.3 that implements
>a 'historyfollow' view.
>
>Feel free to do with it what you will. It would need some substantial
>tidying up by someone who knows much more about perl than me :)
>
>We use it such that anywhere a 'history' link is provided a
>'historyfollow' link is also provided next to it - This patch doesn't
>implement that bit though.
>
>Hope it helps.
>
>Cheers,
>Guy.
>
>---
>
>--- a/gitweb/gitweb.perl
>+++ b/gitweb/gitweb.perl
>@@ -478,6 +478,7 @@ my %actions = (
> "forks" => \&git_forks,
> "heads" => \&git_heads,
> "history" => \&git_history,
>+ "historyfollow" => \&git_history_follow,
> "log" => \&git_log,
> "rss" => \&git_rss,
> "atom" => \&git_atom,
>@@ -2311,25 +2312,39 @@ sub parse_commit {
> }
>
> sub parse_commits {
>- my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
>+ my ($commit_id, $maxcount, $skip, $mode, $filename, @args) = @_;
> my @cos;
>
> $maxcount ||= 1;
> $skip ||= 0;
>
> local $/ = "\0";
>+ # The '--max-count' argument is not available when doing a
>+ # '--follow' to 'git log'
>+ my $count_arg = ("--max-count=" . $maxcount) ;
>+ if (defined $mode && $mode eq "--follow") {
>+ $count_arg = "--follow" ;
>+ }
>
>- open my $fd, "-|", git_cmd(), "rev-list",
>- "--header",
>+
>+ open my $fd, "-|", git_cmd(), "log",
>+ "-z",
>+ "--pretty=raw",
> @args,
>- ("--max-count=" . $maxcount),
>+ ($count_arg ? ($count_arg ) : ()),
> ("--skip=" . $skip),
> @extra_options,
> $commit_id,
> "--",
> ($filename ? ($filename) : ())
>- or die_error(500, "Open git-rev-list failed");
>+ or die_error(500, "Open git-log failed");
> while (my $line = <$fd>) {
>+ # Need to put a delimiter on the end of output
>+ # 'git-log -z' doesn't put one before EOF like rev-list
>does
>+ $line = ($line . '\0');
>+ # Need to strip the word commit from the start so it
>+ # looks like rev-list output
>+ $line =~ s/^commit // ;
> my %co = parse_commit_text($line);
> push @cos, \%co;
> }
>@@ -5363,6 +5378,13 @@ sub git_commitdiff_plain {
> }
>
> sub git_history {
>+ my $mode = shift || '';
>+ my $history_call = "history";
>+
>+ if ($mode eq "--follow") {
>+ $history_call .= "historyfollow" ;
>+ }
>+
> if (!defined $hash_base) {
> $hash_base = git_get_head_hash($project);
> }
>@@ -5377,7 +5399,7 @@ sub git_history {
> my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
>
> my @commitlist = parse_commits($hash_base, 101, (100 * $page),
>- $file_name, "--full-history")
>+ $mode, $file_name,
>"--full-history")
> or die_error(404, "No such file or directory on given
>branch");
>
> if (!defined $hash && defined $file_name) {
>@@ -5398,7 +5420,7 @@ sub git_history {
> my $paging_nav = '';
> if ($page > 0) {
> $paging_nav .=
>- $cgi->a({-href => href(action=>"history",
>hash=>$hash, hash_base=>$hash_base,
>+ $cgi->a({-href => href(action=>"$history_call",
>hash=>$hash, hash_base=>$hash_base,
> file_name=>$file_name)},
> "first");
> $paging_nav .= " ⋅ " .
>@@ -5429,6 +5451,11 @@ sub git_history {
> git_footer_html();
> }
>
>+sub git_history_follow {
>+ git_history('--follow');
>+}
>+
>+
> sub git_search {
> gitweb_check_feature('search') or die_error(403, "Search is
>disabled");
> if (!defined $searchtext) {
>@@ -5469,7 +5496,7 @@ sub git_search {
> $greptype = "--committer=";
> }
> $greptype .= $searchtext;
>- my @commitlist = parse_commits($hash, 101, (100 *
>$page), undef,
>+ my @commitlist = parse_commits($hash, 101, (100 *
>$page), undef, undef,
> $greptype,
>'--regexp-ignore-case',
> $search_use_regexp ?
>'--extended-regexp' : '--fixed-strings');
>
>@@ -5737,7 +5764,7 @@ sub git_feed {
>
> # log/feed of current (HEAD) branch, log of given branch,
>history of file/directory
> my $head = $hash || 'HEAD';
>- my @commitlist = parse_commits($head, 150, 0, $file_name);
>+ my @commitlist = parse_commits($head, 150, 0, undef,
>$file_name);
>
> my %latest_commit;
> my %latest_date;
>---
>
>Guy.
>____________________________________________________
>Guy Blucher
>Defence Science and Technology Organisation
>AUSTRALIA
>
>IMPORTANT : This email remains the property of the Australian Defence
>Organisation and is subject to the jurisdiction of section 70 of the
>Crimes Act 1914. If you have received this email in error, you are
>requested to contact the sender and delete the email.
^ permalink raw reply
* [PATCH] t4030: Don't use echo -n
From: Brian Gernhardt @ 2008-10-30 22:12 UTC (permalink / raw)
To: Git List; +Cc: Shawn O Pearce
Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
t/t4030-diff-textconv.sh | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3945731..7ec244f 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -96,16 +96,15 @@ cat >expect.typechange <<'EOF'
-1
diff --git a/file b/file
new file mode 120000
-index ad8b3d2..67be421
+index ad8b3d2..8e4020b
--- /dev/null
+++ b/file
@@ -0,0 +1 @@
+frotz
-\ No newline at end of file
EOF
# make a symlink the hard way that works on symlink-challenged file systems
test_expect_success 'textconv does not act on symlinks' '
- echo -n frotz > file &&
+ echo frotz > file &&
git add file &&
git ls-files -s | sed -e s/100644/120000/ |
git update-index --index-info &&
--
1.6.0.3.523.g304d0
^ permalink raw reply related
* [RFC PATCH v2 0/2] Detection of directory renames
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
To: git
This is an update of my previous patch. It brings:
* a couple of fixes to annoying segfaults
* a testsuite showing where we're standing.
* getting rid of POSIX dirname
* improvement of debug messages
The code is still full of FIXME's, not ready for inclusin into any
branch whatsoever, and I'd be happy to hear about it if you can make
it break in a way or another - at least in a way not yet shown by the
testsuite :)
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply
* [PATCH 1/2] Introduce rename factorization in diffcore.
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
To: git
In-Reply-To: <20081030220532.3325.54457.stgit@gandelf.nowhere.earth>
Rename factorization tries to group together files moving from and to
identical directories - the most common case being directory renames.
This feature is activated by the new --factorize-renames diffcore
flag.
This is only the first step, adding the basic functionnality and
adding support to raw diff output (and it breaks unified-diff output
which does not know how to handle that stuff yet).
Even the output format may not be kept as is. For now both the result
of "mv a b" and "mv a/* b/" are displayed as "Rnnn a/ b/", which is
probably not what we want. "Rnnn a/* b/" could be a good choice for
the latter if we want them to be distinguished, and even if we want
them to look the same.
Other future developements to be made on top of this include:
* extension of unified-diff format to express this
* application of such new diffs
* teach git-svn (and others ?) to make use of that flag
* merge correctly in case of addition into a moved dir
* detect "directory splits" so merge can flag a conflict on file adds
* use inexact dir renames to bump score of below-threshold renames
from/to same locations
* add yours here
---
diff-lib.c | 6 +
diff.c | 5 +
diff.h | 3 +
diffcore-rename.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++++++--
diffcore.h | 1
tree-diff.c | 4 +
6 files changed, 235 insertions(+), 11 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index ae96c64..dcc4c2c 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -179,7 +179,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
changed = ce_match_stat(ce, &st, ce_option);
if (!changed) {
ce_mark_uptodate(ce);
- if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+ if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
+ !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
continue;
}
oldmode = ce->ce_mode;
@@ -310,7 +311,8 @@ static int show_modified(struct oneway_unpack_data *cbdata,
oldmode = old->ce_mode;
if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
- !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+ !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
+ !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
return 0;
diff_change(&revs->diffopt, oldmode, mode,
diff --git a/diff.c b/diff.c
index e368fef..f91fcf6 100644
--- a/diff.c
+++ b/diff.c
@@ -2437,6 +2437,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
DIFF_OPT_SET(options, REVERSE_DIFF);
else if (!strcmp(arg, "--find-copies-harder"))
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+ else if (!strcmp(arg, "--factorize-renames")) {
+ DIFF_OPT_SET(options, FACTORIZE_RENAMES);
+ if (!options->detect_rename)
+ options->detect_rename = DIFF_DETECT_RENAME;
+ }
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
else if (!strcmp(arg, "--color"))
diff --git a/diff.h b/diff.h
index a49d865..db1658b 100644
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_IGNORE_SUBMODULES (1 << 18)
#define DIFF_OPT_DIRSTAT_CUMULATIVE (1 << 19)
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
+#define DIFF_OPT_FACTORIZE_RENAMES (1 << 21)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
@@ -220,6 +221,8 @@ extern void diffcore_std(struct diff_options *);
" -C detect copies.\n" \
" --find-copies-harder\n" \
" try unchanged files as candidate for copy detection.\n" \
+" --factorize-renames\n" \
+" factorize renames of all files of a directory.\n" \
" -l<n> limit rename attempts up to <n> paths.\n" \
" -O<file> reorder diffs according to the <file>.\n" \
" -S<string> find filepair whose only one side contains the string.\n" \
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 1b2ebb4..fc789bc 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -52,6 +52,32 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
return &(rename_dst[first]);
}
+static struct diff_rename_dst *locate_rename_dst_dir(struct diff_filespec *dir)
+{
+ /* code mostly duplicated from locate_rename_dst - not sure we
+ * could merge them efficiently,though
+ */
+ int first, last;
+ int dirlength = strlen(dir->path);
+
+ first = 0;
+ last = rename_dst_nr;
+ while (last > first) {
+ int next = (last + first) >> 1;
+ struct diff_rename_dst *dst = &(rename_dst[next]);
+ int cmp = strncmp(dir->path, dst->two->path, dirlength);
+ if (!cmp)
+ return dst;
+ if (cmp < 0) {
+ last = next;
+ continue;
+ }
+ first = next+1;
+ }
+ /* not found */
+ return NULL;
+}
+
/* Table of rename/copy src files */
static struct diff_rename_src {
struct diff_filespec *one;
@@ -409,6 +435,165 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)
m[worst] = *o;
}
+struct diff_dir_rename {
+ struct diff_filespec *one;
+ struct diff_filespec *two;
+ int discarded;
+ struct diff_dir_rename* next;
+};
+
+/*
+ * Marks as such file_rename if it is made uninteresting by dir_rename.
+ * Returns -1 if the file_rename is outside of the range in which given
+ * renames concerned by dir_rename are to be found (ie. end of loop),
+ * 0 otherwise.
+ */
+static int maybe_mark_uninteresting(struct diff_rename_dst* file_rename,
+ struct diff_dir_rename* dir_rename,
+ int one_len, int two_len)
+{
+ if (!file_rename->pair) /* file add */
+ return 0;
+ if (strncmp(file_rename->two->path,
+ dir_rename->two->path, two_len))
+ return -1;
+ if (strncmp(file_rename->pair->one->path,
+ dir_rename->one->path, one_len) ||
+ !basename_same(file_rename->pair->one, file_rename->two) ||
+ file_rename->pair->score != MAX_SCORE)
+ return 0;
+
+ file_rename->pair->uninteresting_rename = 1;
+ fprintf (stderr, "[DBG] %s* -> %s* makes %s -> %s uninteresting\n",
+ dir_rename->one->path, dir_rename->two->path,
+ file_rename->pair->one->path, file_rename->two->path);
+ return 0;
+}
+
+// FIXME: prevent possible overflow
+/*
+ * Copy dirname of src into dst, with final "/".
+ * Only handles relative paths since there is no relative path in a git repo.
+ * Writes "./" when there is no "/" in src.
+ * May overwrite more chars than really needed, if src ends with a "/".
+ */
+static const char* copy_dirname(char* dst, const char* src)
+{
+ char* lastslash = strrchr(src, '/');
+ if (!lastslash)
+ return strcpy(dst, "./");
+ strncpy(dst, src, lastslash - src + 1);
+ dst[lastslash - src + 1] = '\0';
+
+ // if src ends with a "/" strip the last component
+ if (lastslash[1] == '\0') {
+ lastslash = strrchr(dst, '/');
+ if (!lastslash)
+ return strcpy(dst, ".");
+ lastslash[1] = '\0';
+ }
+
+ return dst;
+}
+
+/*
+ * FIXME: we could optimize the 100%-rename case by preventing
+ * recursion to unfold what we know we would refold here.
+ * FIXME: do we want to replace linked list with sorted array ?
+ * FIXME: this prototype only handles renaming of dirs without
+ * a subdir.
+ * FIXME: leaks like hell.
+ * FIXME: ideas to evaluate a similarity score, anyone ?
+ * 10% * tree similarity + 90% * moved files similarity ?
+ */
+static struct diff_dir_rename* factorization_candidates = NULL;
+static void diffcore_factorize_renames(void)
+{
+ char one_parent_path[PATH_MAX], two_parent_path[PATH_MAX];
+ int i;
+
+ for (i = 0; i < rename_dst_nr; i++) {
+ // FIXME: what are those ?
+ if (!rename_dst[i].pair)
+ continue;
+ // dummy renames used by copy detection
+ if (!strcmp(rename_dst[i].pair->one->path, rename_dst[i].pair->two->path))
+ continue;
+
+ copy_dirname(one_parent_path, rename_dst[i].pair->one->path);
+
+ struct diff_filespec* one_parent = alloc_filespec(one_parent_path);
+ fill_filespec(one_parent, null_sha1 /*FIXME*/, S_IFDIR);
+
+ if (!locate_rename_dst_dir(one_parent)) {
+ // one_parent_path is empty in result tree
+
+ // already considered ?
+ struct diff_dir_rename* seen;
+ for (seen=factorization_candidates; seen; seen = seen->next)
+ if (!strcmp(seen->one->path, one_parent_path)) break;
+ if (!seen) {
+ // record potential dir rename
+ copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
+
+ seen = xmalloc(sizeof(*seen));
+ seen->one = one_parent;
+ seen->two = alloc_filespec(two_parent_path);
+ fill_filespec(seen->two, null_sha1 /*FIXME*/, S_IFDIR);
+ seen->discarded = 0;
+ seen->next = factorization_candidates;
+ factorization_candidates = seen;
+ fprintf (stderr, "[DBG] %s -> %s suggests possible rename from %s to %s\n",
+ rename_dst[i].pair->one->path,
+ rename_dst[i].pair->two->path,
+ one_parent_path, two_parent_path);
+ fflush(stdout);
+ continue;
+ }
+ if (seen->discarded)
+ continue;
+ // check that seen entry matches this rename
+ copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
+ if (strcmp(two_parent_path, seen->two->path)) {
+ fprintf (stderr, "[DBG] discarding dir split of %s from renames (into %s and %s)\n",
+ one_parent_path, two_parent_path, seen->two->path);
+ seen->discarded = 1;
+ }
+
+ /* all checks ok, we keep that entry */
+ }
+ }
+
+ // turn candidates into real renames
+ struct diff_dir_rename* candidate;
+ for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
+ int two_idx, i, one_len, two_len;
+ if (candidate->discarded)
+ continue;
+
+ if (!locate_rename_dst_dir(candidate->two)) {
+ fprintf (stderr, "PANIC: %s candidate of rename not in target tree (from %s)\n",
+ candidate->two->path, candidate->one->path);
+ }
+ // bisect to an entry within candidate dst dir
+ two_idx = locate_rename_dst_dir(candidate->two) - rename_dst;
+
+ // now remove extraneous 100% files inside.
+ one_len = strlen(candidate->one->path);
+ two_len = strlen(candidate->two->path);
+ for (i = two_idx; i < rename_dst_nr; i++)
+ if (maybe_mark_uninteresting (rename_dst+i, candidate,
+ one_len, two_len) < 0)
+ break;
+ for (i = two_idx-1; i >= 0; i--)
+ if (maybe_mark_uninteresting (rename_dst+i, candidate,
+ one_len, two_len) < 0)
+ break;
+ }
+
+ return;
+}
+
void diffcore_rename(struct diff_options *options)
{
int detect_rename = options->detect_rename;
@@ -446,13 +631,22 @@ void diffcore_rename(struct diff_options *options)
p->one->rename_used++;
register_rename_src(p->one, p->score);
}
- else if (detect_rename == DIFF_DETECT_COPY) {
- /*
- * Increment the "rename_used" score by
- * one, to indicate ourselves as a user.
- */
- p->one->rename_used++;
- register_rename_src(p->one, p->score);
+ else {
+ if (detect_rename == DIFF_DETECT_COPY) {
+ /*
+ * Increment the "rename_used" score by
+ * one, to indicate ourselves as a user.
+ */
+ p->one->rename_used++;
+ register_rename_src(p->one, p->score);
+ }
+ if (DIFF_OPT_TST(options, FACTORIZE_RENAMES)) {
+ /* similarly, rename factorization needs to
+ * see all files from second tree
+ */
+ //p->two->rename_used++; // FIXME: would we need that ?
+ locate_rename_dst(p->two, 1);
+ }
}
}
if (rename_dst_nr == 0 || rename_src_nr == 0)
@@ -561,8 +755,24 @@ void diffcore_rename(struct diff_options *options)
/* At this point, we have found some renames and copies and they
* are recorded in rename_dst. The original list is still in *q.
*/
+
+ /* Now possibly factorize those renames and copies. */
+ if (DIFF_OPT_TST(options, FACTORIZE_RENAMES))
+ diffcore_factorize_renames();
+
outq.queue = NULL;
outq.nr = outq.alloc = 0;
+
+ // first, turn factorization_candidates into real renames
+ struct diff_dir_rename* candidate;
+ for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
+ struct diff_filepair* pair;
+ if (candidate->discarded) continue;
+ pair = diff_queue(&outq, candidate->one, candidate->two);
+ pair->score = MAX_SCORE;
+ pair->renamed_pair = 1;
+ }
+
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
struct diff_filepair *pair_to_free = NULL;
@@ -577,7 +787,8 @@ void diffcore_rename(struct diff_options *options)
struct diff_rename_dst *dst =
locate_rename_dst(p->two, 0);
if (dst && dst->pair) {
- diff_q(&outq, dst->pair);
+ if (!dst->pair->uninteresting_rename)
+ diff_q(&outq, dst->pair);
pair_to_free = p;
}
else
diff --git a/diffcore.h b/diffcore.h
index 713cca7..6d2e65b 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -66,6 +66,7 @@ struct diff_filepair {
unsigned broken_pair : 1;
unsigned renamed_pair : 1;
unsigned is_unmerged : 1;
+ unsigned uninteresting_rename : 1;
};
#define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
diff --git a/tree-diff.c b/tree-diff.c
index 9f67af6..872f757 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -49,7 +49,9 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
show_entry(opt, "+", t2, base, baselen);
return 1;
}
- if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
+ if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) &&
+ !DIFF_OPT_TST(opt, FACTORIZE_RENAMES) &&
+ !hashcmp(sha1, sha2) && mode1 == mode2)
return 0;
/*
^ permalink raw reply related
* [PATCH 2/2] Add testcases for the --factorize-renames diffcore flag.
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
To: git
In-Reply-To: <20081030220532.3325.54457.stgit@gandelf.nowhere.earth>
This notably includes a couple of tests for cases known not to be
working correctly yet.
---
t/t4030-diff-rename-factorize.sh | 209 ++++++++++++++++++++++++++++++++++++++
1 files changed, 209 insertions(+), 0 deletions(-)
create mode 100755 t/t4030-diff-rename-factorize.sh
diff --git a/t/t4030-diff-rename-factorize.sh b/t/t4030-diff-rename-factorize.sh
new file mode 100755
index 0000000..fcf8fb6
--- /dev/null
+++ b/t/t4030-diff-rename-factorize.sh
@@ -0,0 +1,209 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Yann Dirson
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Test rename factorization in diff engine.
+
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+test_expect_success \
+ 'commit the index.' \
+ 'git update-ref HEAD $(echo "original empty commit" | git commit-tree $(git write-tree))'
+
+mkdir a
+echo >a/path0 'Line 1
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line 8
+Line 9
+Line 10
+line 11
+Line 12
+Line 13
+Line 14
+Line 15
+'
+sed <a/path0 >a/path1 s/Line/Record/
+sed <a/path0 >a/path2 s/Line/Stuff/
+sed <a/path0 >a/path3 s/Line/Blurb/
+
+test_expect_success \
+ 'update-index --add file inside a directory.' \
+ 'git update-index --add a/path*'
+
+test_expect_success \
+ 'write that tree.' \
+ 'tree=$(git write-tree) && test -n "$tree"'
+
+test_expect_success \
+ 'commit the index.' \
+ 'git update-ref HEAD $(echo "original set of files" | git commit-tree $tree)'
+
+mv a b
+test_expect_success \
+ 'renamed the directory.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames after directory move.' \
+ 'git diff-index --factorize-renames $tree >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+EOF
+
+test_expect_success \
+ 'validate the output for directory move.' \
+ 'compare_diff_patch current.filtered expected'
+
+# now test non-100% renames
+
+echo 'Line 16' >> b/path0
+mv b/path2 b/2path
+rm b/path3
+echo anything > b/path100
+test_expect_success \
+ 'edited dir contents.' \
+ 'git update-index --add --remove b/* b/path2 b/path3'
+
+test_expect_success \
+ 'git diff-index --factorize-renames after directory move and content changes.' \
+ 'git diff-index --factorize-renames $tree >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+:100644 000000 c6971ab9f08a6cd9c89a0f87d94ae347aad6144a 0000000000000000000000000000000000000000 D a/path3
+:100644 100644 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 R100 a/path2 b/2path
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 4db595d12886f90e36765fc1732c17bccb836663 R093 a/path0 b/path0
+:000000 100644 0000000000000000000000000000000000000000 1ba4650885513e62386fd3e23aeb45beeb67d3bb A b/path100
+EOF
+
+test_expect_success \
+ 'validate the output for directory move and content changes.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test bulk moves that are not directory moves (get consensus before going further ?)
+
+mkdir c
+for i in 0 1 2; do cp a/path$i c/apath$i; done
+test_expect_success \
+ 'add files into a new directory.' \
+ 'git update-index --add c/apath*'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "first set of changes"'
+
+mv c/* a/
+test_expect_success \
+ 'move all of the new dir contents into a preexisting dir.' \
+ 'git update-index --add --remove a/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'git diff-index --factorize-renames without full-dir rename.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 c/* a/
+EOF
+
+test_expect_failure \
+ 'validate the output for bulk rename without full-dir rename.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test moves to toplevel (seriously broken)
+
+mv c/* .
+test_expect_success \
+ 'move all of the new dir contents into toplevel.' \
+ 'git update-index --add --remove apath* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'git diff-index --factorize-renames files bulk-moved to toplevel.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 c/* ./
+EOF
+
+test_expect_failure \
+ 'validate the output for files bulk-moved to toplevel.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test renaming with subdirs (lacks hiding of renamed subdirs)
+
+mv c a/
+test_expect_success \
+ 'move the new dir as subdir of another.' \
+ 'git update-index --add --remove a/c/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "move as subdir"'
+
+mv a b
+echo foo >> b/c/apath0
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 a/c/apath0 a/c/apath1 a/c/apath2 b/path* b/c/apath*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames on a move including a subdir.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 00084e5ea68b5ae339b7c4b429e4a70fe25d069b R096 a/c/apath0 b/c/apath0
+EOF
+
+test_expect_failure \
+ 'validate the output for a move including a subdir.' \
+ 'compare_diff_patch current.filtered expected'
+
+# now test moving all files from toplevel into subdir (does not hides file moves) (needs consensus on syntax)
+#FIXME: maybe handle this as special case of move of a dir into one of its own subdirs ?
+
+git reset --hard HEAD~2
+
+mv a/* .
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 path*'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "move all files to toplevel"'
+
+mkdir z
+mv path* z/
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove path0 path1 path2 path3 z/path*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames everything from toplevel.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 ./* z/
+EOF
+
+test_expect_failure \
+ 'validate the output for a move of everything from toplevel.' \
+ 'compare_diff_patch current.filtered expected'
+
+test_done
^ permalink raw reply related
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Yann Dirson @ 2008-10-30 22:46 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Sam Vilain, Pierre Habouzit, Mike Hommey, Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.2.00.0810301423520.13034@xanadu.home>
On Thu, Oct 30, 2008 at 02:28:35PM -0400, Nicolas Pitre wrote:
> > It's not about magic, it's about sensible defaults. Currently this use
> > case is an error, and the resultant command is very long to type, and
> > involves typing the branch name twice. I end up writing things like:
> >
> > git checkout -b {,origin/}wr34251-do-something
> >
> > For the user who doesn't know to use the ksh-style {} blocks this is
> > voodoo. The longer form is cumbersome.
>
> This is no excuse for promoting semantics only useful in such special
> cases.
It is really not so rare to have an upstream repo with branches such
as "stable", "next" and the like. This syntax extension would make is
as straightforward to work on "stable" as it is on remote HEAD
(usually master, which has already been magically setup for you).
BTW this use case reminds me that the remote HEAD has its own special
treatment for "clone", which AFAIK cannot be overriden from
command-line (I still sometimes lack what cogito provided as "cg clone
URL#branch").
> As long as it checks it out with a detached head if it is a remote
> branch then I have no issue.
Yes it is possible, but that does not necessarily make a UI
improvement worthless.
Best regards,
--
Yann
^ permalink raw reply
* Re: [PATCH 1/2] Add --verbose|-v to test-chmtime
From: Jeff King @ 2008-10-30 22:58 UTC (permalink / raw)
To: Alex Riesen; +Cc: Git Mailing List, Junio C Hamano, René Scharfe
In-Reply-To: <81b0412b0810300426u2ccbe51at1bf5f989b6333ed1@mail.gmail.com>
On Thu, Oct 30, 2008 at 12:26:24PM +0100, Alex Riesen wrote:
> This allows us replace perl when getting the mtime of a file because
> of time zone conversions, though at the moment only one platform which
> does this has been identified: Cygwin when used with ActiveState Perl
> (as usual).
> [...]
> test-chmtime -v +0 filename1 | cut -f 1
Personally, I would have:
- split the argument refactoring and the addition of the "-v" argument
into two patches to make reviewers lives easier
- just used a special timespec that means "don't change anything, but
show show"
but I think those are mostly nitpicks, so I am OK with the series as-is.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox