git add -A fails in empty repository since 1.8.5
This was discussed on the Git user list recently [1]. #in a repo with no files git add -A fatal: pathspec '.' did not match any files The same goes for git add . (and -u). Whereas I think some warning feedback is useful, we are curious whether this is an intentional change or not. [1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349). On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen tfn...@gmail.com wrote: This was discussed on the Git user list recently [1]. #in a repo with no files git add -A fatal: pathspec '.' did not match any files The same goes for git add . (and -u). Whereas I think some warning feedback is useful, we are curious whether this is an intentional change or not. [1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@gmail.com wrote:
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349).
On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
This was discussed on the Git user list recently [1].
#in a repo with no files
git add -A
fatal: pathspec '.' did not match any files
The same goes for git add . (and -u).
Whereas I think some warning feedback is useful, we are curious
whether this is an intentional change or not.
I was not aware of this case when I made the change. It's caused by
this change that removes pathspec.raw[i][0] check in builtin/add.c in
84b8b5d .
- for (i = 0; pathspec.raw[i]; i++) {
- if (!seen[i] pathspec.raw[i][0]
-!file_exists(pathspec.raw[i])) {
+ for (i = 0; i pathspec.nr; i++) {
+ const char *path = pathspec.items[i].match;
+ if (!seen[i] !file_exists(path)) {
Adding it back requires some thinking because path in the new code
could be something magic.. and the new behavior makes sense, so I'm
inclined to keep it as is, unless people have other opinions.
[1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats
Am 11.12.2013 08:46, schrieb Christian Couder:
+enum repl_fmt { SHORT, MEDIUM, FULL };
SHORT is predefined on Windows, could you choose another name?
MinGW:
builtin/replace.c:23: error: 'SHORT' redeclared as different kind of symbol
c:\git\msysgit\mingw\bin\../lib/gcc/mingw32/4.4.0/../../../../include/winnt.h:78:
note: previous declaration of 'SHORT'
was here
make: *** [builtin/replace.o] Error 1
MSVC:
CC builtin/replace.o
replace.c
builtin/replace.c(23) : error C2365: SHORT: Erneute Definition; vorherige
Definition war Typedef.
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332):
Siehe Deklaration von 'SHORT'
builtin/replace.c(23) : error C2086: 'repl_fmt SHORT': Neudefinition
builtin/replace.c(23): Siehe Deklaration von 'SHORT'
builtin/replace.c(36) : error C2275: 'SHORT': Ungültige Verwendung dieses Typs
als Ausdruck
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332):
Siehe Deklaration von 'SHORT'
builtin/replace.c(67) : error C2275: 'SHORT': Ungültige Verwendung dieses Typs
als Ausdruck
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332):
Siehe Deklaration von 'SHORT'
builtin/replace.c(173) : warning C4090: 'Initialisierung': Unterschiedliche
'const'-Bezeichner
make: *** [builtin/replace.o] Error 1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 10.12.2013 00:45, schrieb Jonathan Nieder:
Karsten Blees wrote:
Googling some more, I believe the most protable way to achieve this
via 'compiler settings' is
#pragma pack(push)
#pragma pack(4)
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
#pragma pack(pop)
This is supported by at least GCC, MSVC and HP (see your link). The
downside is that we cannot use macros (in git-compat-util.h) to emit
#pragmas. But we wouldn't have to, as compilers aren't supposed to
barf on unknown #pragmas.
Technically this can be done using macros:
#if (gcc)
#define BEGIN_PACKED _Pragma(pack(push,4))
#define END_PACKED _Pragma(pack(pop))
#elif (msvc)
#define BEGIN_PACKED __pragma(pack(push,4))
#define END_PACKED __pragma(pack(pop))
#else
/* Just tolerate a little padding. */
#define BEGIN_PACKED
#define END_PACKED
#end
Then you can do:
BEGIN_PACKED
struct hashmap_entry {
...
};
END_PACKED
Sorry for the delay...
My intention with #pragma pack was to support as many compilers / platforms as
possible (even though I can't test them all). From what I could find on the
'net, support for the _Pragma operator is slim. And as the MSVC build is 32-bit
only (AFAIK), this is pretty much a GCC-only solution (and thus equivalent, but
much more verbose than the initial __attribute__((__packed__)) variant).
Whether that's nicer or uglier than the alternatives I leave to you.
;-)
If it was really up to me ( :-) ), I'd like to do this:
-- 8 ---
Subject: [PATCH] hashmap.h: make sure map entries are tightly packed
Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but struct hashmap_entry needs to be packed for this to work. E.g.
struct name_entry {
struct hashmap_entry ent; /* consists of pointer + int */
int namelen;
char *name;
};
should be 24 instead of 32 bytes.
Packing structures is compiler-specific, most compilers support [1]:
#pragma pack(n) - to set structure packing
#pragma pack() - to reset structure packing to default
Compilers are supposed to ignore unknown #pragmas, so using this without
further #if compiler guards should be safe.
Wasting a few bytes for padding is acceptable, however, compiling the rest
of git with packed structures (and thus mis-aligned arrays of structs) is
not. Add a test to ensure that '#pragma pack()' really resets the packing.
[1] Compiler docs regarding #pragma pack:
http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Structure_002dPacking-Pragmas.html#Structure_002dPacking-Pragmas
http://msdn.microsoft.com/en-us/library/2e70t5y1%28v=vs.80%29.aspx
http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#pragma-PACK
http://clang.llvm.org/docs/UsersManual.html#microsoft-extensions
http://uw714doc.sco.com/en/SDK_cprog/_Preprocessing_Directives.html
http://osr507doc.sco.com/en/tools/ANSI_F.3.13_Preprocessing.html
http://docs.oracle.com/cd/E19422-01/819-3690/Pragmas_App.html#73499
http://publib.boulder.ibm.com/infocenter/comphelp/v7v91/index.jsp?topic=%2Fcom.ibm.vacpp7a.doc%2Fcompiler%2Fref%2Frnpgpack.htm
Supposedly wants #pragma pack(0) to reset:
http://techpubs.sgi.com/library/tpl/cgi-bin/getdoc.cgi/0650/bks/SGI_Developer/books/Pragmas/sgi_html/ch04.html
Not supported:
http://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-DD32852C-A0F9-4AC6-BF67-D10D064CC87A.htm
Signed-off-by: Karsten Blees bl...@dcon.de
---
hashmap.h | 7 +++
t/t0011-hashmap.sh | 6 ++
test-hashmap.c | 17 +
3 files changed, 30 insertions(+)
diff --git a/hashmap.h b/hashmap.h
index a816ad4..93b330b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,10 +15,17 @@ extern unsigned int memihash(const void *buf, size_t len);
/* data structures */
+/*
+ * Set struct packing to 4 so that we don't waste memory on 64-bit systems.
+ * Struct hashmap_entry is used as prefix to other (un-packed) structures,
+ * so this won't cause alignment issues e.g. for odd elements in an array.
+ */
+#pragma pack(4)
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
+#pragma pack()
typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
const void *keydata);
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 391e2b6..3f3c90b 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -237,4 +237,10 @@ test_expect_success 'grow / shrink' '
'
+test_expect_success '#pragma pack() resets packing to default' '
+
+ test_hashmap pragma-pack ok
+
+'
+
test_done
diff --git a/test-hashmap.c b/test-hashmap.c
index f5183fb..2d5a63a 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -36,6 +36,12 @@ static struct test_entry *alloc_test_entry(int hash, char
*key, int klen,
return entry;
}
+struct
Re: [PATCH v5 02/14] add a hashtable implementation that supports O(1) removal
Am 14.12.2013 03:04, schrieb Jonathan Nieder: Hi, Karsten Blees wrote: test-hashmap.c | 340 Here come two small tweaks on top (meant for squashing in or applying to the series, whichever is more convenient). Thanks, Jonathan Nieder (2): Add test-hashmap to .gitignore Drop unnecessary #includes from test-hashmap .gitignore | 1 + test-hashmap.c | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) Thanks, these two make perfect sense. I'm too damn slow again (merged to next two days ago), but FWIW: Acked-by: Karsten Blees bl...@dcon.de -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hashmap.h: Use 'unsigned int' for hash-codes everywhere
Signed-off-by: Karsten Blees bl...@dcon.de
---
Am 09.12.2013 18:48, schrieb Junio C Hamano:
Karsten Blees karsten.bl...@gmail.com writes:
* kb/fast-hashmap (2013-11-18) 14 commits
(merged to 'next' on 2013-12-06 at f90be3d)
Damn, a day too late :-) I found these two glitches today...is a
fixup patch OK or should I do a reroll (or separate patch on top)?
A separate patch on top would be the most appropriate.
OK, this one's a no-brainer I think. See $gmane/239430 for the latest proposal
on the struct packing front.
Documentation/technical/api-hashmap.txt | 2 +-
hashmap.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-hashmap.txt
b/Documentation/technical/api-hashmap.txt
index b2280f1..42ca234 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -80,7 +80,7 @@ prevent expensive resizing. If 0, the table is dynamically
resized.
If `free_entries` is true, each hashmap_entry in the map is freed as well
(using stdlib's free()).
-`void hashmap_entry_init(void *entry, int hash)`::
+`void hashmap_entry_init(void *entry, unsigned int hash)`::
Initializes a hashmap_entry structure.
+
diff --git a/hashmap.h b/hashmap.h
index f5b3b61..a816ad4 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int
free_entries);
/* hashmap_entry functions */
-static inline void hashmap_entry_init(void *entry, int hash)
+static inline void hashmap_entry_init(void *entry, unsigned int hash)
{
struct hashmap_entry *e = entry;
e-hash = hash;
--
1.8.5.1.276.g562b27a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 18.12.2013 14:10, schrieb Karsten Blees: + printf(sizeof(pointer+int) (%u) is not a +multiple of sizeof(pointer) (%u)!\n, +sizeof(struct pointer_int), +sizeof(void *)); Should be: - sizeof(struct pointer_int), - sizeof(void *)); + (unsigned) sizeof(struct pointer_int), + (unsigned) sizeof(void *)); (sizeof() is 'unsigned int' on 32-bit and 'long unsigned int' on 64-bit; without the cast, %u produces warnings on 64-bit and %lu on 32-bit...) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
subscribe git -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 09.12.2013 15:03, schrieb Karsten Blees:
3.) Inject individual fields via macro
Instead of injecting a struct hashmap_entry, which implies alignment to
sizeof(struct hashmap_entry), we could inject the individual fields, e.g.
#define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int
__hash;
struct name_entry {
HASHMAP_ENTRY_HEADER
int namelen;
char *name;
};
I've tried this as well. However, the change is much more intrusive, and
produces lots of strict-aliasing warnings in GCC 4.4, probably due to this bug
[1]. So I don't think its a good solution. For anyone interested, the patch can
be found at [2].
[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42032
[2] https://github.com/kblees/git/commits/kb/hashmap-v5-fixes-macro
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/12] connect.c: replace some use of starts_with() with skip_prefix()
name will be reset unconditionally soon after skip_prefix() returns
NULL.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
connect.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index c763eed..1bb70aa 100644
--- a/connect.c
+++ b/connect.c
@@ -131,7 +131,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t
src_len,
for (;;) {
struct ref *ref;
unsigned char old_sha1[20];
- char *name;
+ const char *name;
int len, name_len;
char *buffer = packet_buffer;
@@ -145,8 +145,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t
src_len,
if (!len)
break;
- if (len 4 starts_with(buffer, ERR ))
- die(remote error: %s, buffer + 4);
+ if ((name = skip_prefix(buffer, ERR )) != NULL)
+ die(remote error: %s, name);
if (len 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] !=
' ')
die(protocol error: expected sha/ref, got '%s',
buffer);
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/12] Make starts_with() a wrapper of skip_prefix()
starts_with() started out as a copy of prefixcmp(). But if we don't
care about the sorting order, the logic looks closer to
skip_prefix(). This looks like a good thing to do.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
git-compat-util.h | 6 +-
strbuf.c | 9 -
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index b73916b..84f1078 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -350,7 +350,6 @@ extern void set_die_routine(NORETURN_PTR void
(*routine)(const char *err, va_lis
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
extern void set_die_is_recursing_routine(int (*routine)(void));
-extern int starts_with(const char *str, const char *prefix);
extern int prefixcmp(const char *str, const char *prefix);
extern int ends_with(const char *str, const char *suffix);
extern int suffixcmp(const char *str, const char *suffix);
@@ -361,6 +360,11 @@ static inline const char *skip_prefix(const char *str,
const char *prefix)
return strncmp(str, prefix, len) ? NULL : str + len;
}
+static inline int starts_with(const char *str, const char *prefix)
+{
+ return skip_prefix(str, prefix) != NULL;
+}
+
#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
#ifndef PROT_READ
diff --git a/strbuf.c b/strbuf.c
index 83caf4a..bd4c0d8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,15 +1,6 @@
#include cache.h
#include refs.h
-int starts_with(const char *str, const char *prefix)
-{
- for (; ; str++, prefix++)
- if (!*prefix)
- return 1;
- else if (*str != *prefix)
- return 0;
-}
-
int prefixcmp(const char *str, const char *prefix)
{
for (; ; str++, prefix++)
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/12] Convert a lot of starts_with() to skip_prefix()
The purpose is remove hard coded string length. Some could be a few
lines away from the string comparison and easy to be missed when the
string is changed.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/for-each-ref.c | 9 +
builtin/mailinfo.c | 6 +++---
builtin/merge.c| 8 +---
builtin/remote.c | 3 +--
commit.c | 5 +
diff.c | 9 +++--
fetch-pack.c | 9 +
http-backend.c | 5 +++--
http-push.c| 6 +++---
http.c | 5 +++--
log-tree.c | 5 +++--
pager.c| 2 +-
pathspec.c | 5 +++--
refs.c | 12 +++-
sha1_name.c| 12 +++-
transport-helper.c | 15 +++
transport.c| 14 --
17 files changed, 64 insertions(+), 66 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6551e7b..25c1388 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -662,6 +662,7 @@ static void populate_value(struct refinfo *ref)
const char *refname;
const char *formatp;
struct branch *branch = NULL;
+ const char *next;
if (*name == '*') {
deref = 1;
@@ -674,18 +675,18 @@ static void populate_value(struct refinfo *ref)
refname = ref-symref ? ref-symref : ;
else if (starts_with(name, upstream)) {
/* only local branches may have an upstream */
- if (!starts_with(ref-refname, refs/heads/))
+ if ((next = skip_prefix(ref-refname, refs/heads/))
== NULL)
continue;
- branch = branch_get(ref-refname + 11);
+ branch = branch_get(next);
if (!branch || !branch-merge || !branch-merge[0] ||
!branch-merge[0]-dst)
continue;
refname = branch-merge[0]-dst;
- } else if (starts_with(name, color:)) {
+ } else if ((next = skip_prefix(name, color:)) != NULL) {
char color[COLOR_MAXLEN] = ;
- color_parse(name + 6, --format, color);
+ color_parse(next, --format, color);
v-s = xstrdup(color);
continue;
} else if (!strcmp(name, flag)) {
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2100e23..daaafbd 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -328,13 +328,13 @@ static int check_header(const struct strbuf *line,
}
/* for inbody stuff */
- if (starts_with(line-buf, From) isspace(line-buf[5])) {
+ if (isspace(*skip_prefix_defval(line-buf, From, NOSPACE))) {
ret = 1; /* Should this return 0? */
goto check_header_out;
}
- if (starts_with(line-buf, [PATCH]) isspace(line-buf[7])) {
+ if (isspace(*skip_prefix_defval(line-buf, [PATCH], NOSPACE))) {
for (i = 0; header[i]; i++) {
- if (!memcmp(Subject, header[i], 7)) {
+ if (starts_with(header[i], Subject)) {
handle_header(hdr_data[i], line);
ret = 1;
goto check_header_out;
diff --git a/builtin/merge.c b/builtin/merge.c
index 590d907..603f80a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -569,10 +569,12 @@ static void parse_branch_merge_options(char *bmo)
static int git_merge_config(const char *k, const char *v, void *cb)
{
int status;
+ const char *kk, *kkk;
- if (branch starts_with(k, branch.)
- starts_with(k + 7, branch)
- !strcmp(k + 7 + strlen(branch), .mergeoptions)) {
+ if (branch
+ (kk = skip_prefix(k, branch.)) != NULL
+ (kkk = skip_prefix(kk, branch)) != NULL
+ !strcmp(kkk, .mergeoptions)) {
free(branch_mergeoptions);
branch_mergeoptions = xstrdup(v);
return 0;
diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..218c8c8 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -259,14 +259,13 @@ static const char *abbrev_ref(const char *name, const
char *prefix)
static int config_read_branches(const char *key, const char *value, void *cb)
{
- if (starts_with(key, branch.)) {
+ if ((key = skip_prefix(key, branch.)) != NULL) {
const char *orig_key = key;
char *name;
struct string_list_item *item;
struct branch_info *info;
enum { REMOTE, MERGE, REBASE } type;
- key += 7;
if (ends_with(key, .remote)) {
[PATCH 03/12] Add and use skip_prefix_defval()
This is a variant of skip_prefix() that returns a specied pointer
instead of NULL if no prefix is found. It's helpful to simplify
if (starts_with(foo, bar))
foo += 3;
into
foo = skip_prefix_gently(foo, bar, foo);
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/checkout.c| 6 ++
builtin/fast-export.c | 3 +--
builtin/merge.c | 4 ++--
builtin/show-branch.c | 14 +-
git-compat-util.h | 9 +++--
git.c | 3 +--
notes.c | 6 ++
wt-status.c | 12
8 files changed, 24 insertions(+), 33 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..6531ed4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1151,10 +1151,8 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
const char *argv0 = argv[0];
if (!argc || !strcmp(argv0, --))
die (_(--track needs a branch name));
- if (starts_with(argv0, refs/))
- argv0 += 5;
- if (starts_with(argv0, remotes/))
- argv0 += 8;
+ argv0 = skip_prefix_defval(argv0, refs/, argv0);
+ argv0 = skip_prefix_defval(argv0, remotes/, argv0);
argv0 = strchr(argv0, '/');
if (!argv0 || !argv0[1])
die (_(Missing branch name; try -b));
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index b8d8a3a..cd0a302 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -476,8 +476,7 @@ static void handle_tag(const char *name, struct tag *tag)
}
}
- if (starts_with(name, refs/tags/))
- name += 10;
+ name = skip_prefix_defval(name, refs/tags/, name);
printf(tag %s\nfrom :%d\n%.*s%sdata %d\n%.*s\n,
name, tagged_mark,
(int)(tagger_end - tagger), tagger,
diff --git a/builtin/merge.c b/builtin/merge.c
index 4941a6c..590d907 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1106,8 +1106,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
* current branch.
*/
branch = branch_to_free = resolve_refdup(HEAD, head_sha1, 0, flag);
- if (branch starts_with(branch, refs/heads/))
- branch += 11;
+ if (branch)
+ branch = skip_prefix_defval(branch, refs/heads/, branch);
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index d9217ce..6078132 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -284,8 +284,7 @@ static void show_one_commit(struct commit *commit, int
no_name)
pp_commit_easy(CMIT_FMT_ONELINE, commit, pretty);
pretty_str = pretty.buf;
}
- if (starts_with(pretty_str, [PATCH] ))
- pretty_str += 8;
+ pretty_str = skip_prefix_defval(pretty_str, [PATCH] , pretty_str);
if (!no_name) {
if (name name-head_name) {
@@ -473,14 +472,13 @@ static void snarf_refs(int head, int remotes)
}
}
-static int rev_is_head(char *head, int headlen, char *name,
+static int rev_is_head(const char *head, int headlen, const char *name,
unsigned char *head_sha1, unsigned char *sha1)
{
if ((!head[0]) ||
(head_sha1 sha1 hashcmp(head_sha1, sha1)))
return 0;
- if (starts_with(head, refs/heads/))
- head += 11;
+ head = skip_prefix_defval(head, refs/heads/, head);
if (starts_with(name, refs/heads/))
name += 11;
else if (starts_with(name, heads/))
@@ -811,10 +809,8 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
head_sha1, NULL))
has_head++;
}
- if (!has_head) {
- int offset = starts_with(head, refs/heads/) ? 11 : 0;
- append_one_rev(head + offset);
- }
+ if (!has_head)
+ append_one_rev(skip_prefix_defval(head, refs/heads/,
head));
}
if (!ref_name_cnt) {
diff --git a/git-compat-util.h b/git-compat-util.h
index 84f1078..b72a80d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -354,10 +354,15 @@ extern int prefixcmp(const char *str, const char *prefix);
extern int ends_with(const char *str, const char *suffix);
extern int suffixcmp(const char *str, const char *suffix);
-static inline const char *skip_prefix(const char *str, const char *prefix)
+static inline const char *skip_prefix_defval(const char *str, const char
*prefix, const char *defval)
{
size_t len = strlen(prefix);
- return strncmp(str, prefix, len) ? NULL : str + len;
+ return strncmp(str, prefix, len) ?
[PATCH 11/12] diff.c: convert diff_scoreopt_parse to use skip_prefix()
While at there, partly fix the reporting as well. The reported value
with arg+2 is only correct with -C/-B/-M, not with long option names.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
diff.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index 4da77fd..d629cc5 100644
--- a/diff.c
+++ b/diff.c
@@ -3367,7 +3367,7 @@ static int opt_arg(const char *arg, int arg_short, const
char *arg_long, int *va
return 1;
}
-static int diff_scoreopt_parse(const char *opt);
+static int diff_scoreopt_parse(const char **opt);
static inline int short_opt(char opt, const char **argv,
const char **optarg)
@@ -3627,13 +3627,13 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
/* renames options */
else if (starts_with(arg, -B) || starts_with(arg,
--break-rewrites=) ||
!strcmp(arg, --break-rewrites)) {
- if ((options-break_opt = diff_scoreopt_parse(arg)) == -1)
- return error(invalid argument to -B: %s, arg+2);
+ if ((options-break_opt = diff_scoreopt_parse(arg)) == -1)
+ return error(invalid argument to -B: %s, arg);
}
else if (starts_with(arg, -M) || starts_with(arg, --find-renames=)
||
!strcmp(arg, --find-renames)) {
- if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
- return error(invalid argument to -M: %s, arg+2);
+ if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
+ return error(invalid argument to -M: %s, arg);
options-detect_rename = DIFF_DETECT_RENAME;
}
else if (!strcmp(arg, -D) || !strcmp(arg, --irreversible-delete)) {
@@ -3643,8 +3643,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
!strcmp(arg, --find-copies)) {
if (options-detect_rename == DIFF_DETECT_COPY)
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
- if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
- return error(invalid argument to -C: %s, arg+2);
+ if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
+ return error(invalid argument to -C: %s, arg);
options-detect_rename = DIFF_DETECT_COPY;
}
else if (!strcmp(arg, --no-renames))
@@ -3879,29 +3879,29 @@ int parse_rename_score(const char **cp_p)
return (int)((num = scale) ? MAX_SCORE : (MAX_SCORE * num / scale));
}
-static int diff_scoreopt_parse(const char *opt)
+static int diff_scoreopt_parse(const char **opt_p)
{
+ const char *opt = *opt_p;
int opt1, opt2, cmd;
if (*opt++ != '-')
return -1;
cmd = *opt++;
+ *opt_p = opt;
if (cmd == '-') {
/* convert the long-form arguments into short-form versions */
- if (starts_with(opt, break-rewrites)) {
- opt += strlen(break-rewrites);
+ if ((opt = skip_prefix_defval(opt, break-rewrites, *opt_p))
!= *opt_p) {
if (*opt == 0 || *opt++ == '=')
cmd = 'B';
- } else if (starts_with(opt, find-copies)) {
- opt += strlen(find-copies);
+ } else if ((opt = skip_prefix_defval(opt, find-copies,
*opt_p)) != *opt_p) {
if (*opt == 0 || *opt++ == '=')
cmd = 'C';
- } else if (starts_with(opt, find-renames)) {
- opt += strlen(find-renames);
+ } else if ((opt = skip_prefix_defval(opt, find-renames,
*opt_p)) != *opt_p) {
if (*opt == 0 || *opt++ == '=')
cmd = 'M';
}
}
+ *opt_p = opt;
if (cmd != 'M' cmd != 'C' cmd != 'B')
return -1; /* that is not a -M, -C nor -B option */
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/12] refs.c: use skip_prefix() in prune_ref()
This removes the magic number 5, which is the string length of
refs/. This comes from get_loose_refs(), called in packed_refs().
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
refs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 1fb658f..217093f 100644
--- a/refs.c
+++ b/refs.c
@@ -2318,7 +2318,8 @@ static void try_remove_empty_parents(char *name)
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
- struct ref_lock *lock = lock_ref_sha1(r-name + 5, r-sha1);
+ const char *name = skip_prefix_defval(r-name, refs/, r-name);
+ struct ref_lock *lock = lock_ref_sha1(name, r-sha1);
if (lock) {
unlink_or_warn(git_path(%s, r-name));
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/12] diff.c: reduce code duplication in --stat-xxx parsing
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
diff.c | 67 +-
1 file changed, 25 insertions(+), 42 deletions(-)
diff --git a/diff.c b/diff.c
index d754e2f..4da77fd 100644
--- a/diff.c
+++ b/diff.c
@@ -3405,6 +3405,23 @@ int parse_long_opt(const char *opt, const char **argv,
return 2;
}
+static int parse_statopt(const char *arg, const char *next,
+const char *opt, int *value, char **end)
+{
+ const char *p = skip_prefix(arg, opt);
+ if (!p)
+ return 0;
+ if (*p == '=')
+ *value = strtoul(p + 1, end, 10);
+ else if (!*p !next)
+ die(Option '--stat%s' requires a value, opt);
+ else if (!*p) {
+ *value = strtoul(next, end, 10);
+ return 2;
+ }
+ return 1;
+}
+
static int stat_opt(struct diff_options *options, const char **av)
{
const char *arg = av[0];
@@ -3417,50 +3434,16 @@ static int stat_opt(struct diff_options *options, const
char **av)
arg += strlen(--stat);
end = (char *)arg;
-
switch (*arg) {
case '-':
- if (starts_with(arg, -width)) {
- arg += strlen(-width);
- if (*arg == '=')
- width = strtoul(arg + 1, end, 10);
- else if (!*arg !av[1])
- die(Option '--stat-width' requires a value);
- else if (!*arg) {
- width = strtoul(av[1], end, 10);
- argcount = 2;
- }
- } else if (starts_with(arg, -name-width)) {
- arg += strlen(-name-width);
- if (*arg == '=')
- name_width = strtoul(arg + 1, end, 10);
- else if (!*arg !av[1])
- die(Option '--stat-name-width' requires a
value);
- else if (!*arg) {
- name_width = strtoul(av[1], end, 10);
- argcount = 2;
- }
- } else if (starts_with(arg, -graph-width)) {
- arg += strlen(-graph-width);
- if (*arg == '=')
- graph_width = strtoul(arg + 1, end, 10);
- else if (!*arg !av[1])
- die(Option '--stat-graph-width' requires a
value);
- else if (!*arg) {
- graph_width = strtoul(av[1], end, 10);
- argcount = 2;
- }
- } else if (starts_with(arg, -count)) {
- arg += strlen(-count);
- if (*arg == '=')
- count = strtoul(arg + 1, end, 10);
- else if (!*arg !av[1])
- die(Option '--stat-count' requires a value);
- else if (!*arg) {
- count = strtoul(av[1], end, 10);
- argcount = 2;
- }
- }
+ if ((argcount = parse_statopt(arg, av[1], -width, width,
end)) != 0 ||
+ (argcount = parse_statopt(arg, av[1], -name-width,
name_width, end)) != 0 ||
+ (argcount = parse_statopt(arg, av[1], -graph-width,
graph_width, end)) != 0 ||
+ (argcount = parse_statopt(arg, av[1], -count, count,
end)) != 0)
+ /* nothing else, it's the OR chain that's important */
+ ;
+ else
+ argcount = 1;
break;
case '=':
width = strtoul(arg+1, end, 10);
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/12] environment.c: replace starts_with() in strip_namespace() with skip_prefix()
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
environment.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/environment.c b/environment.c
index 3c76905..bc2d916 100644
--- a/environment.c
+++ b/environment.c
@@ -171,9 +171,7 @@ const char *get_git_namespace(void)
const char *strip_namespace(const char *namespaced_ref)
{
- if (!starts_with(namespaced_ref, get_git_namespace()))
- return NULL;
- return namespaced_ref + namespace_len;
+ return skip_prefix(namespaced_ref, get_git_namespace());
}
static int git_work_tree_initialized;
--
1.8.5.1.208.g019362e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
On Wed, Dec 18, 2013 at 1:21 AM, Eric S. Raymond e...@thyrsus.com wrote: Jakub Narębski jna...@gmail.com: No, cvs-fast-export does not have --export-marks. It doesn't generate the SHA1s that would require. Even if it did, it's not clear how that would help. I was thinking about how the following part of git-fast-export `--import-marks=file` Any commits that have already been marked will not be exported again. If the backend uses a similar --import-marks file, this allows for incremental bidirectional exporting of the repository by keeping the marks the same across runs. I understand that. But it's not relevant - cvs-fast-import doesn't know about git SHA1s, and cannot. It is a bit strange that markfile has explicitly SHA-1 (:markid SHA-1), instead of generic reference to commit, in the case of CVS it would be commitid (what to do for older repositories, though?), in case of Bazaar its revision id (GUID), etc. Can we assume that SCM v1 fast-export and SCM v2 fast-import markfile uses compatibile commit names in markfile? How cvs-fast-export know where to start exporting from in incremental mode? You give it a cutoff date. This is the same way cvsps-2.x and 3.x worked, and it's what the cvsimport wrapper expects to pass down. Nice to know. I think it would be possible for remote-helper for cvs-fast-export to find this cutoff date automatically (perhaps with some safety margin), for fetching (incremental import). BTW. does cvs-fast-export support incremental *output*, or does it perform also incremental *work*? As I tried to explain previously in my response to John Herland, it's incremental output only. There is *no* CVS exporter known to me, or him, that supports incremental work. That would be at best be impractically difficult; given CVS's limitations it may be actually impossible. I wouldn't bet against impossible. Even with saving (or re-calculating from git import) guesses about CVS history made so far? Anyway I hope that incremental CVS import would be needed less and less as CVS is replaced by any more modern version control system. Anyway, that might mean that generic fast-import stream based incremental (i.e. supporting proper thin fetch) remote helper is out of question, perhaps writing one for cvs / cvs-fe would bring incremental import from CVS to git? Sorry, I don't understand that. I was thinking about creating remote-helper for cvs-fast-export, so that git can use local CVS repository as remote, using e.g. cvsroot::path as repo URL, and using this mechanism for incremental import (aka fetch). (Or even cvssync::URL for automatic cvssync + cvs-fast-export). But from what I understand this is not as easy as it seems, even with remote-helper API having support for fast-import stream. -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] gitk: Replace next and prev buttons with down and up arrows.
Users often find that next and prev do the opposite of what they
expect. For example, next moves to the next match down the list, but
that is almost always backwards in time. Replacing the text with arrows
makes it clear where the buttons will take the user.
Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
Finally got around to drawing some up and down arrows.
M.
gitk | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index 33c3a6c..abd2ef3 100755
--- a/gitk
+++ b/gitk
@@ -2263,9 +2263,35 @@ proc makewindow {} {
# build up the bottom bar of upper window
${NS}::label .tf.lbar.flabel -text [mc Find]
-${NS}::button .tf.lbar.fnext -text [mc next] -command {dofind 1 1}
-${NS}::button .tf.lbar.fprev -text [mc prev] -command {dofind -1 1}
+
+set bm_down_data {
+ #define down_width 16
+ #define down_height 16
+ static unsigned char down_bits[] = {
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x87, 0xe1, 0x8e, 0x71, 0x9c, 0x39, 0xb8, 0x1d,
+ 0xf0, 0x0f, 0xe0, 0x07, 0xc0, 0x03, 0x80, 0x01};
+}
+image create bitmap bm-down -data $bm_down_data -foreground $uifgcolor
+${NS}::button .tf.lbar.fnext -width 26 -command {dofind 1 1}
+.tf.lbar.fnext configure -image bm-down
+
+set bm_up_data {
+ #define up_width 16
+ #define up_height 16
+ static unsigned char up_bits[] = {
+ 0x80, 0x01, 0xc0, 0x03, 0xe0, 0x07, 0xf0, 0x0f,
+ 0xb8, 0x1d, 0x9c, 0x39, 0x8e, 0x71, 0x87, 0xe1,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01};
+}
+image create bitmap bm-up -data $bm_up_data -foreground $uifgcolor
+${NS}::button .tf.lbar.fprev -width 26 -command {dofind -1 1}
+.tf.lbar.fprev configure -image bm-up
+
${NS}::label .tf.lbar.flab2 -text [mc commit]
+
pack .tf.lbar.flabel .tf.lbar.fnext .tf.lbar.fprev .tf.lbar.flab2 \
-side left -fill y
set gdttype [mc containing:]
--
1.8.5.2.2.g49768e2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
incremental fast-import and marks (Re: I have end-of-lifed cvsps)
Jakub Narebski wrote: It is a bit strange that markfile has explicitly SHA-1 (:markid SHA-1), instead of generic reference to commit, in the case of CVS it would be commitid (what to do for older repositories, though?), in case of Bazaar its revision id (GUID), etc. Usually importers use at least two separate files to save state, one mapping between git object names and mark numbers, and the other mapping between native revision identifiers and mark numbers. That way, when the importer uses marks to refer to previously imported commits or blobs, fast-import knows what commits or blobs it is talking about. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
Jakub Narębski jna...@gmail.com: It is a bit strange that markfile has explicitly SHA-1 (:markid SHA-1), instead of generic reference to commit, in the case of CVS it would be commitid (what to do for older repositories, though?), in case of Bazaar its revision id (GUID), etc. Can we assume that SCM v1 fast-export and SCM v2 fast-import markfile uses compatibile commit names in markfile? For use in reposurgeon I have defined a generic cross-VCS reference to commit I call an action stamp; it consists of an RFC3339 date followed by a committer email address. Here's an example: 2013-02-06T09:35:10Z!e...@thyrsus.com In any VCS with changesets (git, Subversion, bzr, Mercurial) this almost always suffices to uniquely identify a commit. The almost is because in these systems it is possible for a user to do multiple commits in the same second. And now you know why I wish git had subsecond timestamp resolution! If it did, uniqueness of these in a git stream could be guaranteed. The implied model completely breaks for CVS, of course. There you have to use commitids and plain give up when those don't exist. I think it would be possible for remote-helper for cvs-fast-export to find this cutoff date automatically (perhaps with some safety margin), for fetching (incremental import). Yes. As I tried to explain previously in my response to John Herland, it's incremental output only. There is *no* CVS exporter known to me, or him, that supports incremental work. That would be at best be impractically difficult; given CVS's limitations it may be actually impossible. I wouldn't bet against impossible. Even with saving (or re-calculating from git import) guesses about CVS history made so far? Even with that. cvsps-2.x tried to do something like this. It was a lose. Anyway I hope that incremental CVS import would be needed less and less as CVS is replaced by any more modern version control system. I agree. I have never understood why people on this list are attached to it. I was thinking about creating remote-helper for cvs-fast-export, so that git can use local CVS repository as remote, using e.g. cvsroot::path as repo URL, and using this mechanism for incremental import (aka fetch). (Or even cvssync::URL for automatic cvssync + cvs-fast-export). But from what I understand this is not as easy as it seems, even with remote-helper API having support for fast-import stream. It's a swamp I wouldn't want to walk into. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] Add and use skip_prefix_defval()
On Wed, Dec 18, 2013 at 09:53:48PM +0700, Nguyễn Thái Ngọc Duy wrote: This is a variant of skip_prefix() that returns a specied pointer instead of NULL if no prefix is found. It's helpful to simplify if (starts_with(foo, bar)) foo += 3; into foo = skip_prefix_gently(foo, bar, foo); Should this be skip_prefix_defval instead of skip_prefix_gently? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats
On Wed, Dec 18, 2013 at 1:37 PM, Karsten Blees karsten.bl...@gmail.com wrote:
Am 11.12.2013 08:46, schrieb Christian Couder:
+enum repl_fmt { SHORT, MEDIUM, FULL };
SHORT is predefined on Windows, could you choose another name?
Ok, I will change to:
enum repl_fmt { SHORT_FMT, MEDIUM_FMT, FULL_FMT };
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
On Wed, Dec 18, 2013 at 11:27:10AM -0500, Eric S. Raymond wrote: For use in reposurgeon I have defined a generic cross-VCS reference to commit I call an action stamp; it consists of an RFC3339 date followed by a committer email address. Here's an example: 2013-02-06T09:35:10Z!e...@thyrsus.com In any VCS with changesets (git, Subversion, bzr, Mercurial) this almost always suffices to uniquely identify a commit. The almost is because in these systems it is possible for a user to do multiple commits in the same second. FWIW, this has quite a few collisions in git.git: $ git log --format='%ct %ce' | sort | uniq -c | sort -rn | head 22 1172221032 normalper...@yhbt.net 22 1172221031 normalper...@yhbt.net 22 1172221029 normalper...@yhbt.net 21 1190197351 gits...@pobox.com 21 1172221030 normalper...@yhbt.net 20 1190197350 gits...@pobox.com 17 1172221033 normalper...@yhbt.net 15 1263457676 gits...@pobox.com 15 1193717011 gits...@pobox.com 14 1367447590 gits...@pobox.com In git, it may happen quite a bit during git am or git rebase, in which a large number of commits are replayed in a tight loop. You can use the author timestamp instead, but it also collides (try %at %ae in the above command instead). And now you know why I wish git had subsecond timestamp resolution! If it did, uniqueness of these in a git stream could be guaranteed. It's still not guaranteed. Even with sufficient resolution that no two operations could possibly complete in the same time unit, clocks do not always march forward. They get reset, they may skew from machine to machine, the same operation may happen on different machines, etc. The probability of such collisions is significantly reduced, though, if only because the extra precision adds an essentially random factor. But in some cases you might even see the same commit replayed on top of different parts of the graph, or affecting different paths (e.g., by filter-branch). I.e., no matter what your precision, multiple hacked-up views of the changeset will still always have that same timestamp. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hashmap.h: Use 'unsigned int' for hash-codes everywhere
Karsten Blees karsten.bl...@gmail.com writes: OK, this one's a no-brainer I think. See $gmane/239430 for the latest proposal on the struct packing front. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/12] Make starts_with() a wrapper of skip_prefix()
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
starts_with() started out as a copy of prefixcmp(). But if we don't
care about the sorting order, the logic looks closer to
skip_prefix(). This looks like a good thing to do.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Sure, but the implementation of skip_prefix() scans the prefix
string twice, while prefixcmp() aka starts_with() does it only once.
I'd expect a later step in this series would rectify this micro
regression in the performance, though ;-)
git-compat-util.h | 6 +-
strbuf.c | 9 -
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index b73916b..84f1078 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -350,7 +350,6 @@ extern void set_die_routine(NORETURN_PTR void
(*routine)(const char *err, va_lis
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
extern void set_die_is_recursing_routine(int (*routine)(void));
-extern int starts_with(const char *str, const char *prefix);
extern int prefixcmp(const char *str, const char *prefix);
extern int ends_with(const char *str, const char *suffix);
extern int suffixcmp(const char *str, const char *suffix);
@@ -361,6 +360,11 @@ static inline const char *skip_prefix(const char *str,
const char *prefix)
return strncmp(str, prefix, len) ? NULL : str + len;
}
+static inline int starts_with(const char *str, const char *prefix)
+{
+ return skip_prefix(str, prefix) != NULL;
+}
+
#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
#ifndef PROT_READ
diff --git a/strbuf.c b/strbuf.c
index 83caf4a..bd4c0d8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,15 +1,6 @@
#include cache.h
#include refs.h
-int starts_with(const char *str, const char *prefix)
-{
- for (; ; str++, prefix++)
- if (!*prefix)
- return 1;
- else if (*str != *prefix)
- return 0;
-}
-
int prefixcmp(const char *str, const char *prefix)
{
for (; ; str++, prefix++)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] Add and use skip_prefix_defval()
Kent R. Spillner kspill...@acm.org writes: On Wed, Dec 18, 2013 at 09:53:48PM +0700, Nguyễn Thái Ngọc Duy wrote: This is a variant of skip_prefix() that returns a specied pointer instead of NULL if no prefix is found. It's helpful to simplify if (starts_with(foo, bar)) foo += 3; into foo = skip_prefix_gently(foo, bar, foo); Should this be skip_prefix_defval instead of skip_prefix_gently? Yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Joey Hess j...@kitenet.net writes:
In sha1_file.c, when git is built on linux, it will use
getrlimit(RLIMIT_NOFILE). I've been deploying git binaries to some
unusual systems, like embedded NAS devices, and it seems some with older
kernels like 2.6.33 fail with fatal: cannot get RLIMIT_NOFILE: Bad address.
I could work around this by building git without RLIMIT_NOFILE defined,
but perhaps it would make sense to improve the code to fall back
to one of the other methods for getting the limit, and/or return the
hardcoded 1 as a fallback. This would make git binaries more robust
against old/broken/misconfigured kernels.
Hmph, perhaps you are right. Like this?
sha1_file.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index daacc0c..a3a0014 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -809,8 +809,12 @@ static unsigned int get_max_fd_limit(void)
#ifdef RLIMIT_NOFILE
struct rlimit lim;
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
+ if (getrlimit(RLIMIT_NOFILE, lim)) {
+ static int warn_only_once;
+ if (!warn_only_once++)
+ warning(cannot get RLIMIT_NOFILE: %s,
strerror(errno));
+ return 1; /* see the caller ;-) */
+ }
return lim.rlim_cur;
#elif defined(_SC_OPEN_MAX)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/12] Hard coded string length cleanup
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: I reimplemented skip_prefix() again just to realize this function already exists. Which reminds me there are a bunch of places that could benefit from this function, the same reason that I wanted to reimplement it. So this is series to make it more popular (so hopefully I'll see it used somewhere and know that it exists) and the code cleaner. The pattern compare a string, then skip the compared part by a hard coded string length is almost killed. I left a few in places for those who want to contribute :) Overall the goal of getting rid of the if the first N bytes of the ptr match this string, advance ptr by N bytes pattern, whether the first N is implicitly given by using prefixcmp() or explicitly given by using memcmp(), is very good. And the result looked good from a cursory read---though I have not gone through the series with fine toothed comb yet. Thanks. Nguyễn Thái Ngọc Duy (12): Make starts_with() a wrapper of skip_prefix() Convert starts_with() to skip_prefix() for option parsing Add and use skip_prefix_defval() Replace some use of starts_with() with skip_prefix() Convert a lot of starts_with() to skip_prefix() fetch.c: replace some use of starts_with() with skip_prefix() connect.c: replace some use of starts_with() with skip_prefix() refs.c: replace some use of starts_with() with skip_prefix() diff.c: reduce code duplication in --stat-xxx parsing environment.c: replace starts_with() in strip_namespace() with skip_prefix() diff.c: convert diff_scoreopt_parse to use skip_prefix() refs.c: use skip_prefix() in prune_ref() builtin/branch.c | 3 +- builtin/checkout.c | 6 +- builtin/fast-export.c| 3 +- builtin/fetch-pack.c | 13 ++-- builtin/fetch.c | 16 ++--- builtin/for-each-ref.c | 9 +-- builtin/index-pack.c | 17 +++--- builtin/ls-remote.c | 9 +-- builtin/mailinfo.c | 11 ++-- builtin/merge.c | 12 ++-- builtin/reflog.c | 9 +-- builtin/remote.c | 3 +- builtin/rev-parse.c | 41 ++--- builtin/send-pack.c | 18 +++--- builtin/show-branch.c| 14 ++--- builtin/unpack-objects.c | 5 +- builtin/update-ref.c | 21 +++ commit.c | 5 +- connect.c| 6 +- daemon.c | 75 +++ diff.c | 153 +-- environment.c| 4 +- fetch-pack.c | 9 +-- git-compat-util.h| 15 - git.c| 16 ++--- http-backend.c | 5 +- http-push.c | 6 +- http.c | 5 +- log-tree.c | 5 +- merge-recursive.c| 13 ++-- notes.c | 6 +- pager.c | 2 +- pathspec.c | 5 +- pretty.c | 3 +- refs.c | 20 --- revision.c | 60 +-- setup.c | 3 +- sha1_name.c | 12 +--- strbuf.c | 9 --- tag.c| 7 +-- transport-helper.c | 15 +++-- transport.c | 14 +++-- upload-pack.c| 5 +- wt-status.c | 15 ++--- 44 files changed, 334 insertions(+), 369 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/12] Make starts_with() a wrapper of skip_prefix()
Junio C Hamano gits...@pobox.com writes:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
starts_with() started out as a copy of prefixcmp(). But if we don't
care about the sorting order, the logic looks closer to
skip_prefix(). This looks like a good thing to do.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Sure, but the implementation of skip_prefix() scans the prefix
string twice, while prefixcmp() aka starts_with() does it only once.
I'd expect a later step in this series would rectify this micro
regression in the performance, though ;-)
... and I did not see one, but it would be trivial on top.
git-compat-util.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index b72a80d..59265af 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -356,8 +356,11 @@ extern int suffixcmp(const char *str, const char *suffix);
static inline const char *skip_prefix_defval(const char *str, const char
*prefix, const char *defval)
{
- size_t len = strlen(prefix);
- return strncmp(str, prefix, len) ? defval : str + len;
+ for ( ; ; str++, prefix++)
+ if (!*prefix)
+ return str;
+ else if (*str != *prefix)
+ return defval;
}
static inline const char *skip_prefix(const char *str, const char *prefix)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Junio C Hamano wrote: Hmph, perhaps you are right. Like this? Works for me. -- see shy jo signature.asc Description: Digital signature
Re: git add -A fails in empty repository since 1.8.5
Duy Nguyen pclo...@gmail.com writes:
On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@gmail.com wrote:
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349).
On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
This was discussed on the Git user list recently [1].
#in a repo with no files
git add -A
fatal: pathspec '.' did not match any files
The same goes for git add . (and -u).
Whereas I think some warning feedback is useful, we are curious
whether this is an intentional change or not.
The logic to produce that error message is primarily to catch a typo
like:
$ git add Nakefile
when the user meant to say Makefile.
It could be argued that a git add [any option] ., with an
explicit . given by the end-user, that is run in an empty
directory may be an error worth reporting. Just like it is likely
for the user to have wanted to add some other file when he typed
Nakefile and it is not good to silently decide ah, nothing matches
the pathspec, so not adding anything is the right thing to do in
such a case, it is plausible that the user thought that he was in
some other directory he wanted to add its contents to the index when
he gave us the explicit ., while he was in fact in a wrong
directory, and it is not good to silently decide nothing there to
add so I won't do anything without any indication of an error.
We should *not* error out git add [any option] without any
end-user pathspecs, especially with that error message, on the other
hand.
I was not aware of this case when I made the change. It's caused by
this change that removes pathspec.raw[i][0] check in builtin/add.c in
84b8b5d .
- for (i = 0; pathspec.raw[i]; i++) {
- if (!seen[i] pathspec.raw[i][0]
-!file_exists(pathspec.raw[i])) {
+ for (i = 0; i pathspec.nr; i++) {
+ const char *path = pathspec.items[i].match;
+ if (!seen[i] !file_exists(path)) {
Adding it back requires some thinking because path in the new code
could be something magic.. and the new behavior makes sense, so I'm
inclined to keep it as is, unless people have other opinions.
[1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
Jeff King p...@peff.net: In git, it may happen quite a bit during git am or git rebase, in which a large number of commits are replayed in a tight loop. That's a good point - a repeatable real-world case in which we can expect that behavior. This case could be solved, though, with a slight tweak to the commit generator in git (given subsecond timestamps). It could keep the time of last commit and stall by an arbitrary small amount, enough to show up as a timestamp difference. Action stamps work pretty well inside reposurgeon because they're mainly used to identify commits from older VCSes that can't run that fast. Collisions are theoretically possible but I'm never seen one in the wild. You can use the author timestamp instead, but it also collides (try %at %ae in the above command instead). Yes, obviously for the same reason. And now you know why I wish git had subsecond timestamp resolution! If it did, uniqueness of these in a git stream could be guaranteed. It's still not guaranteed. Even with sufficient resolution that no two operations could possibly complete in the same time unit, clocks do not always march forward. They get reset, they may skew from machine to machine, the same operation may happen on different machines, etc. Right...but the *same person* submitting operations from *different machines* within the time window required to be caught by these effects is at worst fantastically unlikely. That case is exactly why action stamps have an email part. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
Jeff King p...@peff.net writes:
Converting it to use strbuf looks like it will actually let us drop a
bunch of copying, too, as we just end up in mkpath at the very lowest
level. I.e., something like below.
Thanks; I may have a few minor comments, but overall, I like the
placement of mkpath() in the resulting callchain a lot better than
the original.
As an aside, I have noticed us using this push/pop approach to treating a
strbuf as a stack of paths elsewhere, too. I.e:
size_t baselen = base-len;
strbuf_addf(base, /%s, some_thing);
some_call(base);
base-len = baselen;
I wondered if there was any kind of helper we could add to make it look
nicer. But I don't think so; the hairy part is that you must remember to
reset base-len after the call, and there is no easy way around that in
C. If you had object destructors that ran as the stack unwound, or
dynamic scoping, it would be easy to manipulate the object. Wrapping
won't work because strbuf isn't just a length wrapping an immutable
buffer; it actually has to move the NUL in the buffer.
Anyway, not important, but perhaps somebody is more clever than I am.
Hmph... interesting but we would need a lot more thought than the
time necessary to respond to one piece of e-mail for this ;-)
Perhaps later...
diff --git a/builtin/prune.c b/builtin/prune.c
index 6366917..4ca8ec1 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -17,9 +17,8 @@ static int verbose;
static unsigned long expire;
static int show_progress = -1;
-static int prune_tmp_object(const char *path, const char *filename)
+static int prune_tmp_object(const char *fullpath)
{
- const char *fullpath = mkpath(%s/%s, path, filename);
struct stat st;
if (lstat(fullpath, st))
return error(Could not stat '%s', fullpath);
This function is called to remove
* Any tmp_* found directly in .git/objects/
* Any tmp_* found directly in .git/objects/pack/
* Any tmp_obj_* found directly in .git/objects/??/
and shares the same expiration logic with prune_object(). The only
difference from the other function is what the file is called in
dry-run or verbose report (stale temporary file vs sha-1 typename).
We may want to rename it to prune_tmp_file(); its usage may have
been limited to an unborn loose object file at some point in the
history, but it does not look that way in today's code.
-static int prune_dir(int i, char *path)
+static int prune_dir(int i, struct strbuf *path)
{
- DIR *dir = opendir(path);
+ size_t baselen = path-len;
+ DIR *dir = opendir(path-buf);
struct dirent *de;
if (!dir)
@@ -77,28 +76,39 @@ static int prune_dir(int i, char *path)
if (lookup_object(sha1))
continue;
- prune_object(path, de-d_name, sha1);
+ strbuf_addf(path, /%s, de-d_name);
+ prune_object(path-buf, sha1);
+ path-len = baselen;
This is minor, but I prefer using strbuf_setlen() for this.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Jeff King p...@peff.net writes:
That is, does sysconf actually work on such a system (or does it need a
similar run-time fallback)? And either way, we should try falling back
to OPEN_MAX rather than 1 if we have it.
Interesting.
As far as the warning, I am not sure I see a point. The user does not
have any useful recourse, and git should continue to operate as normal.
Having every single git invocation print by the way, RLIMIT_NOFILE does
not work on your system seems like it would get annoying.
Very true. That makes the resulting function look like this:
8 --
static unsigned int get_max_fd_limit(void)
{
#ifdef RLIMIT_NOFILE
struct rlimit lim;
if (!getrlimit(RLIMIT_NOFILE, lim))
return lim.rlim_cur;
#endif
#if defined(_SC_OPEN_MAX)
{
long sc_open_max = sysconf(_SC_OPEN_MAX);
if (0 sc_open_max)
return sc_open_max;
}
#if defined(OPEN_MAX)
return OPEN_MAX;
#else
return 1; /* see the caller ;-) */
#endif
}
8 --
But the sysconf part makes me wonder; here is what we see in
http://pubs.opengroup.org/onlinepubs/9699919799/functions/sysconf.html
If name is an invalid value, sysconf() shall return -1 and set errno
to indicate the error. If the variable corresponding to name is
described in limits.h as a maximum or minimum value and the
variable has no limit, sysconf() shall return -1 without changing
the value of errno. Note that indefinite limits do not imply
infinite limits; see limits.h.
For a broken system (like RLIMIT_NOFILE defined for the compiler,
but the actual call returns a bogus error), the compiler may see the
_SC_OPEN_MAX defined, while sysconf() may say I've never heard of
such a name and return -1, or the system, whether broken or not,
may want to say Unlimited and return -1. The caller takes
anything unreasonable as a positive value capped to 25 or something,
so there isn't a real harm if we returned a bogus value from here,
but I am not sure what the safe default behaviour of this function
should be to help such a broken system while not harming systems
that are functioning correctly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
On Wed, Dec 18, 2013 at 11:53:47AM -0500, Martin Langhoff wrote: On Wed, Dec 18, 2013 at 11:27 AM, Eric S. Raymond e...@thyrsus.com wrote: Anyway I hope that incremental CVS import would be needed less and less as CVS is replaced by any more modern version control system. I agree. I have never understood why people on this list are attached to it. I think I have answered this question already once in this thread, and a few times in similar threads with Eric in the past. People track CVS repos that they have not control over. Smart programmers forced to work with a corporate CVS repo. It happens also with SVN, and witness the popularity of git-svn which can sanely interact with an active svn repo. This is a valid use case. Hard (impossible?) to support. But there should be no surprise as to its reasons. And at this point the git-cvsimport manpage says: WARNING: git cvsimport uses cvsps version 2, which is considered deprecated; it does not work with cvsps version 3 and later. If you are performing a one-shot import of a CVS repository consider using cvs2git[1] or parsecvs[2]. Which I think sums up the position nicely; if you're doing a one-shot import then the standalone tools are going to be a better choice, but if you're trying to use Git for your work on top of CVS the only choice is cvsps with git-cvsimport. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: It could be argued that a git add [any option] ., with an explicit . given by the end-user, that is run in an empty directory may be an error worth reporting. But what we have right now is really weird: I know. That is why I said It _could_ be argued. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
On Wed, Dec 18, 2013 at 11:35:34AM -0800, Junio C Hamano wrote: This function is called to remove * Any tmp_* found directly in .git/objects/ * Any tmp_* found directly in .git/objects/pack/ * Any tmp_obj_* found directly in .git/objects/??/ and shares the same expiration logic with prune_object(). The only difference from the other function is what the file is called in dry-run or verbose report (stale temporary file vs sha-1 typename). We may want to rename it to prune_tmp_file(); its usage may have been limited to an unborn loose object file at some point in the history, but it does not look that way in today's code. Yes, certainly the rename makes sense (and I think the history is as you mentioned). I noticed the similarity between the two, as well. It might be simple to refactor into a single function. - prune_object(path, de-d_name, sha1); + strbuf_addf(path, /%s, de-d_name); + prune_object(path-buf, sha1); + path-len = baselen; This is minor, but I prefer using strbuf_setlen() for this. Good catch. I do not think it is minor at all; my version is buggy. After the loop ends, path-len does not match the NUL in path-buf. That is OK if the next caller is strbuf-aware, but if it were to pass path-buf straight to a system call, that would be rather...confusing. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Jeff King wrote: I wish we understood why getrlimit was failing. Returning EFAULT seems like an odd choice if it is not implemented for the system. On such a system, do the other fallbacks actually work? Would it work to do: That is, does sysconf actually work on such a system (or does it need a similar run-time fallback)? And either way, we should try falling back to OPEN_MAX rather than 1 if we have it. For what it's worth, the system this happened on was a QNAP TS-219PII Linux willow 2.6.33.2 #1 Fri Mar 1 04:41:48 CST 2013 armv5tel unknown I don't have access to it to run tests of sysconf. (I already suggested its owner upgrade its firmware.) As far as the warning, I am not sure I see a point. The user does not have any useful recourse, and git should continue to operate as normal. Having every single git invocation print by the way, RLIMIT_NOFILE does not work on your system seems like it would get annoying. I agree with that. -- see shy jo signature.asc Description: Digital signature
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
Jeff King p...@peff.net writes:
+ prune_object(path-buf, sha1);
+ path-len = baselen;
This is minor, but I prefer using strbuf_setlen() for this.
Good catch. I do not think it is minor at all; my version is buggy.
After the loop ends, path-len does not match the NUL in path-buf. That
is OK if the next caller is strbuf-aware, but if it were to pass
path-buf straight to a system call, that would be rather...confusing.
Hmph, rmdir(path-buf) at the end of prune_dir() may have that exact
issue.
Will squash in the following.
builtin/prune.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/prune.c b/builtin/prune.c
index 4ca8ec1..99f3f35 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -17,7 +17,7 @@ static int verbose;
static unsigned long expire;
static int show_progress = -1;
-static int prune_tmp_object(const char *fullpath)
+static int prune_tmp_file(const char *fullpath)
{
struct stat st;
if (lstat(fullpath, st))
@@ -78,13 +78,13 @@ static int prune_dir(int i, struct strbuf *path)
strbuf_addf(path, /%s, de-d_name);
prune_object(path-buf, sha1);
- path-len = baselen;
+ strbuf_setlen(path, baselen);
continue;
}
if (!prefixcmp(de-d_name, tmp_obj_)) {
strbuf_addf(path, /%s, de-d_name);
- prune_tmp_object(path-buf);
- path-len = baselen;
+ prune_tmp_file(path-buf);
+ strbuf_setlen(path, baselen);
continue;
}
fprintf(stderr, bad sha1 file: %s/%s\n, path-buf,
de-d_name);
@@ -108,7 +108,7 @@ static void prune_object_dir(const char *path)
for (i = 0; i 256; i++) {
strbuf_addf(buf, %02x, i);
prune_dir(i, buf);
- buf.len = baselen;
+ strbuf_setlen(buf, baselen);
}
}
@@ -130,7 +130,7 @@ static void remove_temporary_files(const char *path)
}
while ((de = readdir(dir)) != NULL)
if (!prefixcmp(de-d_name, tmp_))
- prune_tmp_object(mkpath(%s/%s, path, de-d_name));
+ prune_tmp_file(mkpath(%s/%s, path, de-d_name));
closedir(dir);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
On Wed, Dec 18, 2013 at 12:07:02PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: +prune_object(path-buf, sha1); +path-len = baselen; This is minor, but I prefer using strbuf_setlen() for this. Good catch. I do not think it is minor at all; my version is buggy. After the loop ends, path-len does not match the NUL in path-buf. That is OK if the next caller is strbuf-aware, but if it were to pass path-buf straight to a system call, that would be rather...confusing. Hmph, rmdir(path-buf) at the end of prune_dir() may have that exact issue. Will squash in the following. Thanks. Are you picking this up with a commit message, or did you want me to re-send with the usual message/signoff? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
Jeff King p...@peff.net writes:
On Wed, Dec 18, 2013 at 12:07:02PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
+ prune_object(path-buf, sha1);
+ path-len = baselen;
This is minor, but I prefer using strbuf_setlen() for this.
Good catch. I do not think it is minor at all; my version is buggy.
After the loop ends, path-len does not match the NUL in path-buf. That
is OK if the next caller is strbuf-aware, but if it were to pass
path-buf straight to a system call, that would be rather...confusing.
Hmph, rmdir(path-buf) at the end of prune_dir() may have that exact
issue.
Will squash in the following.
Thanks. Are you picking this up with a commit message, or did you want
me to re-send with the usual message/signoff?
I think this should be sufficient ;-)
-- 8 --
From: Jeff King p...@peff.net
Date: Tue, 17 Dec 2013 18:22:31 -0500
Subject: [PATCH] builtin/prune.c: use strbuf to avoid having to worry about
PATH_MAX
While at it, rename prune_tmp_object(), which used to be a helper to
remove temporary files that were created to become loose object
files, to prune_tmp_file(), as the function is also used to remove
any random cruft whose name begins with tmp_ directly in .git/object
or .git/object/pack directories these days.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/prune.c | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/builtin/prune.c b/builtin/prune.c
index 6366917..99f3f35 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -17,9 +17,8 @@ static int verbose;
static unsigned long expire;
static int show_progress = -1;
-static int prune_tmp_object(const char *path, const char *filename)
+static int prune_tmp_file(const char *fullpath)
{
- const char *fullpath = mkpath(%s/%s, path, filename);
struct stat st;
if (lstat(fullpath, st))
return error(Could not stat '%s', fullpath);
@@ -32,9 +31,8 @@ static int prune_tmp_object(const char *path, const char
*filename)
return 0;
}
-static int prune_object(char *path, const char *filename, const unsigned char
*sha1)
+static int prune_object(const char *fullpath, const unsigned char *sha1)
{
- const char *fullpath = mkpath(%s/%s, path, filename);
struct stat st;
if (lstat(fullpath, st))
return error(Could not stat '%s', fullpath);
@@ -50,9 +48,10 @@ static int prune_object(char *path, const char *filename,
const unsigned char *s
return 0;
}
-static int prune_dir(int i, char *path)
+static int prune_dir(int i, struct strbuf *path)
{
- DIR *dir = opendir(path);
+ size_t baselen = path-len;
+ DIR *dir = opendir(path-buf);
struct dirent *de;
if (!dir)
@@ -77,28 +76,39 @@ static int prune_dir(int i, char *path)
if (lookup_object(sha1))
continue;
- prune_object(path, de-d_name, sha1);
+ strbuf_addf(path, /%s, de-d_name);
+ prune_object(path-buf, sha1);
+ strbuf_setlen(path, baselen);
continue;
}
if (!prefixcmp(de-d_name, tmp_obj_)) {
- prune_tmp_object(path, de-d_name);
+ strbuf_addf(path, /%s, de-d_name);
+ prune_tmp_file(path-buf);
+ strbuf_setlen(path, baselen);
continue;
}
- fprintf(stderr, bad sha1 file: %s/%s\n, path, de-d_name);
+ fprintf(stderr, bad sha1 file: %s/%s\n, path-buf,
de-d_name);
}
closedir(dir);
if (!show_only)
- rmdir(path);
+ rmdir(path-buf);
return 0;
}
static void prune_object_dir(const char *path)
{
+ struct strbuf buf = STRBUF_INIT;
+ size_t baselen;
int i;
+
+ strbuf_addstr(buf, path);
+ strbuf_addch(buf, '/');
+ baselen = buf.len;
+
for (i = 0; i 256; i++) {
- static char dir[4096];
- sprintf(dir, %s/%02x, path, i);
- prune_dir(i, dir);
+ strbuf_addf(buf, %02x, i);
+ prune_dir(i, buf);
+ strbuf_setlen(buf, baselen);
}
}
@@ -120,7 +130,7 @@ static void remove_temporary_files(const char *path)
}
while ((de = readdir(dir)) != NULL)
if (!prefixcmp(de-d_name, tmp_))
- prune_tmp_object(path, de-d_name);
+ prune_tmp_file(mkpath(%s/%s, path, de-d_name));
closedir(dir);
}
--
1.8.5.2-297-g3e57c29
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: RLIMIT_NOFILE fallback
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
That is, does sysconf actually work on such a system (or does it need a
similar run-time fallback)? And either way, we should try falling back
to OPEN_MAX rather than 1 if we have it.
Interesting.
As far as the warning, I am not sure I see a point. The user does not
have any useful recourse, and git should continue to operate as normal.
Having every single git invocation print by the way, RLIMIT_NOFILE does
not work on your system seems like it would get annoying.
Very true. That makes the resulting function look like this:
8 --
static unsigned int get_max_fd_limit(void)
{
#ifdef RLIMIT_NOFILE
struct rlimit lim;
if (!getrlimit(RLIMIT_NOFILE, lim))
return lim.rlim_cur;
#endif
#if defined(_SC_OPEN_MAX)
{
long sc_open_max = sysconf(_SC_OPEN_MAX);
if (0 sc_open_max)
return sc_open_max;
}
err, here we need
#endif /* defined(_SC_OPEN_MAX) */
to truly implement the structure try all the available functions,
and then fall back to OPEN_MAX.
#if defined(OPEN_MAX)
return OPEN_MAX;
#else
return 1; /* see the caller ;-) */
#endif
}
8 --
But the sysconf part makes me wonder; here is what we see in
http://pubs.opengroup.org/onlinepubs/9699919799/functions/sysconf.html
If name is an invalid value, sysconf() shall return -1 and set errno
to indicate the error. If the variable corresponding to name is
described in limits.h as a maximum or minimum value and the
variable has no limit, sysconf() shall return -1 without changing
the value of errno. Note that indefinite limits do not imply
infinite limits; see limits.h.
For a broken system (like RLIMIT_NOFILE defined for the compiler,
but the actual call returns a bogus error), the compiler may see the
_SC_OPEN_MAX defined, while sysconf() may say I've never heard of
such a name and return -1, or the system, whether broken or not,
may want to say Unlimited and return -1. The caller takes
anything unreasonable as a positive value capped to 25 or something,
so there isn't a real harm if we returned a bogus value from here,
but I am not sure what the safe default behaviour of this function
should be to help such a broken system while not harming systems
that are functioning correctly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
John Keeping j...@keeping.me.uk: Which I think sums up the position nicely; if you're doing a one-shot import then the standalone tools are going to be a better choice, but if you're trying to use Git for your work on top of CVS the only choice is cvsps with git-cvsimport. Which will trash your history - the bugs in that are worse than the bugs in 3.0, which are bad enough that I *terminated* it. Lovely -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer
On Wed, Dec 18, 2013 at 12:15:19PM -0800, Junio C Hamano wrote: Thanks. Are you picking this up with a commit message, or did you want me to re-send with the usual message/signoff? I think this should be sufficient ;-) -- 8 -- From: Jeff King p...@peff.net Date: Tue, 17 Dec 2013 18:22:31 -0500 Subject: [PATCH] builtin/prune.c: use strbuf to avoid having to worry about PATH_MAX While at it, rename prune_tmp_object(), which used to be a helper to remove temporary files that were created to become loose object files, to prune_tmp_file(), as the function is also used to remove any random cruft whose name begins with tmp_ directly in .git/object or .git/object/pack directories these days. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com Great, thanks. You might also want to stick a: Noticed-by: Michael Haggerty mhag...@alum.mit.edu in there. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
Which will trash your history - the bugs in that are worse than the bugs in 3.0, which are bad enough that I *terminated* it. Which *might* trash your history. cvsps v2 and git cvsimport work as advertised with simple, linear CVS repositories. I maintain a git mirror of an active CVS repo and run git cvsimport every few days to sync with the latest upstream changes. The only problem I encountered so far was when you released cvsps v3 and broke git cvsimport. :) I had to manually downgrade to cvsps v2.2b1 and configure my package manager to ignore cvsps updates, but I haven't had any problems since. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
Duy Nguyen pclo...@gmail.com writes:
On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@gmail.com wrote:
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349).
On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
This was discussed on the Git user list recently [1].
#in a repo with no files
git add -A
fatal: pathspec '.' did not match any files
The same goes for git add . (and -u).
Whereas I think some warning feedback is useful, we are curious
whether this is an intentional change or not.
I was not aware of this case when I made the change. It's caused by
this change that removes pathspec.raw[i][0] check in builtin/add.c in
84b8b5d .
- for (i = 0; pathspec.raw[i]; i++) {
- if (!seen[i] pathspec.raw[i][0]
-!file_exists(pathspec.raw[i])) {
+ for (i = 0; i pathspec.nr; i++) {
+ const char *path = pathspec.items[i].match;
+ if (!seen[i] !file_exists(path)) {
Isn't that pathspec.raw[i][0] check merely an attempt to work around
the combination of
(1) the current directory pathspec . is sanitized down to an
empty string by the pathspec code; and
(2) even though file_exists() is willing to say yes to a non-file
(namely, a directory), it is not prepared to take an empty
string resulting from (1) to mean the directory ..
Adding it back requires some thinking because path in the new code
could be something magic..
Ehh, why? Shouldn't something magic that did _not_ match
(i.e. not in seen[]) diagnosed as such?
I am wondering why we even need !file_exists(path) check there in
the first place. We run fill_directory() and then let
prune_directory() report which pathspec did not have any match via
the seen[] array. We also match pathspec against the index to see
if there are pathspec that does not match anything. So at that
point of the codeflow, we ought to be able to make sure that seen[]
is the _only_ thing we need to consult to see if there are any
pathspec elements that did not match.
Stepping back even further, I wonder if this yes, I found a
matching entity and know this is not an end-user typo bit actually
should be _in_ struct pathspec. Traditionally we implemented that
bit as a separate seen[] array parallel to const char **pathspec
array, but that was merely because we only had the list of strings.
Now we express a pathspec as a list of struct pathspec elements,
I think seen[] can and should become part of the pathspec. Am I
missing something?
and the new behavior makes sense, so I'm
inclined to keep it as is, unless people have other opinions.
[1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()
In order to fix branchname DF conflicts during `fetch --prune`, the way
the header is output to the screen needs to be refactored. Here is an
exmaple of the output with the line in question denoted by '':
$ git fetch --prune --dry-run upstream
From https://github.com/git/git
a155a5f..5512ac5 maint - upstream/maint
d7aced9..7794a68 master - upstream/master
523f7c4..3e57c29 next - upstream/next
+ 462f102...0937cdf pu - upstream/pu (forced update)
e24105a..5d352bc todo - upstream/todo
* [new tag] v1.8.5.2 - v1.8.5.2
* [new tag] v1.8.5.2 - v1.8.5.2
pretty_url():
This function when passed a transport url will anonymize the transport
of the url. It will strip a trailing '/'. It will also strip a trailing
'.git'. It will return the newly formated url for use. I do not believe
there is a need for stripping the trailing '/' and '.git' from a url,
but it was already there and I wanted to make as little changes as
possible.
print_url():
This function will convert a transport url to a pretty url using
pretty_url(). Then it will print out the pretty url to stderr as
indicated above in the example output. It uses a global variable
named gshown_url' to prevent this header for being printed twice.
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 60 -
1 file changed, 42 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 3d978eb..b3145f6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,42 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
+static int gshown_url = 0;
+
+static char *pretty_url(const char *raw_url) {
+ if (raw_url) {
+ int url_len, i;
+ char *pretty_url, *url;
+
+ url = transport_anonymize_url(raw_url);
+
+ url_len = strlen(url);
+ for (i = url_len - 1; url[i] == '/' 0 = i; i--)
+ ;
+ url_len = i + 1;
+ if (4 i !strncmp(.git, url + i - 3, 4))
+ url_len = i - 3;
+
+ pretty_url = xcalloc(1, 1 + url_len);
+ memcpy(pretty_url, url, url_len);
+
+ free(url);
+ return pretty_url;
+ }
+ return xstrdup(foreign);
+}
+
+static void print_url(const char *raw_url) {
+ if (!gshown_url) {
+ char *url = pretty_url(raw_url);
+
+ fprintf(stderr, _(From %s\n), url);
+
+ gshown_url = 1;
+ free(url);
+ }
+}
+
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -535,7 +571,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
{
FILE *fp;
struct commit *commit;
- int url_len, i, shown_url = 0, rc = 0;
+ int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
@@ -546,10 +582,8 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
if (!fp)
return error(_(cannot open %s: %s\n), filename,
strerror(errno));
- if (raw_url)
- url = transport_anonymize_url(raw_url);
- else
- url = xstrdup(foreign);
+ url = pretty_url(raw_url);
+ url_len = strlen(url);
rm = ref_map;
if (check_everything_connected(iterate_ref_map, 0, rm)) {
@@ -606,13 +640,6 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
what = rm-name;
}
- url_len = strlen(url);
- for (i = url_len - 1; url[i] == '/' 0 = i; i--)
- ;
- url_len = i + 1;
- if (4 i !strncmp(.git, url + i - 3, 4))
- url_len = i - 3;
-
strbuf_reset(note);
if (*what) {
if (*kind)
@@ -651,13 +678,10 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
REFCOL_WIDTH,
*what ? what : HEAD);
if (note.len) {
- if (verbosity = 0 !shown_url) {
- fprintf(stderr, _(From %.*s\n),
- url_len, url);
- shown_url = 1;
- }
- if (verbosity = 0)
+
[PATCH 3/3] fetch --prune: Repair branchname DF conflicts
When a branchname DF conflict occurs during a fetch, --prune should
be able to fix it. When fetching with --prune, the fetching process
happens before pruning causing the branchname DF conflict to persist
and report an error. This patch prunes before fetching, thus
correcting DF conflicts during a fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e50b697..845c687 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.1.163.gd7aced9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] fetch --prune: Always print header url
If fetch --prune is run with no new refs to fetch, but it has refs
to prune. Then, the header url is not printed as it would if there were
new refs to fetch. the following is example output showing this
behavior:
$ git fetch --prune --dry-run origin
x [deleted] (none) - origin/world
After this patch the output of fetch --prune should look like this:
$ git fetch --prune --dry-run origin
From https://github.com/git/git
x [deleted] (none) - origin/test
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b3145f6..e50b697 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -732,7 +732,8 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
return ret;
}
-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
+ const char *raw_url)
{
int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
@@ -744,6 +745,7 @@ static int prune_refs(struct refspec *refs, int ref_count,
struct ref *ref_map)
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
if (verbosity = 0) {
+ print_url(raw_url);
fprintf(stderr, x %-*s %-*s - %s\n,
TRANSPORT_SUMMARY(_([deleted])),
REFCOL_WIDTH, _((none)),
prettify_refname(ref-name));
@@ -878,11 +880,12 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (ref_count) {
- prune_refs(refs, ref_count, ref_map);
+ prune_refs(refs, ref_count, ref_map, transport-url);
} else {
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr,
- ref_map);
+ ref_map,
+ transport-url);
}
}
free_refs(ref_map);
--
1.8.5.1.163.gd7aced9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
On Wed, Dec 18, 2013 at 11:50:24AM -0800, Junio C Hamano wrote:
8 --
static unsigned int get_max_fd_limit(void)
{
#ifdef RLIMIT_NOFILE
struct rlimit lim;
if (!getrlimit(RLIMIT_NOFILE, lim))
return lim.rlim_cur;
#endif
#if defined(_SC_OPEN_MAX)
{
long sc_open_max = sysconf(_SC_OPEN_MAX);
if (0 sc_open_max)
return sc_open_max;
}
#if defined(OPEN_MAX)
return OPEN_MAX;
#else
return 1; /* see the caller ;-) */
#endif
}
8 --
Yeah, with the #endif followup you posted, this is what I had in mind.
But the sysconf part makes me wonder; here is what we see in
http://pubs.opengroup.org/onlinepubs/9699919799/functions/sysconf.html
If name is an invalid value, sysconf() shall return -1 and set errno
to indicate the error. If the variable corresponding to name is
described in limits.h as a maximum or minimum value and the
variable has no limit, sysconf() shall return -1 without changing
the value of errno. Note that indefinite limits do not imply
infinite limits; see limits.h.
For a broken system (like RLIMIT_NOFILE defined for the compiler,
but the actual call returns a bogus error), the compiler may see the
_SC_OPEN_MAX defined, while sysconf() may say I've never heard of
such a name and return -1, or the system, whether broken or not,
may want to say Unlimited and return -1. The caller takes
anything unreasonable as a positive value capped to 25 or something,
so there isn't a real harm if we returned a bogus value from here,
but I am not sure what the safe default behaviour of this function
should be to help such a broken system while not harming systems
that are functioning correctly.
According to the POSIX quote above, it sounds like we could do:
#if defined (_SC_OPEN_MAX)
{
long max;
errno = 0;
max = sysconf(_SC_OPEN_MAX);
if (0 max) /* got the limit */
return max;
else if (!errno) /* unlimited, cast to int-max */
return max;
/* otherwise, fall through */
}
#endif
Obviously you could collapse the two branches of the conditional, though
I think it deserves at least a comment to explain what is going on.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Jeff King p...@peff.net writes:
According to the POSIX quote above, it sounds like we could do:
#if defined (_SC_OPEN_MAX)
{
long max;
errno = 0;
max = sysconf(_SC_OPEN_MAX);
if (0 max) /* got the limit */
return max;
else if (!errno) /* unlimited, cast to int-max */
return max;
/* otherwise, fall through */
}
#endif
Obviously you could collapse the two branches of the conditional, though
I think it deserves at least a comment to explain what is going on.
Yes, that is locally OK, but depending on how the caller behaves, we
might need to have an extra saved_errno dance here, which I didn't
want to get into...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
On Wed, Dec 18, 2013 at 01:37:24PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
According to the POSIX quote above, it sounds like we could do:
#if defined (_SC_OPEN_MAX)
{
long max;
errno = 0;
max = sysconf(_SC_OPEN_MAX);
if (0 max) /* got the limit */
return max;
else if (!errno) /* unlimited, cast to int-max */
return max;
/* otherwise, fall through */
}
#endif
Obviously you could collapse the two branches of the conditional, though
I think it deserves at least a comment to explain what is going on.
Yes, that is locally OK, but depending on how the caller behaves, we
might need to have an extra saved_errno dance here, which I didn't
want to get into...
I think we are fine. The only caller is about to clobber errno by
closing packs anyway.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()
Tom Miller jacker...@gmail.com writes: In order to fix branchname DF conflicts during `fetch --prune`, the way the header is output to the screen needs to be refactored. Here is an exmaple of the output with the line in question denoted by '': $ git fetch --prune --dry-run upstream From https://github.com/git/git a155a5f..5512ac5 maint - upstream/maint d7aced9..7794a68 master - upstream/master 523f7c4..3e57c29 next - upstream/next + 462f102...0937cdf pu - upstream/pu (forced update) e24105a..5d352bc todo - upstream/todo * [new tag] v1.8.5.2 - v1.8.5.2 * [new tag] v1.8.5.2 - v1.8.5.2 pretty_url(): This function when passed a transport url will anonymize the transport of the url. It will strip a trailing '/'. It will also strip a trailing '.git'. It will return the newly formated url for use. I do not believe there is a need for stripping the trailing '/' and '.git' from a url, but it was already there and I wanted to make as little changes as possible. OK. I tend to agree that stripping the trailing part is probably not a good idea and we would want to remove that but that definitely should be done as a separate step, or even as a separate series on top of this one. print_url(): This function will convert a transport url to a pretty url using pretty_url(). Then it will print out the pretty url to stderr as indicated above in the example output. It uses a global variable named gshown_url' to prevent this header for being printed twice. Gaah. What is that 'g' doing there? Please don't do that meaningless naming. I do not think the change to introduce such a global variable belongs to this refactoring step. The current caller can decide itself if it called that function, and if you are going to introduce new callers in later steps, they can coordinate among themselves, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git fsck fails on malloc of 80 G
From: Jeff King p...@peff.net One of the problems I ran into recently is that corrupt data can cause it to make a large allocation One thing I notice is that in unpack_compressed_entry() in sha1_file.c, there is a mallocz of size bytes. It appears that size is the size of the object that is being unpacked. If so, this code cannot be correct, because it assumes that any file that is stored in the repository can be put into a buffer allocated in RAM. Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] prune-packed: fix a possible buffer overflow
On 12/17/2013 07:43 PM, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: Why don't we take this opportunity to replace that array with a strbuf? The conversion looks simple with this function. Indeed. Something like this, perhaps? [...] Frankly, with my initial patches I was just trying to paper over the bug with the smallest possible change. It's nice that people are attempting bigger improvements. I went in a slightly different direction: I am spiking out an API for iterating over loose object files. It would be useful in a couple of places. [While doing so, I got sidetracked by the question: what happens if a prune process deletes the objects/XX directory just the same moment that another process is trying to write an object into that directory? I think the relevant function is sha1_file.c:create_tmpfile(). It looks like there is a nonzero but very small race window that could result in a spurious unable to create temporary file error, but even then I don't think there would be any corruption or anything.] But don't let me stop you; the cleanups you are working on are definitely nice and are complementary to my ideas. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
Jeff King p...@peff.net writes:
On Wed, Dec 18, 2013 at 01:37:24PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
According to the POSIX quote above, it sounds like we could do:
#if defined (_SC_OPEN_MAX)
{
long max;
errno = 0;
max = sysconf(_SC_OPEN_MAX);
if (0 max) /* got the limit */
return max;
else if (!errno) /* unlimited, cast to int-max */
return max;
/* otherwise, fall through */
}
#endif
Obviously you could collapse the two branches of the conditional, though
I think it deserves at least a comment to explain what is going on.
Yes, that is locally OK, but depending on how the caller behaves, we
might need to have an extra saved_errno dance here, which I didn't
want to get into...
I think we are fine. The only caller is about to clobber errno by
closing packs anyway.
-Peff
OK.
-- 8 --
Subject: [PATCH] get_max_fd_limit(): fall back to OPEN_MAX upon
getrlimit/sysconf failure
On broken systems where RLIMIT_NOFILE is visible by the compliers
but underlying getrlimit() system call does not behave, we used to
simply die() when we are trying to decide how many file descriptors
to allocate for keeping packfiles open. Instead, allow the fallback
codepath to take over when we get such a failure from getrlimit().
The same issue exists with _SC_OPEN_MAX and sysconf(); restructure
the code in a similar way to prepare for a broken sysconf() as well.
Noticed-by: Joey Hess j...@kitenet.net
Helped-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
sha1_file.c | 37 ++---
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 760dd60..288badd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -807,15 +807,38 @@ void free_pack_by_name(const char *pack_name)
static unsigned int get_max_fd_limit(void)
{
#ifdef RLIMIT_NOFILE
- struct rlimit lim;
+ {
+ struct rlimit lim;
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
+ if (!getrlimit(RLIMIT_NOFILE, lim))
+ return lim.rlim_cur;
+ }
+#endif
+
+#ifdef _SC_OPEN_MAX
+ {
+ long open_max = sysconf(_SC_OPEN_MAX);
+ if (0 open_max)
+ return open_max;
+ /*
+* Otherwise, we got -1 for one of the two
+* reasons:
+*
+* (1) sysconf() did not understand _SC_OPEN_MAX
+* and signaled an error with -1; or
+* (2) sysconf() said there is no limit.
+*
+* We _could_ clear errno before calling sysconf() to
+* tell these two cases apart and return a huge number
+* in the latter case to let the caller cap it to a
+* value that is not so selfish, but letting the
+* fallback OPEN_MAX codepath take care of these cases
+* is a lot simpler.
+*/
+ }
+#endif
- return lim.rlim_cur;
-#elif defined(_SC_OPEN_MAX)
- return sysconf(_SC_OPEN_MAX);
-#elif defined(OPEN_MAX)
+#ifdef OPEN_MAX
return OPEN_MAX;
#else
return 1; /* see the caller ;-) */
--
1.8.5.2-297-g3e57c29
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug when moving submodules (I think?)
I tried to make a script to repro this from scratch but ran into other issues, which may or may not be a bug. I'll put that at the end. To repro all you have to do is: git checkout git://github.com/frioux/dotfiles git reset --hard 92c85161ceec9e52b0b2d2de893ba11f49c80198 git mv zsh .zsh (sha included so this email continues to be valid in the future) You can now see that .git/index.lock has been left behind. On a non-fresh checkout (I'm not sure why my old checkout is special) I get the following error: git: builtin/mv.c:248: cmd_mv: Assertion `pos = 0' failed. I assumed this was just related to moving submodules that are in subdirectories, but when I do that from a fresh repo I get a different error. mkdir -p test/a test/b cd test/a git init touch a.txt git add a.txt git ci -m 'initial commit' cd ../b git init mkdir c touch c/c.txt git submodule add ../a c/a git ci -m 'initial commit' git mv c d git status And the error: fatal: Could not chdir to '../../../../c/a': No such file or directory fatal: 'git status --porcelain' failed in submodule d/a I am using git v1.8.5.1 built from source on the latest ubuntu. If there is anything else I can do to help repro this please do not hesitate to ask. -- fREW Schmidt http://blog.afoolishmanifesto.com pgpgI6eNDJFpI.pgp Description: PGP signature
Re: I have end-of-lifed cvsps
On 12/17/2013 07:47 PM, Eric S. Raymond wrote: Johan Herland jo...@herland.net: However, I fear that you underestimate the number of users that want to use Git against CVS repos that are orders of magnitude larger (in both dimensions: #commits and #files) than your example repo. You may be right. See below... I'm working with Alan Barret now on trying to convert the NetBSD repositories. They break cvs-fast-export through sheer bulk of metadata, by running the machine out of core. This is exactly the kind of huge case that you're talking about. Alan and I are going to take a good hard whack at modifying cvs-fast-export to make this work. Because there really aren't any feasible alternatives. The analysis code in cvsps was never good enough. cvs2git, being written in Python, would hit the core limit faster than anything written in C. cvs2git goes to great lengths to store intermediate data to disk and keep the working set small and therefore (despite the Python overhead) I am confident that it scales better than cvs-fast-export. My usual test repo was gcc: Total CVS Files: 25013 Total CVS Revisions:578010 Total CVS Branches:1487929 Total CVS Tags: 11435500 Total Unique Tags: 814 Total Unique Branches: 116 CVS Repos Size in KB: 2074248 Total SVN Commits: 64501 I also regularly converted mozilla (4.2 GB) and emacs (560 MB) for testing purposes. These could all be converted on a 32-bit computer. Other projects that cvs2svn/cvs2git could handle: FreeBSD, Gentoo, KDE, GNOME, PostgreSQL. (Though for KDE, which I think was in the 16 GB range, I know that they used a giant machine for the conversion.) If you haven't tried cvs2git yet, please start it up somewhere in the background. It might take a while but it should have no trouble with your repos, and then you can compare the tools based on experience rather than speculation. Which matters, because right now the set of people working on CVS lifters begins with me and ends with Michael Rafferty (cvs2git), who seems even less interested in incremental conversion than I am. Unless somebody comes out of nowhere and wants to own that problem, it's not going to get solved. A correct incremental converter could be done (as long as the CVS users don't literally change history retroactively) but it would be a lot of work. Parsing the CVS files isn't the problem; after all, CVS has to do that every time you check out a branch. The problem is the extra bookkeeping that would be needed to keep the overlapping history consistent between runs N and N+1 of the tool. I sketched out what would be necessary once and it came out to several solid weeks of work. But the traffic on the cvs2svn/cvs2git mailing list has trailed off essentially to zero, so either the software is perfect already (haha) or most everybody has already converted. Therefore I don't invest any significant time in that project these days. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] prune-packed: fix a possible buffer overflow
On Wed, Dec 18, 2013 at 11:44:58PM +0100, Michael Haggerty wrote: [While doing so, I got sidetracked by the question: what happens if a prune process deletes the objects/XX directory just the same moment that another process is trying to write an object into that directory? I think the relevant function is sha1_file.c:create_tmpfile(). It looks like there is a nonzero but very small race window that could result in a spurious unable to create temporary file error, but even then I don't think there would be any corruption or anything.] There's a race there, but I think it's hard to trigger. Our strategy with object creation is to call open, recognize ENOENT, mkdir, and then try again. If the rmdir happens before our call to open, then we're fine. If open happens first, then the rmdir will fail. But we don't loop on ENOENT. So if the rmdir happens in the middle, after the mkdir but before we call open again, we'd fail, because we don't treat ENOENT specially in the second call to open. That is unlikely to happen, though, as prune would not be removing a directory it did not just enter and clean up an object from (in which case we would not have gotten the first ENOENT in the creator). I think you'd So you'd have to have something creating and then pruning the directory in the time between our open and mkdir. It would probably be more likely to see it if you had two prunes running (the first one kills the directory, creator notices and calls mkdir, then the second prune kills the directory, too). So it seems unlikely and the worst case is a temporary failure, not a corruption. It's probably not worth caring too much about, but we could solve it pretty easily by looping on ENOENT on creation. On a similar note, I imagine that a simultaneous branch foo/bar and branch -d foo/baz could race over the creation/deletion of refs/heads/foo, but I didn't look into it. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RLIMIT_NOFILE fallback
On Wed, Dec 18, 2013 at 02:59:12PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Yes, that is locally OK, but depending on how the caller behaves, we
might need to have an extra saved_errno dance here, which I didn't
want to get into...
I think we are fine. The only caller is about to clobber errno by
closing packs anyway.
Also, I do not think we would be any worse off than the current code.
getrlimit almost certainly just clobbered errno anyway. Either it is
worth saving for the whole function, or not at all (and I think not at
all).
diff --git a/sha1_file.c b/sha1_file.c
index 760dd60..288badd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -807,15 +807,38 @@ void free_pack_by_name(const char *pack_name)
static unsigned int get_max_fd_limit(void)
{
#ifdef RLIMIT_NOFILE
- struct rlimit lim;
+ {
+ struct rlimit lim;
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
+ if (!getrlimit(RLIMIT_NOFILE, lim))
+ return lim.rlim_cur;
+ }
+#endif
Yeah, I think pulling the variable into its own block makes this more
readable.
+#ifdef _SC_OPEN_MAX
+ {
+ long open_max = sysconf(_SC_OPEN_MAX);
+ if (0 open_max)
+ return open_max;
+ /*
+ * Otherwise, we got -1 for one of the two
+ * reasons:
+ *
+ * (1) sysconf() did not understand _SC_OPEN_MAX
+ * and signaled an error with -1; or
+ * (2) sysconf() said there is no limit.
+ *
+ * We _could_ clear errno before calling sysconf() to
+ * tell these two cases apart and return a huge number
+ * in the latter case to let the caller cap it to a
+ * value that is not so selfish, but letting the
+ * fallback OPEN_MAX codepath take care of these cases
+ * is a lot simpler.
+ */
+ }
+#endif
This is probably OK. I assume sane systems actually provide OPEN_MAX,
and/or have a working getrlimit in the first place.
The fallback of 1 is actually quite low and can have an impact. Both
for performance, but also for concurrent use. We used to run into a
problem at GitHub where pack-objects serving a clone would have its
packfile removed from under it (by a concurrent repack), and then would
die. The normal code paths are able to just retry the object lookup and
find the new pack, but the pack-objects code is a bit more intimate with
the particular packfile and cannot (currently) do so. With a large
enough mmap window and descriptor limit, we just keep the packfiles
open. But if we have to close them for resource limits (like a too-low
descriptor limit), then we can end up in the die() situation above.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/3] diff: Tests for git diff -O
Heavily adapted from Anders' patch:
diff: Add diff.orderfile configuration variable
Signed-off-by: Anders Waldenborg and...@0x63.nu
Signed-off-by: Samuel Bronson naes...@gmail.com
---
t/t4056-diff-order.sh | 70 +++
1 file changed, 70 insertions(+)
create mode 100755 t/t4056-diff-order.sh
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..218f171
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='diff order'
+
+. ./test-lib.sh
+
+create_files () {
+ echo $1 a.h
+ echo $1 b.c
+ echo $1 c/Makefile
+ echo $1 d.txt
+ git add a.h b.c c/Makefile d.txt
+ git commit -m$1
+}
+
+test_expect_success 'setup' '
+ mkdir c
+ create_files 1
+ create_files 2
+
+ cat order_file_1 -\EOF
+ *Makefile
+ *.txt
+ *.h
+ EOF
+
+ cat order_file_2 -\EOF
+ *Makefile
+ *.h
+ *.c
+ EOF
+
+ cat expect_none -\EOF
+ a.h
+ b.c
+ c/Makefile
+ d.txt
+ EOF
+
+ cat expect_1 -\EOF
+ c/Makefile
+ d.txt
+ a.h
+ b.c
+ EOF
+
+ cat expect_2 -\EOF
+ c/Makefile
+ a.h
+ b.c
+ d.txt
+ EOF
+
+ true# end chain of
+'
+
+test_expect_success no order (=tree object order) '
+ git diff --name-only HEAD^..HEAD actual
+ test_cmp expect_none actual
+'
+
+for i in 1 2
+do
+ test_expect_success orderfile using option ($i) '
+ git diff -Oorder_file_$i --name-only HEAD^..HEAD actual
+ test_cmp expect_$i actual
+ '
+done
+
+test_done
--
1.8.4.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/3] diff: Add diff.orderfile configuration variable
diff.orderfile acts as a default for the -O command line option.
[sb: split up aw's original patch; rework tests and docs, treat option
as pathname]
Signed-off-by: Anders Waldenborg and...@0x63.nu
Signed-off-by: Samuel Bronson naes...@gmail.com
---
Documentation/diff-config.txt | 5 +
Documentation/diff-options.txt | 3 +++
diff.c | 5 +
t/t4056-diff-order.sh | 10 ++
4 files changed, 23 insertions(+)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 223b931..f07b451 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -98,6 +98,11 @@ diff.mnemonicprefix::
diff.noprefix::
If set, 'git diff' does not show any source or destination prefix.
+diff.orderfile::
+ File indicating how to order files within a diff, using
+ one shell glob pattern per line.
+ Can be overridden by the '-O' option to linkgit:git-diff[1].
+
diff.renameLimit::
The number of files to consider when performing the copy/rename
detection; equivalent to the 'git diff' option '-l'.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bbed2cd..9b37b2a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -432,6 +432,9 @@ endif::git-format-patch[]
-Oorderfile::
Output the patch in the order specified in the
orderfile, which has one shell glob pattern per line.
+ This overrides the `diff.orderfile` configuration variable
+ (see linkgit:git-config[1]). To cancel `diff.orderfile`,
+ use `-O/dev/null`.
ifndef::git-format-patch[]
-R::
diff --git a/diff.c b/diff.c
index b79432b..f35c83b 100644
--- a/diff.c
+++ b/diff.c
@@ -30,6 +30,7 @@ static int diff_use_color_default = -1;
static int diff_context_default = 3;
static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
+static const char *diff_order_file_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
static int diff_no_prefix;
@@ -201,6 +202,8 @@ int git_diff_ui_config(const char *var, const char *value,
void *cb)
return git_config_string(external_diff_cmd_cfg, var, value);
if (!strcmp(var, diff.wordregex))
return git_config_string(diff_word_regex_cfg, var, value);
+ if (!strcmp(var, diff.orderfile))
+ return git_config_pathname(diff_order_file_cfg, var, value);
if (!strcmp(var, diff.ignoresubmodules))
handle_ignore_submodules_arg(default_diff_options, value);
@@ -3207,6 +3210,8 @@ void diff_setup(struct diff_options *options)
options-detect_rename = diff_detect_rename_default;
options-xdl_opts |= diff_algorithm;
+ options-orderfile = diff_order_file_cfg;
+
if (diff_no_prefix) {
options-a_prefix = options-b_prefix = ;
} else if (!diff_mnemonic_prefix) {
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index 0ac1b95..acd7683 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -91,6 +91,16 @@ do
wait
test_cmp expect_$i actual
'
+
+ test_expect_success orderfile using config ($i) '
+ git -c diff.orderfile=order_file_$i diff --name-only
HEAD^..HEAD actual
+ test_cmp expect_$i actual
+ '
+
+ test_expect_success cancelling configured orderfile ($i) '
+ git -c diff.orderfile=order_file_$i diff -O/dev/null
--name-only HEAD^..HEAD actual
+ test_cmp expect_none actual
+ '
done
test_done
--
1.8.4.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/3] diff: Let git diff -O read orderfile from any file, fail properly
The -O flag really shouldn't silently fail to do anything when given a
path that it can't read from.
However, it should be able to read from un-mmappable files, such as:
* pipes/fifos
* /dev/null: It's a character device (at least on Linux)
* ANY empty file:
Quoting Linux mmap(2), SUSv3 specifies that mmap() should fail if
length is 0. However, in kernels before 2.6.12, mmap() succeeded in
this case: no mapping was created and the call returned addr. Since
kernel 2.6.12, mmap() fails with the error EINVAL for this case.
We especially want -O/dev/null to work, since we will be documenting
it as the way to cancel diff.orderfile when we add that.
(Note: -O/dev/null did have the right effect, since the existing error
handling essentially worked out to silently ignore the orderfile. But
this was probably more coincidence than anything else.)
So, lets toss all of that logic to get the file mmapped and just use
strbuf_read_file() instead, which gives us decent error handling
practically for free.
Signed-off-by: Samuel Bronson naes...@gmail.com
---
diffcore-order.c | 23 ---
t/t4056-diff-order.sh | 26 ++
2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/diffcore-order.c b/diffcore-order.c
index 23e9385..a63f332 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -10,28 +10,21 @@ static int order_cnt;
static void prepare_order(const char *orderfile)
{
- int fd, cnt, pass;
+ int cnt, pass;
+ struct strbuf sb = STRBUF_INIT;
void *map;
char *cp, *endp;
- struct stat st;
- size_t sz;
+ ssize_t sz;
if (order)
return;
- fd = open(orderfile, O_RDONLY);
- if (fd 0)
- return;
- if (fstat(fd, st)) {
- close(fd);
- return;
- }
- sz = xsize_t(st.st_size);
- map = mmap(NULL, sz, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
- close(fd);
- if (map == MAP_FAILED)
- return;
+ sz = strbuf_read_file(sb, orderfile, 0);
+ if (sz 0)
+ die_errno(_(failed to read orderfile '%s'), orderfile);
+ map = strbuf_detach(sb, NULL);
endp = (char *) map + sz;
+
for (pass = 0; pass 2; pass++) {
cnt = 0;
cp = map;
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index 218f171..0ac1b95 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -59,12 +59,38 @@ test_expect_success no order (=tree object order) '
test_cmp expect_none actual
'
+test_expect_success 'missing orderfile' '
+ rm -f bogus_file
+ test_must_fail git diff -Obogus_file --name-only HEAD^..HEAD
+'
+
+test_expect_success POSIXPERM,SANITY 'unreadable orderfile' '
+ unreadable_file
+ chmod -r unreadable_file
+ test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD
+'
+
+test_expect_success 'orderfile is a directory' '
+ test_must_fail git diff -O/ --name-only HEAD^..HEAD
+'
+
for i in 1 2
do
test_expect_success orderfile using option ($i) '
git diff -Oorder_file_$i --name-only HEAD^..HEAD actual
test_cmp expect_$i actual
'
+
+ test_expect_success PIPE orderfile is fifo ($i) '
+ rm -f order_fifo
+ mkfifo order_fifo
+ {
+ cat order_file_$i order_fifo
+ }
+ git diff -O order_fifo --name-only HEAD^..HEAD actual
+ wait
+ test_cmp expect_$i actual
+ '
done
test_done
--
1.8.4.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/3] diff: Add diff.orderfile configuration variable
I expect you've figured out what this patch series is about by now. In this version, I've applied Junio's suggestions from the last version, and also the stuff from the FIXUP commit he made after my stuff in the branch he merged into 'pu'. Samuel Bronson (3): diff: Tests for git diff -O diff: Let git diff -O read orderfile from any file, fail properly diff: Add diff.orderfile configuration variable Documentation/diff-config.txt | 5 ++ Documentation/diff-options.txt | 3 ++ diff.c | 5 ++ diffcore-order.c | 23 - t/t4056-diff-order.sh | 106 + 5 files changed, 127 insertions(+), 15 deletions(-) create mode 100755 t/t4056-diff-order.sh -- 1.8.4.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] prune-packed: fix a possible buffer overflow
On Wed, Dec 18, 2013 at 1:43 AM, Junio C Hamano gits...@pobox.com wrote:
Duy Nguyen pclo...@gmail.com writes:
Why don't we take this opportunity to replace that array with a
strbuf? The conversion looks simple with this function.
Indeed. Something like this, perhaps?
Yes, looking good.
void prune_packed_objects(int opts)
{
int i;
- static char pathname[PATH_MAX];
const char *dir = get_object_directory();
- int len = strlen(dir);
+ struct strbuf pathname = STRBUF_INIT;
+ int top_len;
+ strbuf_addstr(pathname, dir);
if (opts PRUNE_PACKED_VERBOSE)
progress = start_progress_delay(Removing duplicate objects,
256, 95, 2);
- if (len PATH_MAX - 42)
- die(impossible object directory);
- memcpy(pathname, dir, len);
- if (len pathname[len-1] != '/')
- pathname[len++] = '/';
+ if (pathname.len pathname.buf[pathname.len - 1] != '/')
+ strbuf_addch(pathname, '/');
I see this pattern (add a trailing slash) in a few places too. Maybe
we could make a wrapper for it.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/3] diff: Add diff.orderfile configuration variable
Looks good; will replace and merge to 'next', but not today (I am already deep into today's integration cycle). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
On Thu, Dec 19, 2013 at 3:57 AM, Junio C Hamano gits...@pobox.com wrote:
Duy Nguyen pclo...@gmail.com writes:
On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@gmail.com wrote:
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349).
On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
This was discussed on the Git user list recently [1].
#in a repo with no files
git add -A
fatal: pathspec '.' did not match any files
The same goes for git add . (and -u).
Whereas I think some warning feedback is useful, we are curious
whether this is an intentional change or not.
I was not aware of this case when I made the change. It's caused by
this change that removes pathspec.raw[i][0] check in builtin/add.c in
84b8b5d .
- for (i = 0; pathspec.raw[i]; i++) {
- if (!seen[i] pathspec.raw[i][0]
-!file_exists(pathspec.raw[i])) {
+ for (i = 0; i pathspec.nr; i++) {
+ const char *path = pathspec.items[i].match;
+ if (!seen[i] !file_exists(path)) {
Isn't that pathspec.raw[i][0] check merely an attempt to work around
the combination of
(1) the current directory pathspec . is sanitized down to an
empty string by the pathspec code; and
(2) even though file_exists() is willing to say yes to a non-file
(namely, a directory), it is not prepared to take an empty
string resulting from (1) to mean the directory ..
Yeah, and it was added so intentionally in 07d7bed (add: don't
complain when adding empty project root - 2009-04-28). So this is a
regression.
Adding it back requires some thinking because path in the new code
could be something magic..
Ehh, why? Shouldn't something magic that did _not_ match
(i.e. not in seen[]) diagnosed as such?
I am wondering why we even need !file_exists(path) check there in
the first place. We run fill_directory() and then let
prune_directory() report which pathspec did not have any match via
the seen[] array. We also match pathspec against the index to see
if there are pathspec that does not match anything. So at that
point of the codeflow, we ought to be able to make sure that seen[]
is the _only_ thing we need to consult to see if there are any
pathspec elements that did not match.
See e96980e (builtin-add: simplify (and increase accuracy of) exclude
handling - 2007-06-12). It has something to do with directory check
originally, then we don't care about S_ISDIR() any more and turn it to
file_exists(). Maybe it's safe to remove it now. Need to check
fill_directory() again..
Stepping back even further, I wonder if this yes, I found a
matching entity and know this is not an end-user typo bit actually
should be _in_ struct pathspec. Traditionally we implemented that
bit as a separate seen[] array parallel to const char **pathspec
array, but that was merely because we only had the list of strings.
Now we express a pathspec as a list of struct pathspec elements,
I think seen[] can and should become part of the pathspec. Am I
missing something?
Yes it probably better belongs to struct pathspec. Turning it into 1
flag would simplify seen[] memory management too.
and the new behavior makes sense, so I'm
inclined to keep it as is, unless people have other opinions.
[1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
On Thu, Dec 19, 2013 at 12:44 AM, Michael Haggerty mhag...@alum.mit.edu wrote: A correct incremental converter could be done (as long as the CVS users don't literally change history retroactively) but it would be a lot of work. Although I agree with that sentence as it is stated, I also believe that the parenthesized condition rules out a _majority_ of CVS repo of non-trivial size/history. So even though a correct incremental converter could be built, it would be pretty much useless if it did not gracefully handle rewritten history. And in the face of rewritten history it becomes pretty much impossible to define what a correct conversion should even look like (not to mention the difficulty of actually implementing that converter...). Here are just a couple of things a CVS user can do (and that happened fairly regularly at my previous $dayjob) that would make life difficult for an incremental converter (and that also makes stable output from a non-incremental converter hard to solve in practice): - A user deletes $file from $branch by simply removing the $branch symbol on $file (cvs tag -B -d $branch $file). CVS stores no record of this. Many non-incremental importers will see $file as never having existed on $branch. An incremental importer starting from a previously converted state, must somehow deal with that previous state no longer existing from the POV of CVS. - A user moves a release tag on a few files to include a late bugfix into an upcoming release (cvs tag -F -r $new_rev $tag $file). There might be no single point in time where the tagged state existed in the repo, it has become a Frankentag. You could claim user error here, and that such shortcuts should not happen, but that doesn't really prevent it from ever happening. Recreating the tree state of the Frankentag in Git is easy, but what kind of history do you construct to lead up to that tree? - A modularized project develops code on HEAD, and make regular releases of each module by tagging the files in the module dir with $modulename-$version. Afterwards a project-wide stable tag is moved on that subset of files to include the new module release into the stable tag. (stable is conceptually a branch, but the CVS mechanism used here is still the tag, since CVS branches cannot follow eachother like in Git). This is pretty much the same Frankentag scenario as above, except that in this case it might be considered Best Practice (it was at our $dayjob), and not a shortcut/user error made by a single user. (None of these examples even involve the cvs admin which allows you to do some truly scary and demented things to your CVS history...) My point here is that people will use whatever available tools they have to solve whatever problems they are currently having. And when CVS is your tool, you will sooner or later end up with a solution that irrevocably rewrites your CVS history. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()
On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano gits...@pobox.com wrote: Tom Miller jacker...@gmail.com writes: In order to fix branchname DF conflicts during `fetch --prune`, the way the header is output to the screen needs to be refactored. Here is an exmaple of the output with the line in question denoted by '': $ git fetch --prune --dry-run upstream From https://github.com/git/git a155a5f..5512ac5 maint - upstream/maint d7aced9..7794a68 master - upstream/master 523f7c4..3e57c29 next - upstream/next + 462f102...0937cdf pu - upstream/pu (forced update) e24105a..5d352bc todo - upstream/todo * [new tag] v1.8.5.2 - v1.8.5.2 * [new tag] v1.8.5.2 - v1.8.5.2 pretty_url(): This function when passed a transport url will anonymize the transport of the url. It will strip a trailing '/'. It will also strip a trailing '.git'. It will return the newly formated url for use. I do not believe there is a need for stripping the trailing '/' and '.git' from a url, but it was already there and I wanted to make as little changes as possible. OK. I tend to agree that stripping the trailing part is probably not a good idea and we would want to remove that but that definitely should be done as a separate step, or even as a separate series on top of this one. I think that removing the trailing part will greatly reduce the complexity to the point were it is unnecessary to have pretty_url(). My goal with extracting this function is to isolate the complexity of formatting the url to a single spot. I am thinking along the lines of the following commit order: 1. Remove the remove trailing part 2. Add print_url() 3. Always print url when pruning 4. Reverse order of prune and fetch print_url(): This function will convert a transport url to a pretty url using pretty_url(). Then it will print out the pretty url to stderr as indicated above in the example output. It uses a global variable named gshown_url' to prevent this header for being printed twice. Gaah. What is that 'g' doing there? Please don't do that meaningless naming. I am not familiar with C conventions and I was trying to stay consistent. I saw other global variables starting with 'g' and made an assumption. It will use the original name in the upcoming patches. I do not think the change to introduce such a global variable belongs to this refactoring step. The current caller can decide itself if it called that function, and if you are going to introduce new callers in later steps, they can coordinate among themselves, no? I agree, there is no reason for introducing it in this step. Thanks for pointing that out. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts
On Wed, Dec 18, 2013 at 3:54 PM, Junio C Hamano gits...@pobox.com wrote:
Tom Miller jacker...@gmail.com writes:
When a branchname DF conflict occurs during a fetch,
You may have started with a specific case in which you want to
change the behaviour of current Git, so it may be clear what you
meant by branchname DF conflict, but that is true for nobody other
than you who will read this log message. Introducing new lingo is
OK as long as it is necessary, but in a case like this, where you
have to describe what situation you are trying to address anyway,
I do not think you need to add a new word to our vocabulary.
When we have a remote-tracking branch frotz/nitfol from a
previous fetch, and the upstream now has branch frotz, we
used to fail to remove frotz/nitfol and recreate frotz with
git fetch --prune from the upstream.
or something like that?
I did not intend to introduce new lingo. I did some searching through
history to see if something like this had been worked on before and
I found a commit by Jeff King that introduced me the the idea of
DF conflicts
commit fa250759794ab98e6edfbbf2f6aa2cb912e535eb
Author: Jeff King p...@peff.net
Date: Mon May 25 06:40:54 2009 -0400
fetch: report ref storage DF errors more accurately
When we fail to store a fetched ref, we recommend that the
user try running git prune to remove up any old refs that
have been deleted by the remote, which would clear up any DF
conflicts. However, ref storage might fail for other
reasons (e.g., permissions problems) in which case the
advice is useless and misleading.
This patch detects when there is an actual DF situation and
only issues the advice when one is found.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
I have no issue with rewording the it to be more clear and to try to
remove any new lingo.
But what should happen when we do not give --prune to git fetch in
such a situation? Should it fail, because we still have frotz/nitfol
and we cannot create frotz without losing it?
You talk about this to some extent in an email from 2009. I have linked
it below for your review.
http://article.gmane.org/gmane.comp.version-control.git/132276
In my opinion, if I supply --prune to fetch I expect it to be
destructive. It should be noted that the reflog can *not* be used
to recover pruned branches from a remote.
--prune should
be able to fix it. When fetching with --prune, the fetching process
happens before pruning causing the branchname DF conflict to persist
and report an error. This patch prunes before fetching, thus
correcting DF conflicts during a fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
I wasn't following previous threads closely (was there a previous
thread???); has this iteration been already tested by trast?
There was a previous thread, but I was just looking for feed back on this
as a WIP. Should I have replied to it with this patchset?
Here is a link to the previous thread.
http://thread.gmane.org/gmane.comp.version-control.git/238530
The commit below should be the same patch he tested. The test was added
by him, and I made it part of this commit. Did I do this wrong?
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e50b697..845c687 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded'
'
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack
3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+
Re: [PATCH] log: properly handle decorations with chained tags
On Tue, Dec 17, 2013 at 04:36:06PM -0800, Junio C Hamano wrote: I think all we need to do, in addition to what the existing code does, is to make sure that we _parse_ the object that the tag points at, to avoid this problem. Something like this, perhaps, instead? Yeah, that's the clean fix I was looking for, but couldn't quite come up with. I'm going to re-roll with your fix instead of mine and my tests. Any objections to adding your sign-off? -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: I have end-of-lifed cvsps
Michael Haggerty mhag...@alum.mit.edu: If you haven't tried cvs2git yet, please start it up somewhere in the background. It might take a while but it should have no trouble with your repos, and then you can compare the tools based on experience rather than speculation. That would be a good thing. Michael, in case you're wondering why I've continued to work on cvs-fast-export when cvs2git exists, there are exactly two reasons: (a) it's a whole lot faster on repos that aren't large enough to demand multipass, and (b) the single-whole-dumpfile output makes it a better reposurgeon front end. But the traffic on the cvs2svn/cvs2git mailing list has trailed off essentially to zero, so either the software is perfect already (haha) or most everybody has already converted. Therefore I don't invest any significant time in that project these days. Reasonable. I'm doing this as a temporary break from working on GPSD. I don't expect to be investing a lot of time in it after I get it to a 1.0 state. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts
Tom Miller jacker...@gmail.com writes:
The commit below should be the same patch he tested. The test was added
by him, and I made it part of this commit. Did I do this wrong?
No, no, no. All my questions were true questions, not complaints
veiled as rhetorical questions. Thanks for many pointers for
clarification.
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e50b697..845c687 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are
excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack
}.pack 3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitk: Fix typo in proc blobdiffmaybeseehere
From: Johannes Sixt j...@kdbg.org
The recent 5de460a2 (Refactor per-line part of getblobdiffline and its
support) introduced blobdiffmaybeseehere, and accidentally forgot the '$'
to access the parameter as a TCL variable. This resulted in a failing
Back button with the error
can't use non-numeric string as operand of !
while executing
if {!$ateof} {
set nlines [expr {[winfo height $ctext]
/ [font metrics textfont -linespace]}]
if {[$ctext compare $target_scrollpos + $nlines ...
(procedure maybe_scroll_ctext line 5)
Signed-off-by: Johannes Sixt j...@kdbg.org
---
Am 12/16/2013 14:56, schrieb Johannes Sixt:
To reproduce, start gitk in any repository, click a commit, then the
back button (left-pointing arrow button) or type Alt+Cursor-Left. The
error I get is this:
can't use non-numeric string as operand of !
It turns out to be just a simple typo.
-- Hannes
gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk b/gitk
index 33c3a6c..1f14796 100755
--- a/gitk
+++ b/gitk
@@ -7922,7 +7922,7 @@ proc blobdiffmaybeseehere {ateof} {
if {$diffseehere = 0} {
mark_ctext_line [lindex [split $diffseehere .] 0]
}
-maybe_scroll_ctext ateof
+maybe_scroll_ctext $ateof
}
proc getblobdiffline {bdf ids} {
--
1.8.5.1.1587.g3845a3d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
