* [JGIT PATCH 2/3] Don't allow DirCacheEntry with mode of 0
From: Shawn O. Pearce @ 2009-09-11 19:58 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Shawn O. Pearce, Adam W. Hawks
In-Reply-To: <1252699129-6961-1-git-send-email-spearce@spearce.org>
A 0 file mode in a DirCacheEntry is not a valid mode. To C git
such a value indicates the record should not be present. We already
were catching this bad state and exceptioning out when writing tree
objects to disk, but we did not fail when writing the dircache back
to disk. This allowed JGit applications to create a dircache file
which C git would not like to read.
Instead of checking the mode during writes, we now check during
mutation. This allows application bugs to be detected sooner and
closer to the cause site. It also allows us to avoid checking most
of the records which we read in from disk, as we can assume these
are formatted correctly.
Some of our unit tests were not setting the FileMode on their test
entry, so they had to be updated to use REGULAR_FILE.
Signed-off-by: Shawn O. Pearce <sop@google.com>
CC: Adam W. Hawks <awhawks@writeme.com>
---
.../spearce/jgit/dircache/DirCacheBasicTest.java | 5 ++-
.../spearce/jgit/dircache/DirCacheBuilderTest.java | 27 +++++++++++++--
.../spearce/jgit/dircache/DirCacheEntryTest.java | 37 ++++++++++++++++++++
.../spearce/jgit/dircache/DirCacheFindTest.java | 5 ++-
.../jgit/dircache/DirCacheIteratorTest.java | 4 ++-
.../jgit/dircache/DirCacheLargePathTest.java | 5 +++
.../spearce/jgit/dircache/DirCacheTreeTest.java | 13 +++++--
.../org/spearce/jgit/dircache/DirCacheBuilder.java | 8 +++--
.../org/spearce/jgit/dircache/DirCacheEditor.java | 11 ++++--
.../org/spearce/jgit/dircache/DirCacheEntry.java | 12 ++++++-
.../org/spearce/jgit/dircache/DirCacheTree.java | 4 --
11 files changed, 111 insertions(+), 20 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
index 4d737c0..3b48b11 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
@@ -40,6 +40,7 @@
import java.io.File;
import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.RepositoryTestCase;
public class DirCacheBasicTest extends RepositoryTestCase {
@@ -170,8 +171,10 @@ public void testBuildThenClear() throws Exception {
final String[] paths = { "a.", "a.b", "a/b", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final DirCacheBuilder b = dc.builder();
for (int i = 0; i < ents.length; i++)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBuilderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBuilderTest.java
index 2cf1d92..f64e4f6 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBuilderTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBuilderTest.java
@@ -59,6 +59,20 @@ public void testBuildEmpty() throws Exception {
}
}
+ public void testBuildRejectsUnsetFileMode() throws Exception {
+ final DirCache dc = DirCache.newInCore();
+ final DirCacheBuilder b = dc.builder();
+ assertNotNull(b);
+
+ final DirCacheEntry e = new DirCacheEntry("a");
+ assertEquals(0, e.getRawMode());
+ try {
+ b.add(e);
+ } catch (IllegalArgumentException err) {
+ assertEquals("FileMode not set for path a", err.getMessage());
+ }
+ }
+
public void testBuildOneFile_FinishWriteCommit() throws Exception {
final String path = "a-file-path";
final FileMode mode = FileMode.REGULAR_FILE;
@@ -162,6 +176,7 @@ public void testFindSingleFile() throws Exception {
assertNotNull(b);
final DirCacheEntry entOrig = new DirCacheEntry(path);
+ entOrig.setFileMode(FileMode.REGULAR_FILE);
assertNotSame(path, entOrig.getPathString());
assertEquals(path, entOrig.getPathString());
b.add(entOrig);
@@ -185,8 +200,10 @@ public void testAdd_InGitSortOrder() throws Exception {
final String[] paths = { "a.", "a.b", "a/b", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final DirCacheBuilder b = dc.builder();
for (int i = 0; i < ents.length; i++)
@@ -207,8 +224,10 @@ public void testAdd_ReverseGitSortOrder() throws Exception {
final String[] paths = { "a.", "a.b", "a/b", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final DirCacheBuilder b = dc.builder();
for (int i = ents.length - 1; i >= 0; i--)
@@ -229,8 +248,10 @@ public void testBuilderClear() throws Exception {
final String[] paths = { "a.", "a.b", "a/b", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
{
final DirCacheBuilder b = dc.builder();
for (int i = 0; i < ents.length; i++)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
index a6ff5a8..971f201 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
@@ -40,6 +40,7 @@
import junit.framework.TestCase;
import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.FileMode;
public class DirCacheEntryTest extends TestCase {
public void testIsValidPath() {
@@ -112,4 +113,40 @@ public void testCreate_ByStringPathAndStage() {
assertEquals("Invalid stage 4 for path a", err.getMessage());
}
}
+
+ public void testSetFileMode() {
+ final DirCacheEntry e = new DirCacheEntry("a");
+
+ assertEquals(0, e.getRawMode());
+
+ e.setFileMode(FileMode.REGULAR_FILE);
+ assertSame(FileMode.REGULAR_FILE, e.getFileMode());
+ assertEquals(FileMode.REGULAR_FILE.getBits(), e.getRawMode());
+
+ e.setFileMode(FileMode.EXECUTABLE_FILE);
+ assertSame(FileMode.EXECUTABLE_FILE, e.getFileMode());
+ assertEquals(FileMode.EXECUTABLE_FILE.getBits(), e.getRawMode());
+
+ e.setFileMode(FileMode.SYMLINK);
+ assertSame(FileMode.SYMLINK, e.getFileMode());
+ assertEquals(FileMode.SYMLINK.getBits(), e.getRawMode());
+
+ e.setFileMode(FileMode.GITLINK);
+ assertSame(FileMode.GITLINK, e.getFileMode());
+ assertEquals(FileMode.GITLINK.getBits(), e.getRawMode());
+
+ try {
+ e.setFileMode(FileMode.MISSING);
+ fail("incorrectly accepted FileMode.MISSING");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid mode 0 for path a", err.getMessage());
+ }
+
+ try {
+ e.setFileMode(FileMode.TREE);
+ fail("incorrectly accepted FileMode.TREE");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid mode 40000 for path a", err.getMessage());
+ }
+ }
}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheFindTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheFindTest.java
index 0eb0302..470c80a 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheFindTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheFindTest.java
@@ -37,6 +37,7 @@
package org.spearce.jgit.dircache;
+import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.RepositoryTestCase;
public class DirCacheFindTest extends RepositoryTestCase {
@@ -45,8 +46,10 @@ public void testEntriesWithin() throws Exception {
final String[] paths = { "a.", "a/b", "a/c", "a/d", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final int aFirst = 1;
final int aLast = 3;
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheIteratorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheIteratorTest.java
index 047c989..71581dc 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheIteratorTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheIteratorTest.java
@@ -68,8 +68,10 @@ public void testNoSubtree_NoTreeWalk() throws Exception {
final String[] paths = { "a.", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final DirCacheBuilder b = dc.builder();
for (int i = 0; i < ents.length; i++)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheLargePathTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheLargePathTest.java
index 4ea286c..a9945f1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheLargePathTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheLargePathTest.java
@@ -40,6 +40,7 @@
import java.io.IOException;
import org.spearce.jgit.errors.CorruptObjectException;
+import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.RepositoryTestCase;
public class DirCacheLargePathTest extends RepositoryTestCase {
@@ -70,6 +71,10 @@ private void testLongPath(final int len) throws CorruptObjectException,
final DirCacheEntry longEnt = new DirCacheEntry(longPath);
final DirCacheEntry shortEnt = new DirCacheEntry(shortPath);
+
+ longEnt.setFileMode(FileMode.REGULAR_FILE);
+ shortEnt.setFileMode(FileMode.REGULAR_FILE);
+
assertEquals(longPath, longEnt.getPathString());
assertEquals(shortPath, shortEnt.getPathString());
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheTreeTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheTreeTest.java
index aca0b90..6efa207 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheTreeTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheTreeTest.java
@@ -40,6 +40,7 @@
import java.io.IOException;
import org.spearce.jgit.errors.CorruptObjectException;
+import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.RepositoryTestCase;
public class DirCacheTreeTest extends RepositoryTestCase {
@@ -75,8 +76,10 @@ public void testSingleSubtree() throws Exception {
final String[] paths = { "a.", "a/b", "a/c", "a/d", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final int aFirst = 1;
final int aLast = 3;
@@ -110,8 +113,10 @@ public void testTwoLevelSubtree() throws Exception {
final String[] paths = { "a.", "a/b", "a/c/e", "a/c/f", "a/d", "a0b" };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final int aFirst = 1;
final int aLast = 4;
final int acFirst = 2;
@@ -167,8 +172,10 @@ public void testWriteReadTree() throws CorruptObjectException, IOException {
final String B = String.format("b%2000s", "b");
final String[] paths = { A + ".", A + "." + B, A + "/" + B, A + "0" + B };
final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
- for (int i = 0; i < paths.length; i++)
+ for (int i = 0; i < paths.length; i++) {
ents[i] = new DirCacheEntry(paths[i]);
+ ents[i].setFileMode(FileMode.REGULAR_FILE);
+ }
final DirCacheBuilder b = dc.builder();
for (int i = 0; i < ents.length; i++)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheBuilder.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheBuilder.java
index aee12fb..8acb3d0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheBuilder.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheBuilder.java
@@ -41,7 +41,6 @@
import java.util.Arrays;
import org.spearce.jgit.lib.AnyObjectId;
-import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.WindowCursor;
import org.spearce.jgit.treewalk.AbstractTreeIterator;
@@ -90,8 +89,13 @@ protected DirCacheBuilder(final DirCache dc, final int ecnt) {
*
* @param newEntry
* the new entry to add.
+ * @throws IllegalArgumentException
+ * If the FileMode of the entry was not set by the caller.
*/
public void add(final DirCacheEntry newEntry) {
+ if (newEntry.getRawMode() == 0)
+ throw new IllegalArgumentException("FileMode not set for path "
+ + newEntry.getPathString());
beforeAdd(newEntry);
fastAdd(newEntry);
}
@@ -187,8 +191,6 @@ public void finish() {
}
private void beforeAdd(final DirCacheEntry newEntry) {
- if (FileMode.TREE.equals(newEntry.getRawMode()))
- throw bad(newEntry, "Adding subtree not allowed");
if (sorted && entryCnt > 0) {
final DirCacheEntry lastEntry = entries[entryCnt - 1];
final int cr = DirCache.cmp(lastEntry, newEntry);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEditor.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEditor.java
index 10b554e..6eaceb7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEditor.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEditor.java
@@ -138,11 +138,16 @@ private void applyEdits() {
}
final DirCacheEntry ent;
- if (missing)
+ if (missing) {
ent = new DirCacheEntry(e.path);
- else
+ e.apply(ent);
+ if (ent.getRawMode() == 0)
+ throw new IllegalArgumentException("FileMode not set"
+ + " for path " + ent.getPathString());
+ } else {
ent = cache.getEntry(eIdx);
- e.apply(ent);
+ e.apply(ent);
+ }
fastAdd(ent);
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
index d7abd6e..872ef33 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
@@ -377,11 +377,21 @@ public FileMode getFileMode() {
/**
* Set the file mode for this entry.
- *
+ *
* @param mode
* the new mode constant.
+ * @throws IllegalArgumentException
+ * If {@code mode} is {@link FileMode#MISSING},
+ * {@link FileMode#TREE}, or any other type code not permitted
+ * in a tree object.
*/
public void setFileMode(final FileMode mode) {
+ switch (mode.getBits() & FileMode.TYPE_MASK) {
+ case FileMode.TYPE_MISSING:
+ case FileMode.TYPE_TREE:
+ throw new IllegalArgumentException("Invalid mode " + mode
+ + " for path " + getPathString());
+ }
NB.encodeInt32(info, infoOffset + P_MODE, mode.getBits());
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheTree.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheTree.java
index 79e95cb..2f2a5ed 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheTree.java
@@ -376,10 +376,6 @@ private int computeSize(final DirCacheEntry[] cache, int cIdx,
}
final FileMode mode = e.getFileMode();
- if (mode.getObjectType() == Constants.OBJ_BAD)
- throw new IllegalStateException("Entry \"" + e.getPathString()
- + "\" has incorrect mode set up.");
-
size += mode.copyToLength();
size += ep.length - pathOffset;
size += OBJECT_ID_LENGTH + 2;
--
1.6.5.rc0.164.g5f6b0
^ permalink raw reply related
* [JGIT PATCH 1/3] Disallow creating invalid DirCacheEntry records
From: Shawn O. Pearce @ 2009-09-11 19:58 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Shawn O. Pearce, Adam W. Hawks
A dircache record must not use a path string like "/a" or "a//b"
as this results in a tree entry being written with a zero length
name component in the record. C git does not support an empty name,
and neither does any modern filesystem.
A record also must not have a stage outside of the standard 0-3
value range, as there are only 2 bits of space available in the
on-disk format of the record to store the stage information.
Any other values would be truncated into this space, storing a
different value than the caller expected.
If an application tries to create a DirCache record with either of
these wrong values, we abort with an IllegalArgumentException.
Signed-off-by: Shawn O. Pearce <sop@google.com>
CC: Adam W. Hawks <awhawks@writeme.com>
---
.../spearce/jgit/dircache/DirCacheEntryTest.java | 115 ++++++++++++++++++++
.../org/spearce/jgit/dircache/DirCacheEntry.java | 55 +++++++++-
2 files changed, 169 insertions(+), 1 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
new file mode 100644
index 0000000..a6ff5a8
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheEntryTest.java
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.dircache;
+
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class DirCacheEntryTest extends TestCase {
+ public void testIsValidPath() {
+ assertTrue(isValidPath("a"));
+ assertTrue(isValidPath("a/b"));
+ assertTrue(isValidPath("ab/cd/ef"));
+
+ assertFalse(isValidPath(""));
+ assertFalse(isValidPath("/a"));
+ assertFalse(isValidPath("a//b"));
+ assertFalse(isValidPath("ab/cd//ef"));
+ assertFalse(isValidPath("a/"));
+ assertFalse(isValidPath("ab/cd/ef/"));
+ assertFalse(isValidPath("a\u0000b"));
+ }
+
+ private static boolean isValidPath(final String path) {
+ return DirCacheEntry.isValidPath(Constants.encode(path));
+ }
+
+ public void testCreate_ByStringPath() {
+ assertEquals("a", new DirCacheEntry("a").getPathString());
+ assertEquals("a/b", new DirCacheEntry("a/b").getPathString());
+
+ try {
+ new DirCacheEntry("/a");
+ fail("Incorrectly created DirCacheEntry");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid path: /a", err.getMessage());
+ }
+ }
+
+ public void testCreate_ByStringPathAndStage() {
+ DirCacheEntry e;
+
+ e = new DirCacheEntry("a", 0);
+ assertEquals("a", e.getPathString());
+ assertEquals(0, e.getStage());
+
+ e = new DirCacheEntry("a/b", 1);
+ assertEquals("a/b", e.getPathString());
+ assertEquals(1, e.getStage());
+
+ e = new DirCacheEntry("a/c", 2);
+ assertEquals("a/c", e.getPathString());
+ assertEquals(2, e.getStage());
+
+ e = new DirCacheEntry("a/d", 3);
+ assertEquals("a/d", e.getPathString());
+ assertEquals(3, e.getStage());
+
+ try {
+ new DirCacheEntry("/a", 1);
+ fail("Incorrectly created DirCacheEntry");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid path: /a", err.getMessage());
+ }
+
+ try {
+ new DirCacheEntry("a", -11);
+ fail("Incorrectly created DirCacheEntry");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid stage -11 for path a", err.getMessage());
+ }
+
+ try {
+ new DirCacheEntry("a", 4);
+ fail("Incorrectly created DirCacheEntry");
+ } catch (IllegalArgumentException err) {
+ assertEquals("Invalid stage 4 for path a", err.getMessage());
+ }
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
index 47b1cc5..d7abd6e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCacheEntry.java
@@ -166,6 +166,10 @@
*
* @param newPath
* name of the cache entry.
+ * @throws IllegalArgumentException
+ * If the path starts or ends with "/", or contains "//" either
+ * "\0". These sequences are not permitted in a git tree object
+ * or DirCache file.
*/
public DirCacheEntry(final String newPath) {
this(Constants.encode(newPath));
@@ -178,6 +182,11 @@ public DirCacheEntry(final String newPath) {
* name of the cache entry.
* @param stage
* the stage index of the new entry.
+ * @throws IllegalArgumentException
+ * If the path starts or ends with "/", or contains "//" either
+ * "\0". These sequences are not permitted in a git tree object
+ * or DirCache file. Or if {@code stage} is outside of the
+ * range 0..3, inclusive.
*/
public DirCacheEntry(final String newPath, final int stage) {
this(Constants.encode(newPath), stage);
@@ -188,6 +197,10 @@ public DirCacheEntry(final String newPath, final int stage) {
*
* @param newPath
* name of the cache entry, in the standard encoding.
+ * @throws IllegalArgumentException
+ * If the path starts or ends with "/", or contains "//" either
+ * "\0". These sequences are not permitted in a git tree object
+ * or DirCache file.
*/
public DirCacheEntry(final byte[] newPath) {
this(newPath, STAGE_0);
@@ -200,8 +213,20 @@ public DirCacheEntry(final byte[] newPath) {
* name of the cache entry, in the standard encoding.
* @param stage
* the stage index of the new entry.
+ * @throws IllegalArgumentException
+ * If the path starts or ends with "/", or contains "//" either
+ * "\0". These sequences are not permitted in a git tree object
+ * or DirCache file. Or if {@code stage} is outside of the
+ * range 0..3, inclusive.
*/
public DirCacheEntry(final byte[] newPath, final int stage) {
+ if (!isValidPath(newPath))
+ throw new IllegalArgumentException("Invalid path: "
+ + toString(newPath));
+ if (stage < 0 || 3 < stage)
+ throw new IllegalArgumentException("Invalid stage " + stage
+ + " for path " + toString(newPath));
+
info = new byte[INFO_LEN];
infoOffset = 0;
path = newPath;
@@ -461,7 +486,7 @@ public void setObjectIdFromRaw(final byte[] bs, final int p) {
* returned string.
*/
public String getPathString() {
- return Constants.CHARSET.decode(ByteBuffer.wrap(path)).toString();
+ return toString(path);
}
/**
@@ -492,4 +517,32 @@ private void encodeTS(final int pIdx, final long when) {
NB.encodeInt32(info, base, (int) (when / 1000));
NB.encodeInt32(info, base + 4, ((int) (when % 1000)) * 1000000);
}
+
+ private static String toString(final byte[] path) {
+ return Constants.CHARSET.decode(ByteBuffer.wrap(path)).toString();
+ }
+
+ static boolean isValidPath(final byte[] path) {
+ if (path.length == 0)
+ return false; // empty path is not permitted.
+
+ boolean componentHasChars = false;
+ for (final byte c : path) {
+ switch (c) {
+ case 0:
+ return false; // NUL is never allowed within the path.
+
+ case '/':
+ if (componentHasChars)
+ componentHasChars = false;
+ else
+ return false;
+ break;
+
+ default:
+ componentHasChars = true;
+ }
+ }
+ return componentHasChars;
+ }
}
--
1.6.5.rc0.164.g5f6b0
^ permalink raw reply related
* [PATCH v3 2/2] add documentation for mailinfo.scissors and '--no-scissors'
From: Nicolas Sebrecht @ 2009-09-11 19:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git, Nicolas Sebrecht
In-Reply-To: <682ef47420f36d8c53e42981370d377b621d7b86.1252698215.git.nicolas.s.dev@gmx.fr>
The 11/09/09, Junio C Hamano wrote:
> Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:
>
> > +---no-scissors::
> > + Do not obey to a scissors line (see linkgit:git-mailinfo[1]).
> > +
>
> obey is v.t. so "do not obey a scissors line" would be grammatical; I
> think "ignore scissors lines" would be better.
>
> > +--no-scissors::
> > + Do not obey to a scissors line. This is only useful if mailinfo.scissors is
> > + enabled (see --scissors).
>
> Ditto; also it is useful in general if you do not know which way it is
> configured. Saying "_only_" is misleading.
>
> Ignore scissors lines; useful for overriding mailinfo.scissors
> settings.
>
> > diff --git a/git-am.sh b/git-am.sh
> > index 26ffe70..f242d1a 100755
> > --- a/git-am.sh
> > +++ b/git-am.sh
> > @@ -16,6 +16,7 @@ s,signoff add a Signed-off-by line to the commit message
> > u,utf8 recode into utf8 (default)
> > k,keep pass -k flag to git-mailinfo
> > c,scissors strip everything before a scissors line
> > +no-scissors don't obey to a scissors line (default)
> > whitespace= pass it through git-apply
> > ignore-space-change pass it through git-apply
> > ignore-whitespace pass it through git-apply
>
> Do we want it to allow --no-no-scissors? I do not think this hunk is
> necessary at all.
This version includes the above comments.
Thanks,
-- >8 --
Signed-off-by: Nicolas Sebrecht <nicolas.s.dev@gmx.fr>
---
Documentation/git-am.txt | 5 ++++-
Documentation/git-mailinfo.txt | 5 +++++
builtin-mailinfo.c | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 87781f4..06e6ea6 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[--3way] [--interactive] [--committer-date-is-author-date]
[--ignore-date] [--ignore-space-change | --ignore-whitespace]
[--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
- [--reject] [-q | --quiet] [--scissors]
+ [--reject] [-q | --quiet] [--scissors | --no-scissors]
[<mbox> | <Maildir>...]
'git am' (--skip | --resolved | --abort)
@@ -44,6 +44,9 @@ OPTIONS
Remove everything in body before a scissors line (see
linkgit:git-mailinfo[1]).
+---no-scissors::
+ ignore scissors lines (see linkgit:git-mailinfo[1]).
+
-q::
--quiet::
Be quiet. Only print error messages.
diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
index 823ab82..d1f9cb8 100644
--- a/Documentation/git-mailinfo.txt
+++ b/Documentation/git-mailinfo.txt
@@ -62,6 +62,11 @@ This is useful if you want to begin your message in a discussion thread
with comments and suggestions on the message you are responding to, and to
conclude it with a patch submission, separating the discussion and the
beginning of the proposed commit log message with a scissors line.
++
+This can enabled by default with the configuration option mailinfo.scissors.
+
+--no-scissors::
+ ignore scissors lines; useful for overriding mailinfo.scissors settings.
<msg>::
The commit log message extracted from e-mail, usually
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 7d22fd7..d498b1c 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -1004,7 +1004,7 @@ static int git_mailinfo_config(const char *var, const char *value, void *unused)
}
static const char mailinfo_usage[] =
- "git mailinfo [-k] [-u | --encoding=<encoding> | -n] [--scissors] msg patch < mail >info";
+ "git mailinfo [-k] [-u | --encoding=<encoding> | -n] [--scissors | --no-scissors] msg patch < mail >info";
int cmd_mailinfo(int argc, const char **argv, const char *prefix)
{
--
1.6.5.rc0.166.g46a82
^ permalink raw reply related
* Re: [PATCH v2] Re: add documentation for mailinfo.scissors and '--no-scissors'
From: Junio C Hamano @ 2009-09-11 18:53 UTC (permalink / raw)
To: Nicolas Sebrecht; +Cc: Nanako Shiraishi, git
In-Reply-To: <20090911134112.GA18684@vidovic>
Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:
> I don't understand your point here. This hunk doesn't allow
> --no-no-scissors but add --no-scissors to usage of 'git am', no?
>
> Do I miss something around $OPTIONS_SPEC?
You missed two thirds of the issue and I missed the other one third ;-)
As scissors is not marked with ! (see PARSEOPT section in the rev-parse
documentation), no-scissors is already available without that hunk, and
that is why your patch is unnecessary.
Your patch did not mark no-scissors with ! either, so it makes the call to
rev-parse --parseopt to process your OPTIONS_SPEC pass --no-no-scissors.
However, you did not add --no-no-scissors arm to the case statement that
processes rev-parse --parseopt output, so the command as the whole still
rejects --no-no-scissors as invalid (this is the last third I missed).
^ permalink raw reply
* Re: one half of a rebase
From: Matthieu Moy @ 2009-09-11 18:39 UTC (permalink / raw)
To: Geoffrey Irving; +Cc: git, Dylan Simon
In-Reply-To: <7f9d599f0909111025q42e3cdc6vba602b84c1d81215@mail.gmail.com>
Geoffrey Irving <irving@naml.us> writes:
> If I could do (2) as a separate operation, it would look something like
>
> git cherry-pick-all topic
I believe
git rebase --onto master master topic
git update-ref master topic
would do the trick.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH 2/2] pager: set LESS=FRSX also on Windows
From: Johannes Sixt @ 2009-09-11 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: msysgit, Alexey Borzenkov, git
In-Reply-To: <200909111940.08652.j6t@kdbg.org>
Previously, this environment variable was set in the pager_preexec
callback, which is conditionally-compiled only on Unix, because it is not,
and cannot be, called on Windows.
With this patch the env member of struct child_process is used to set
the environment variable, which also works on Windows.
Noticed by Alexey Borzenkov.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Alexey,
this will result in a conflict when you apply it to your msysgit because
there is an extra patch that shuffled the setup of pager_process around.
-- Hannes
| 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
--git a/pager.c b/pager.c
index 4921843..f416d38 100644
--- a/pager.c
+++ b/pager.c
@@ -21,8 +21,6 @@ static void pager_preexec(void)
FD_ZERO(&in);
FD_SET(0, &in);
select(1, &in, NULL, &in, NULL);
-
- setenv("LESS", "FRSX", 0);
}
#endif
@@ -70,6 +68,10 @@ void setup_pager(void)
pager_argv[2] = pager;
pager_process.argv = pager_argv;
pager_process.in = -1;
+ if (!getenv("LESS")) {
+ static const char *env[] = { "LESS=FRSX", NULL };
+ pager_process.env = env;
+ }
#ifndef __MINGW32__
pager_process.preexec_cb = pager_preexec;
#endif
--
1.6.5.rc0.28.gfb9b
^ permalink raw reply related
* [PATCH 1/2] start_command: do not clobber cmd->env on Windows code path
From: Johannes Sixt @ 2009-09-11 17:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: msysgit, Alexey Borzenkov, git
In-Reply-To: <1252560077-1725-1-git-send-email-snaury@gmail.com>
Previously, it would not be possible to call start_command twice for the
same struct child_process that has env set.
The fix is achieved by moving the loop that modifies the environment block
into a helper function. This also allows us to make two other helper
functions static.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
We don't call start_command twice in a row anywhere in git.git, but
msysgit has a patch that does, and with the next patch the buglet would
be triggered.
Even after this patch, other members of *cmd are clobbered, so it could
be argued that this patch is unnecessary, but at least the Windows code
path now keeps the same members that the Unix code path keeps, and it
makes start_command easier to read by moving a loop into a helper.
-- Hannes
compat/mingw.c | 16 ++++++++++++++--
compat/mingw.h | 3 +--
run-command.c | 7 ++-----
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index bed4178..36ef8d3 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -824,7 +824,7 @@ void mingw_execvp(const char *cmd, char *const *argv)
free_path_split(path);
}
-char **copy_environ()
+static char **copy_environ(void)
{
char **env;
int i = 0;
@@ -861,7 +861,7 @@ static int lookup_env(char **env, const char *name, size_t nmln)
/*
* If name contains '=', then sets the variable, otherwise it unsets it
*/
-char **env_setenv(char **env, const char *name)
+static char **env_setenv(char **env, const char *name)
{
char *eq = strchrnul(name, '=');
int i = lookup_env(env, name, eq-name);
@@ -886,6 +886,18 @@ char **env_setenv(char **env, const char *name)
return env;
}
+/*
+ * Copies global environ and adjusts variables as specified by vars.
+ */
+char **make_augmented_environ(const char *const *vars)
+{
+ char **env = copy_environ();
+
+ while (*vars)
+ env = env_setenv(env, *vars++);
+ return env;
+}
+
/* this is the first function to call into WS_32; initialize it */
#undef gethostbyname
struct hostent *mingw_gethostbyname(const char *host)
diff --git a/compat/mingw.h b/compat/mingw.h
index 948de66..c43917c 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -222,9 +222,8 @@ void mingw_open_html(const char *path);
* helpers
*/
-char **copy_environ(void);
+char **make_augmented_environ(const char *const *vars);
void free_environ(char **env);
-char **env_setenv(char **env, const char *name);
/*
* A replacement of main() that ensures that argv[0] has a path
diff --git a/run-command.c b/run-command.c
index f3e7abb..ac314a5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -173,11 +173,8 @@ fail_pipe:
if (cmd->dir)
die("chdir in start_command() not implemented");
- if (cmd->env) {
- env = copy_environ();
- for (; *cmd->env; cmd->env++)
- env = env_setenv(env, *cmd->env);
- }
+ if (cmd->env)
+ env = make_augmented_environ(cmd->env);
if (cmd->git_cmd) {
cmd->argv = prepare_git_cmd(cmd->argv);
--
1.6.5.rc0.28.gfb9b
^ permalink raw reply related
* one half of a rebase
From: Geoffrey Irving @ 2009-09-11 17:25 UTC (permalink / raw)
To: git; +Cc: Dylan Simon
If I'm on branch topic and do "git rebase master", git performs two operations:
1. Rewind the current head to master.
2. For each commit in topic that isn't in master, cherry pick it onto
the current head.
I would like to be able to do (2) as a separate operation. For
example, if I start out on branch master and want to get all of
topic's commits on top of the current head, I currently do
git checkout topic
git rebase master
git branch -d master
git branch -m master
If I could do (2) as a separate operation, it would look something like
git cherry-pick-all topic
which is simpler and faster since it avoids switching files back and
forth (master to topic and back). Is there a robust way to achieve
the cherry-pick-all semantics with current commands? If not, how
difficult would it be to partition rebase accordingly?
Thanks,
Geoffrey
^ permalink raw reply
* Re: bash_completion outside repo
From: Jeff King @ 2009-09-11 16:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: james bardin, Todd Zullinger, git
In-Reply-To: <20090911150934.GB977@coredump.intra.peff.net>
On Fri, Sep 11, 2009 at 11:09:34AM -0400, Jeff King wrote:
> Assuming you make such a patch, that will clear up the original issue. I
> wonder if we should fix "git config --list". The current semantics seem
> a little crazy to me, but it is a scriptable interface. I'm inclined to
> call this a bug, though.
And here is a patch to fix it.
-- >8 --
Subject: [PATCH] config: treat non-existent config files as empty
The git_config() function signals error by returning -1 in
two instances:
1. An actual error occurs in opening a config file (parse
errors cause an immediate die).
2. Of the three possible config files, none was found.
However, this second case is often not an error at all; it
simply means that the user has no configuration (they are
outside a repo, and they have no ~/.gitconfig file). This
can lead to confusing errors, such as when the bash
completion calls "git config --list" outside of a repo. If
the user has a ~/.gitconfig, the command completes
succesfully; if they do not, it complains to stderr.
This patch allows callers of git_config to distinguish
between the two cases. Error is signaled by -1, and
otherwise the return value is the number of files parsed.
This means that the traditional "git_config(...) < 0" check
for error should work, but callers who want to know whether
we parsed any files or not can still do so.
We need to tweak one use of git_config in builtin-remote
that previously assumed the return value was either '0' or
'-1'.
Signed-off-by: Jeff King <peff@peff.net>
---
This is actually a bit overengineered. Of the hundreds of calls to
git_config, there are exactly _two_ which check the return value. And
neither of them cares whether we parsed files or not; they really only
care if there was an error. So we could simply return 0 as long as there
is no error.
This also makes me wonder, though. Git can do wildly different things
(including hard-to-reverse things) based on config (e.g., just consider
gc.pruneExpire). Yet we call git_config() without ever checking for
errors. In the actual parsing routines, we die() if there is an error.
But if we fail to open the file due to some transient error, we will
silently ignore the situation.
Granted, such transient errors are unlikely. The biggest reasons for
failing to open a file are that it doesn't exist, or that we have no
permission to read it, both of which are treated explicitly in
git_config as "silently ok". But I wonder if we should simply be dying
on such an error, and git_config() should just have a void return.
builtin-remote.c | 3 ++-
config.c | 4 +---
t/t1300-repo-config.sh | 8 ++++++++
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 0777dd7..3bf1fe8 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1245,7 +1245,8 @@ static int update(int argc, const char **argv)
for (i = 1; i < argc; i++) {
int groups_found = 0;
remote_group.name = argv[i];
- result = git_config(get_remote_group, &groups_found);
+ if (git_config(get_remote_group, &groups_found) < 0)
+ result = -1;
if (!groups_found && (i != 1 || strcmp(argv[1], "default"))) {
struct remote *remote;
if (!remote_is_configured(argv[i]))
diff --git a/config.c b/config.c
index e87edea..e429674 100644
--- a/config.c
+++ b/config.c
@@ -709,9 +709,7 @@ int git_config(config_fn_t fn, void *data)
found += 1;
}
free(repo_config);
- if (found == 0)
- return -1;
- return ret;
+ return ret == 0 ? found : ret;
}
/*
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 83b7294..db987b7 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -289,6 +289,14 @@ test_expect_success 'working --list' \
'git config --list > output && cmp output expect'
cat > expect << EOF
+EOF
+
+test_expect_success '--list without repo produces empty output' '
+ git --git-dir=nonexistent config --list >output &&
+ test_cmp expect output
+'
+
+cat > expect << EOF
beta.noindent sillyValue
nextsection.nonewline wow2 for me
EOF
--
1.6.5.rc0.174.g29a6d.dirty
^ permalink raw reply related
* Re: [RFC/PATCH 2/2] gitweb: check given hash before trying to create snapshot
From: Jakub Narebski @ 2009-09-11 16:42 UTC (permalink / raw)
To: Mark Rada; +Cc: git, Junio C Hamano
In-Reply-To: <9513F576-4154-4281-8545-81841D59B766@mailservices.uwaterloo.ca>
[This mail was very strangely wrapped; I fixed this for readability]
On Fri, 11 Sep 2009, Mark Rada wrote:
> On 2009-09-11, at 3:52 AM, Jakub Narebski wrote:
>
>> Second, I'd rather have better names for snapshots than using full SHA-1.
>> For snapshot of 'v1.5.0' of repository 'repo.git' I'd prefer for snapshot
>> to be named 'repo-v1.5.0', and for snapshot of 'next' branch of the
>> same project to be named for example 'repo-next-20090909', or perhaps
>> 'repo-next-2009-09-10T09:16:18' or 'repo-next-20090909-g5f6b0ff',
>> or 'repo-v1.6.5-rc0-164-g5f6b0ff'.
>
> Ah, yeah, well, I let $hash still hold the originally passed value,
> which would be used to create the outputted file name (unless
> overwritten by the client by using curl -o or something).
Or by specifying different file name than proposed by browser.
> Then $snapshot holds the full hash. This way, if you were to type
> something like
>
> http://git.kernel.org/?p=git/git.git;a=snapshot;h=next;sf=tgz
>
> into your browser window, you would get git.git-next.tar.gz back, but
> the backend could look up something like
>
> git-5f6b0ffff13f5cd762d0a5a4e1c4dede58e8a537.tar.gz
>
> using the $snapshot variable in some hypothetical cache (or even
> without the cache it won't mangle the nicer name $hash might have).
O.K.
>
> Also, right now gitweb will not accept tags for hashes. This seems to be
> because it passes the --verify option to rev-parse, but the output
> from using and not using the verify option seems to be the same (other
> than also accepting all tree-ishes). Could you let me know if there is
> a good reason not to take off the --verify option? Otherwise, I would
> like to take it off in the next version of this patch.
Errr, what?
$ 5096:[gitweb/web@git]# git rev-parse --verify v1.5.0
6db027ffe03210324939b3dd655c4223ca023b45
$ git rev-parse --verify refs/tags/v1.5.0
6db027ffe03210324939b3dd655c4223ca023b45
So it works as intended. The problem must be in some other place.
The '--verify' option is needed because git-rev-parse would otherwise
pass parameters it does not understand 'as is'. Compare
$ git rev-parse --verify v9.9.9 2>/dev/null
$ git rev-parse v9.9.9 2>/dev/null
v9.9.9
>
> Your point about adding the short hash to snapshots of branch heads is
> cool, I'll try that for the next version of the patch.
I think it would be better left for a _separate_ patch, as it is
separate feature (and I guess more complicated one).
>>> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-
>>> gitweb-standalone-http-status.sh
>>> index d0ff21d..4f8f147 100644
>>> --- a/t/t9501-gitweb-standalone-http-status.sh
>>> +++ b/t/t9501-gitweb-standalone-http-status.sh
>>> @@ -75,4 +75,30 @@ test_expect_success \
>>> test_debug 'cat gitweb.output'
>>>
>>>
>>> +test_expect_success \
>>> + 'snapshots: bad treeish id' \
>>> + 'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
>>> + grep "400 - Not a valid hash id:" gitweb.output'
>>> +test_debug 'cat gitweb.output'
>>> +
>>> +test_expect_success \
>>> + 'snapshots: good treeish id' \
>>> + 'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
>>> + grep "Status: 200 OK" gitweb.output'
>>> +test_debug 'cat gitweb.output'
>>
>> Why you don't check for "Status: 400" too?
>
> I'm not sure which test you are referring to (I think the second). The
> second test is valid and should return a nice .git-master.tar.gz
> tarball.
The output of CGI script like gitweb (and therefore gitweb.output file,
as it is generated now) contains both HTTP headers, separated by single
empty CRLF delimited line from the proper output of a script.
In first test you check that _contents_ contain specific error message,
but you do not check if HTTP status code matches it (it should, because
of how die_error works). In second test you check HTTP status. If the
t/t9501-gitweb-standalone-http-status.sh is to be about status, I guess
that you should check HTTP status, and not contents of the page (which
is more likely to change, e.g. due to some prettifying).
In t9501 tests you need, I think, only the HTTP headers part, unless
you want also to check that the contents matches. There was some sed
script shown to extract only HTTP headers.
>>> Second, any given treeish will always be translated to the full length,
>>> unambiguous, hash id; this will be useful for things like creating
>>> unique names for snapshot caches.
>>
>> But this is not a good idea, IMHO.
>>
>> First, it introduces feature that nobody uses (at least yet); we can
>> introduce this feature when it is needed instead.
>
> Sorry for promoting vapourware, I did originally rip this patch out from
> something else. I removed the comment from the v2 commit message.
Ah. O.K. then.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Cannot clone redirecting stdout
From: Jeff King @ 2009-09-11 16:20 UTC (permalink / raw)
To: Johan Sørensen; +Cc: Aloisio, git, support
In-Reply-To: <20090911160510.GA10848@coredump.intra.peff.net>
On Fri, Sep 11, 2009 at 12:05:10PM -0400, Jeff King wrote:
> Ah. I have a theory. If I do a clone of git://gitorious.org/qt/qt.git,
> the counting/compressing stages take a long time (I timed it at 1m40
> before it actually sends any data). And looking at upload-pack.c, we
> leave the 30-second alarm set while creating the pack. Meaning we die 30
> seconds into creating the pack.
>
> When progress is being displayed, however, the progress timer actually
> uses SIGALRM, as well. So we are constantly resetting the timer and it
> never goes off.
Hmm. Actually, this is not quite right. It looks like we call out to
pack-objects as an external program, so there is no conflict with the
signal. And we do proxy the output of pack-objects, which will keep our
timer resetting every time we see a chunk of output. But pack-objects
produces no output during the deltification phase, unless progress is
turned on. So we still hit our timeout in upload-pack during that
phase.
So our options are:
1. Turn off the timer during deltification, which could mean that it
would potentially go forever. But it's not controlled by the user.
I think the 'timeout' feature is really about the client just
opening the connection and sitting.
2. Keep progress on during deltification, but just throw it away
instead of relaying it if no-progress is in effect.
3. Accept that hitting the timeout during deltification _should_ cause
it to die. In that case, then the case with progress is wrong, and
we should stop resetting the timer just because we got some
progress output from pack-objects. But this may be redefining the
intent of --timeout. I don't know what the original intent was, or
what users of the feature are expecting.
-Peff
^ permalink raw reply
* Re: Cannot clone redirecting stdout
From: Jeff King @ 2009-09-11 16:05 UTC (permalink / raw)
To: Johan Sørensen; +Cc: Aloisio, git, support
In-Reply-To: <9e0f31700909110846h54959ae6u466ceda40799ba37@mail.gmail.com>
On Fri, Sep 11, 2009 at 05:46:23PM +0200, Johan Sørensen wrote:
> Some quick tests seem to indicate it's related to the fact that our
> wonderful little fork+exec git-daemon[1] (which is different from the
> one distributed with git) exec's to "git-upload-pack --strict
> --timeout=30 /path/to/repo". Now, why exactly that'll trigger when the
> no-progress flag is given I'm not sure of. The daemon itself execs as
> soon as it figures out what repo the client requested, so apart from
> the timeout the only thing it reacts to is the header (the
> "${headersize}git-upload-pack project/repo.git\0host=gitorious.org\0"
> part).
>
> We also do redirect stderr to /dev/null for reasons I cannot remember
> (so probably not good ones), that may be related as well. Well run
> some more tests...
Ah. I have a theory. If I do a clone of git://gitorious.org/qt/qt.git,
the counting/compressing stages take a long time (I timed it at 1m40
before it actually sends any data). And looking at upload-pack.c, we
leave the 30-second alarm set while creating the pack. Meaning we die 30
seconds into creating the pack.
When progress is being displayed, however, the progress timer actually
uses SIGALRM, as well. So we are constantly resetting the timer and it
never goes off.
And we should be able to test this theory. How long does it take for the
failure case to fail:
$ time git clone -n git://gitorious.org/qt/qt.git repo >log
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed
real 0m31.106s
user 0m0.000s
sys 0m0.012s
Hmm. Suspicious. :)
So that implies to me a few things:
1. You guys should really pack your repos, as you are wasting over a
minute of CPU time every time somebody clones this repo.
2. Upload-pack has what I consider a bug. The --timeout should be
suspended while we are actually crunching numbers to create the
pack. We probably don't want it when sending the pack, either, as
people with slow connections (or big repos) will get timed out
during the send. Probably we just want to apply it to times when we
are waiting to get the list of refs from the client.
3. Upload-pack and the progress code are both using the global alarm
timer and signal, and that is papering over the bug in (2) when
progress is enabled. I'm not sure of the simplest way of having
those interact. But maybe we can just ignore it, because we
probably don't want to using the --timeout alarm during the packing
phase, anyway.
-Peff
^ permalink raw reply
* Re: [PATCH 3/4] reset: add option "--merge-dirty" to "git reset"
From: Linus Torvalds @ 2009-09-11 16:02 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski
In-Reply-To: <200909110729.32581.chriscool@tuxfamily.org>
On Fri, 11 Sep 2009, Christian Couder wrote:
>
> No, I realize it was my writing skills that were lacking again. I will
> rework the commit message using some tables like in Daniel's message.
Ok, thanks.
> > The patch also seems to imply that it's always about HEAD. True?
>
> Yes.
It might be worth noting/explaining very explicitly. "git reset" uin
general does not ever try to make HEAD special - except as a _default_
location, of course, and as the thing that is actually modified.
I realize _why_ (HEAD is kind of the "base" for the index state), but I
think it's worth a comment or something. I did a double-take when I saw
that part of the diff exactly because it's unusual.
Linus
^ permalink raw reply
* Re: Cannot clone redirecting stdout
From: Johan Sørensen @ 2009-09-11 15:46 UTC (permalink / raw)
To: Jeff King; +Cc: Aloisio, git, support
In-Reply-To: <20090911135110.GA30860@coredump.intra.peff.net>
On Fri, Sep 11, 2009 at 3:51 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Sep 11, 2009 at 06:23:29AM -0400, Jeff King wrote:
>
>> > I faced a problem when trying to clone git://gitorious.org/qt/qt.git
>> >
>> > this works:
>> > git clone -n git://gitorious.org/qt/qt.git repo
>> >
>> > this doesn't:
>> > git clone -n git://gitorious.org/qt/qt.git repo >log
>> > fatal: The remote end hung up unexpectedly
>> > fatal: early EOF
>> > fatal: index-pack failed
>>
>> I can reproduce the problem here. But after staring at the strace for a
>> long time, I don't think the problem is on the client side. The remote
>> end _does_ hang up unexpectedly.
>>
>> Looking at what we send, the only difference between the redirected and
>> unredirected case I could find is that we send the "no-progress" flag to
>> the server, which then hangs up on us instead of sending us the pack.
>> Which makes no sense.
>
> I did a little more testing, and I can't reproduce the problem against a
> local git-daemon. I tried using several versions for the server, going
> all the way back to v1.5.0, which pre-dates no-progress, and all of them
> worked just fine.
>
> So I am inclined to think there is something non-standard or broken at
> gitorious.org. I'm cc'ing support@gitorious to see if they have any
> comment.
Some quick tests seem to indicate it's related to the fact that our
wonderful little fork+exec git-daemon[1] (which is different from the
one distributed with git) exec's to "git-upload-pack --strict
--timeout=30 /path/to/repo". Now, why exactly that'll trigger when the
no-progress flag is given I'm not sure of. The daemon itself execs as
soon as it figures out what repo the client requested, so apart from
the timeout the only thing it reacts to is the header (the
"${headersize}git-upload-pack project/repo.git\0host=gitorious.org\0"
part).
We also do redirect stderr to /dev/null for reasons I cannot remember
(so probably not good ones), that may be related as well. Well run
some more tests...
[1]: http://gitorious.org/gitorious/mainline/blobs/master/script/git-daemon
>
> -Peff
> --
> 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
* Re: [RFC/PATCH 2/2] gitweb: check given hash before trying to create snapshot
From: Mark Rada @ 2009-09-11 15:44 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Mark Rada, git, Junio C Hamano
In-Reply-To: <200909110952.50536.jnareb@gmail.com>
On 2009-09-11, at 3:52 AM, Jakub Narebski wrote:
> Second, I'd rather have better names for snapshots than using full
> SHA-1.
> For snapshot of 'v1.5.0' of repository 'repo.git' I'd prefer for
> snapshot
> to be named 'repo-v1.5.0', and for snapshot of 'next' branch of the
> same
> project to be named for example 'repo-next-20090909', or perhaps
> 'repo-next-2009-09-10T09:16:18' or 'repo-next-20090909-g5f6b0ff',
> or 'repo-v1.6.5-rc0-164-g5f6b0ff'.
Ah, yeah, well, I let $hash still hold the originally passed value,
which
would be used to create the outputted file name (unless overwritten by
the
client by using curl -o or something). Then $snapshot holds the full
hash.
This way, if you were to type something like
http://git.kernel.org/?p=git/git.git;a=snapshot;h=next;sf=tgz
into your browser window, you would get git.git-next.tar.gz back, but
the
backend could look up something like
git-5f6b0ffff13f5cd762d0a5a4e1c4dede58e8a537.tar.gz
using the $snapshot variable in some hypothetical cache (or even
without the
cache it won't mangle the nicer name $hash might have).
Also, right now gitweb will not accept tags for hashes. This seems to be
because it passes the --verify option to rev-parse, but the output
from using
and not using the verify option seems to be the same (other than also
accepting
all tree-ishes). Could you let me know if there is a good reason not
to take
off the --verify option? Otherwise, I would like to take it off in the
next
version of this patch.
Your point about adding the short hash to snapshots of branch heads is
cool,
I'll try that for the next version of the patch.
>> }
>>
>> my $name = $project;
>> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-
>> gitweb-standalone-http-status.sh
>> index d0ff21d..4f8f147 100644
>> --- a/t/t9501-gitweb-standalone-http-status.sh
>> +++ b/t/t9501-gitweb-standalone-http-status.sh
>> @@ -75,4 +75,30 @@ test_expect_success \
>> test_debug 'cat gitweb.output'
>>
>>
>> +test_expect_success \
>> + 'snapshots: bad treeish id' \
>> + 'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
>> + grep "400 - Not a valid hash id:" gitweb.output'
>> +test_debug 'cat gitweb.output'
>> +
>> +test_expect_success \
>> + 'snapshots: good treeish id' \
>> + 'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
>> + grep "Status: 200 OK" gitweb.output'
>> +test_debug 'cat gitweb.output'
>
> Why you don't check for "Status: 400" too?
I'm not sure which test you are referring to (I think the second). The
second test is valid and should return a nice .git-master.tar.gz
tarball.
>> Second, any given treeish will always be translated to the full
>> length,
>> unambiguous, hash id; this will be useful for things like creating
>> unique names for snapshot caches.
>
> But this is not a good idea, IMHO.
>
> First, it introduces feature that nobody uses (at least yet); we can
> introduce this feature when it is needed instead.
Sorry for promoting vapourware, I did originally rip this patch out from
something else. I removed the comment from the v2 commit message.
--
Mark Rada (ferrous26)
marada@uwaterloo.ca
^ permalink raw reply
* Re: bash_completion outside repo
From: Jeff King @ 2009-09-11 15:09 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: james bardin, Todd Zullinger, git
In-Reply-To: <20090911143651.GE1033@spearce.org>
On Fri, Sep 11, 2009 at 07:36:51AM -0700, Shawn O. Pearce wrote:
> > instead of just using "git config --get-regexp 'remote\..*\.url'", which
> > would be slightly more efficient, and also doesn't have this bug. ;)
>
> F'king oversight. You are right, this should be --get-regexp.
> There isn't a reason here, probably other than "I forgot about
> --get-regexp when I wrote the original code".
OK. I will leave a bash-completion patch for you (or somebody else
interested) as I don't use it myself.
Assuming you make such a patch, that will clear up the original issue. I
wonder if we should fix "git config --list". The current semantics seem
a little crazy to me, but it is a scriptable interface. I'm inclined to
call this a bug, though.
-Peff
^ permalink raw reply
* Re: [ JGIT ] incompatiblity found in DirCache
From: Shawn O. Pearce @ 2009-09-11 15:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Adam W. Hawks, git
In-Reply-To: <200909092311.07145.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> onsdag 09 september 2009 20:55:39 skrev "Adam W. Hawks" <awhawks@writeme.com>:
> > When using the DirCache interface to the index you can create a
> > invalid/corrupt tree for git 1.6.5.
> >
> > The problem seems to be you can add a path to the index that starts
> > with a "/" and DirCache creates a entry with a mode but no path.
> > This causes git 1.6.5 to fail with a corrupt tree.
>
> I think there are more ways of entering bad stuff. Preventing a
> deliberate programmatic creation of invalid trees is probably not
> the most important thing, but then again, validating the data to
> prevent e.g. the EGit plugin from doing it by mistake due to bugs
> could probably be worthwhile.
We already check for and fail fast on a 0 mode in DirCache, as this
mode is also not valid in the index, or in a git tree.
We should be doing the same thing for an empty path name. "a//b" is
not a valid path in the index, as "" is not a valid tree entry path.
For the same reason, "/a" is not a valid path in the index.
Unfortunately our API also allows you to try and create a name of
"a\u0000b", which is a valid Java string, but will create a corrupt
tree. \u0000 in a name with more than 4095 bytes will also create
a corrupt index (shorter strings are semi-valid because shorter
strings use a Pascal like string format, and longer ones use a C
like string format). Though C git is unable to access a path whose
name contains "\u0000", no matter how long the string is.
I think we should try a bit harder in DirCache to prevent these sorts
of really bad entries from being constructed by application code.
Yes, applications should not do this, but I think the library also
should not write known bogus trash to the disk and claim it is OK.
I'll try to work up a patch for this today.
--
Shawn.
^ permalink raw reply
* Re: obnoxious CLI complaints
From: Linus Torvalds @ 2009-09-11 14:47 UTC (permalink / raw)
To: René Scharfe; +Cc: Jakub Narebski, Brendan Miller, git
In-Reply-To: <4AA97B61.6030301@lsrfire.ath.cx>
On Fri, 11 Sep 2009, René Scharfe wrote:
>
> Using zlib directly avoids the overhead of a pipe and of buffering the
> output for blocked writes; surprisingly (to me), it isn't any faster.
In fact, it should be slower.
On SMP, you're quite likely better off using the pipe, and compressing on
another CPU. Of course, it's usually the case that the compression is _so_
much slower than generating the tar-file (especially for the hot-cache
case) that it doesn't matter or the pipe overhead is even a slowdown.
But especially if generating the tar-file has some delays in it
(cold-cache object lookup, whatever), the "compress in separate process"
is likely simply better, because you can compress while the other process
is looking up data for the tar.
Linus
^ permalink raw reply
* Re: bash_completion outside repo
From: Shawn O. Pearce @ 2009-09-11 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: james bardin, Todd Zullinger, git
In-Reply-To: <20090911141730.GA384@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> I also question why it is using "git config --list" at all in snippets
> like this:
>
> for i in $(git --git-dir="$d" config --list); do
> case "$i" in
> remote.*.url=*)
> i="${i#remote.}"
> echo "${i/.url=*/}"
> ;;
> esac
> done
>
> instead of just using "git config --get-regexp 'remote\..*\.url'", which
> would be slightly more efficient, and also doesn't have this bug. ;)
F'king oversight. You are right, this should be --get-regexp.
There isn't a reason here, probably other than "I forgot about
--get-regexp when I wrote the original code".
--
Shawn.
^ permalink raw reply
* Re: bash_completion outside repo
From: Jeff King @ 2009-09-11 14:17 UTC (permalink / raw)
To: james bardin; +Cc: Todd Zullinger, git
In-Reply-To: <a3b675320909110700k7eb7286qc8cb5691aae214c7@mail.gmail.com>
On Fri, Sep 11, 2009 at 10:00:33AM -0400, james bardin wrote:
> I did a make install, and dropped the completion file in
> /etc/bash_completion.d/. No other settings changed. I did a quick
> check, and it happens with the current 1.6.5 snapshot too, and on a
> fedora 10 box I found.
>
> It seems I only get this error if I don't have a global config.
> Touching ~/.gitconfig stops the error.
Ah, I see. It looks like we use "git config --list" to view several bits
of configuration. However, it is not happy if there is no config file to
list.
However, I'm not sure that "config --list" isn't broken. Inside a repo,
doing "git config --list" shows the repo config and my global config,
and exits with no error. Outside a repo, it shows my global config, and
exits with no error. But if I _don't_ have global config, it produces an
error. Shouldn't it treat that as simply "no config is available"?
I also question why it is using "git config --list" at all in snippets
like this:
for i in $(git --git-dir="$d" config --list); do
case "$i" in
remote.*.url=*)
i="${i#remote.}"
echo "${i/.url=*/}"
;;
esac
done
instead of just using "git config --get-regexp 'remote\..*\.url'", which
would be slightly more efficient, and also doesn't have this bug. ;)
-Peff
^ permalink raw reply
* Re: bash_completion outside repo
From: james bardin @ 2009-09-11 14:00 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git
In-Reply-To: <20090911133313.GF2582@inocybe.localdomain>
On Fri, Sep 11, 2009 at 9:33 AM, Todd Zullinger <tmz@pobox.com> wrote:
> james bardin wrote:
>> I'm making a git rpm for our site, and I'm getting an error with
>> bash_completion on RHEL/CentOS 5.
>
> Out of curiosity, have you tried rebuilding the Fedora packages from
> rawhide? They should work on RHEL/CentOS 5 just fine (I use them
> myself).
>
I was trying to prune out some of the dependencies to make internal
deployment easier.
I bundled the doc tarballs instead of requiring asciidoc, and git
includes its own perl(Error) if it's not available.
On Fri, Sep 11, 2009 at 9:11 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> I can't reproduce this with git version 1.6.5.rc0.164.g5f6b0 on Fedora 11.
>
> Which exact steps reproduce it for you, and which relevant settings (PS1
> and GIT_PS1_...) do you use? Do you have a system or global .gitconfig?
>
I did a make install, and dropped the completion file in
/etc/bash_completion.d/. No other settings changed. I did a quick
check, and it happens with the current 1.6.5 snapshot too, and on a
fedora 10 box I found.
It seems I only get this error if I don't have a global config.
Touching ~/.gitconfig stops the error.
^ permalink raw reply
* Re: Cannot clone redirecting stdout
From: Jeff King @ 2009-09-11 13:51 UTC (permalink / raw)
To: Aloisio; +Cc: git, support
In-Reply-To: <20090911102329.GA13044@sigill.intra.peff.net>
On Fri, Sep 11, 2009 at 06:23:29AM -0400, Jeff King wrote:
> > I faced a problem when trying to clone git://gitorious.org/qt/qt.git
> >
> > this works:
> > git clone -n git://gitorious.org/qt/qt.git repo
> >
> > this doesn't:
> > git clone -n git://gitorious.org/qt/qt.git repo >log
> > fatal: The remote end hung up unexpectedly
> > fatal: early EOF
> > fatal: index-pack failed
>
> I can reproduce the problem here. But after staring at the strace for a
> long time, I don't think the problem is on the client side. The remote
> end _does_ hang up unexpectedly.
>
> Looking at what we send, the only difference between the redirected and
> unredirected case I could find is that we send the "no-progress" flag to
> the server, which then hangs up on us instead of sending us the pack.
> Which makes no sense.
I did a little more testing, and I can't reproduce the problem against a
local git-daemon. I tried using several versions for the server, going
all the way back to v1.5.0, which pre-dates no-progress, and all of them
worked just fine.
So I am inclined to think there is something non-standard or broken at
gitorious.org. I'm cc'ing support@gitorious to see if they have any
comment.
-Peff
^ permalink raw reply
* [PATCH v2] Re: add documentation for mailinfo.scissors and '--no-scissors'
From: Nicolas Sebrecht @ 2009-09-11 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Sebrecht, Nanako Shiraishi, git
In-Reply-To: <7veiqe0x05.fsf@alter.siamese.dyndns.org>
[ Thank you for this review. ]
The 11/09/09, Junio C Hamano wrote:
> Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:
>
> > diff --git a/git-am.sh b/git-am.sh
> > index 26ffe70..f242d1a 100755
> > --- a/git-am.sh
> > +++ b/git-am.sh
> > @@ -16,6 +16,7 @@ s,signoff add a Signed-off-by line to the commit message
> > u,utf8 recode into utf8 (default)
> > k,keep pass -k flag to git-mailinfo
> > c,scissors strip everything before a scissors line
> > +no-scissors don't obey to a scissors line (default)
> > whitespace= pass it through git-apply
> > ignore-space-change pass it through git-apply
> > ignore-whitespace pass it through git-apply
>
> Do we want it to allow --no-no-scissors? I do not think this hunk is
> necessary at all.
I don't understand your point here. This hunk doesn't allow
--no-no-scissors but add --no-scissors to usage of 'git am', no?
Do I miss something around $OPTIONS_SPEC?
--
Nicolas Sebrecht
^ permalink raw reply
* Re: bash_completion outside repo
From: Todd Zullinger @ 2009-09-11 13:33 UTC (permalink / raw)
To: james bardin; +Cc: git
In-Reply-To: <a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 770 bytes --]
james bardin wrote:
> I'm making a git rpm for our site, and I'm getting an error with
> bash_completion on RHEL/CentOS 5.
Out of curiosity, have you tried rebuilding the Fedora packages from
rawhide? They should work on RHEL/CentOS 5 just fine (I use them
myself).
> When outside a repo, any completion causes git to spit out
> fatal: error processing config file(s)
>
> This is 1.6.4.2 using the contrib bash completion file
I can't reproduce this on either Fedora or CentOS.
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A great many people think they are thinking when they are merely
rearranging their prejudices.
-- William James
[-- Attachment #2: Type: application/pgp-signature, Size: 542 bytes --]
^ permalink raw reply
* Re: bash_completion outside repo
From: Michael J Gruber @ 2009-09-11 13:11 UTC (permalink / raw)
To: james bardin; +Cc: git
In-Reply-To: <a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com>
james bardin venit, vidit, dixit 10.09.2009 17:13:
> Hi,
>
> I'm making a git rpm for our site, and I'm getting an error with
> bash_completion on RHEL/CentOS 5.
>
> When outside a repo, any completion causes git to spit out
> fatal: error processing config file(s)
>
> This is 1.6.4.2 using the contrib bash completion file
>
> Thanks
> -jim
I can't reproduce this with git version 1.6.5.rc0.164.g5f6b0 on Fedora 11.
Which exact steps reproduce it for you, and which relevant settings (PS1
and GIT_PS1_...) do you use? Do you have a system or global .gitconfig?
Michael
^ 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