Re: What's cooking in git.git (Sep 2013, #03; Wed, 11)
Johannes Sixt j.s...@viscovery.net writes: Am 9/12/2013 17:24, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: Am 9/12/2013 1:32, schrieb Junio C Hamano: * jc/ref-excludes (2013-09-03) 2 commits Thanks for a dose of sanity. I didn't look at rev-parse. I vaguely recall somebody offered follow-ups (was it you?) and at that point I placed this on the back-burner. Yes, I offered to pick up the topic as time permits. Don't hold your breath, though :-) 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] send-email: don't call methods on undefined values
brian m. carlson sand...@crustytoothpaste.net writes:
On Mon, Sep 09, 2013 at 09:45:10AM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion
if ($smtp-code == 220) {
$smtp = Net::SMTP::SSL-start_SSL($smtp,
ssl_verify_params())
- or die STARTTLS failed!
.$smtp-message;
+ or die STARTTLS failed!
.IO::Socket::SSL::errstr();
I agree that $smtp-message may be bogus at this point, but could
require IO::Socket::SSL have failed on us in ssl_verify_params?
In that degraded mode, we do not do SSL peer verification, but
otherwise we would still attempt to talk with the peer, and in such
a case, IO::Socket::SSL would not be available to us at this point,
no?
Since Net::SMTP::SSL uses IO::Socket::SSL (in fact, it is an
IO::Socket::SSL), we can be guaranteed that it is, in fact, available at
this point. I guess strictly we don't need that require in
IO::Socket::SSL since we'll already be guaranteed that it exists by the
require of Net::SMTP::SSL.
That require came from around $gmane/230533, which was about
making sure the update to pacify newer Net::SMTP::SSL does not break
folks with older packages, I think.
I just didn't/don't know the history of Net::SMTP::SSL, and that was
where my comments came from.
As long as Net::SMTP::SSL uses IO::Socket::SSL has been true since
the ancient past, I agree that that 'require' of the latter does not
need to be guarded by an 'eval'; at that point, we are already in
the Net::SMTP::SSL codepath. And your patch I replied to should be
the right thing to do.
Thanks for clarifying.
--
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
Removing all notes containing a specific string
Hello, I'd like to know if that's possible to parse all notes to detect a special string and if it's the case, remove the note like git-notes remove would do. Thanks -- Francis -- 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: Removing all notes containing a specific string
On Fri, Sep 13, 2013 at 8:51 AM, Francis Moreau francis.m...@gmail.com wrote: Hello, I'd like to know if that's possible to parse all notes to detect a special string and if it's the case, remove the note like git-notes remove would do. Hi, There's no built-in command/option to do this, but the following shell one-liner should do the job: git grep -l $mystring refs/notes/commits | cut -d':' -f2 | tr -d '/' | xargs git notes remove ...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 v2 2/4] pathspec: strip multiple trailing slashes from submodules
On Fri, Sep 13, 2013 at 08:28:24AM +0700, Duy Nguyen wrote: On Fri, Sep 13, 2013 at 3:21 AM, John Keeping j...@keeping.me.uk wrote: On Thu, Sep 12, 2013 at 12:48:10PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: This allows us to replace the submodule path trailing slash removal in builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to parse_pathspec() without changing the behaviour with respect to multiple trailing slashes. Where does prefix_pathspec()'s input, which could have an unwanted trailing slash, come from? If it is read from some of our internal data structure and known to have at most one, then this change makes me feel very uneasy to cope with potentially sloppy end-user input and data generated by ourselves with the same logic. It will allow our internal to be sloppy without forcing us notice and fix that sloppiness. If it is coming from an end-user input, then I would not object to the change, though. I added this in response to Duy's comment on v1 [1]. [1] http://article.gmane.org/gmane.comp.version-control.git/234548 Looks like I add more noise to this thread than anything valuable. Yes, once argv goes through parse_pathspec it's normalized so we do not need to strip consecutive slashes any more. I'm not entirely sure if it also converts Windows '\\' to '/'.. If it goes through normalize_path_copy_len then it does. Junio, can you please drop the first two patches in this series? I can resend the final two unmodified if necessary, but I suspect it's easier for you to just drop the commits. Looking more closely, this does come from user input (via the argv passed into parse_pathspec) but does (some of the time) go through prefix_path_gently which calls normalize_path_copy_len. It's not immediately clear to me when prefix_pathspec goes through this particular code path, but I think we may be able to drop this (and the previous patch) without affecting the user. -- 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
Your registration code (E3O6M6Y)
Your registration code (E3O6M6Y) This is to update you regarding your CARD ATM of US$1.8Million Contact the Atm Director with your delivery address for more information via their details below. MD. Ms. Kieffer Robert Email:(dhlcom_p...@yahoo.fr) Tel:+229 98298252 Mr. Ahamad Yaki -- 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: Converting repo from HG, `git filter-branch --prune-empty -- --all` is extremely slow and errors out.
On Thu, Sep 12, 2013 at 7:47 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Indeed, I remember writing my own simplified version of 'git filter-branch' that was much faster. If I recall correctly, the trick was avoiding 'git write-tree' which can be done if you are not using any tree filter, but 'git filter-branch' is not that smart. If all you want to do is prune empty commits, it should be easy to write a script that simply does 'git commit-tree'. I might decide to do that based on my script if I have time today. Here it is, it's straightforward and should be easy to understand. -- Felipe Contreras filter-branch Description: Binary data
Re: Re-Transmission of blobs?
On Thu, Sep 12, 2013 at 03:44:53PM -0400, Jeff King wrote: On Thu, Sep 12, 2013 at 12:35:32PM +0200, Josef Wolf wrote: I'm not sure I understand correctly. I see that bitmaps can be used to implement set operations. But how comes that walking the graph requires a lot of CPU? Isn't it O(n)? Yes and no. Your n there is the entirety of history. Is this really true? (and each one needs to be pulled off of the disk, decompressed, and reconstructed from deltas). While you need to unpack commits/trees to traverse further down, I can't see any reason to unpack/reconstruct blobs just to see whether you need to send it. The SHA is all you need to know, isn't it? Secondly, the graph traversal ends up seeing the same sha1s over and over again in tree entries (because most entries in the tree don't change from commit to commit). Whenever you see an object (whether commit or tree) that you already have seen, you can stop traversing further down this part of the graph/tree, as everything you will see on this part has already be seen before. Why would you see the same commits/trees over and over again? You'd stop traversing on the boundary of the already-seen-territory, leaving the vast majority of the duplicated structure under the carpet. Somehow I fail to see the problem here. -- Josef Wolf j...@raven.inka.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
Re: Re-Transmission of blobs?
On Thu, Sep 12, 2013 at 08:06:35PM +, Pyeron, Jason J CTR (US) wrote: Yes, but it is those awfully slow connections (slower that the looping issue) which happen to always drop while cloning from our office. And the round trip should be mitigated by http-keep-alives. [ ... ] But, again if the connection drops, we have already lost the delta advantage. I would think the scenario would go like this: git clone url://blah/blah [fail] cd blah git clone --resume #uses normal methods [fail] while ! git clone --resume --HitItWithAStick replace clone with fetch for that use case too Last time I checked, cloning could not be resumed: http://git.661346.n2.nabble.com/git-clone-and-unreliable-links-td7570652.html If you're on a slow/unreliable link, you've lost. :-( :-( :-( -- Josef Wolf j...@raven.inka.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
Re: [PATCH v2 1/2] version-gen: cleanup
Felipe Contreras felipe.contre...@gmail.com writes:
+describe () {
+ VN=$(git describe --match v[0-9]* --abbrev=7 HEAD 2/dev/null) ||
return 1
case $VN in
+ *$LF*)
+ return 1
+ ;;
v[0-9]*)
git update-index -q --refresh
test -z $(git diff-index --name-only HEAD --) ||
+ VN=$VN-dirty
+ return 0
+ ;;
esac
+}
+
+# First see if there is a version file (included in release tarballs),
+# then try 'git describe', then default.
+if test -f version
+then
+ VN=$(cat version) || VN=$DEF_VER
+elif test -d ${GIT_DIR:-.git} -o -f .git describe
then
Makes sense; using a helper function makes the primary logic easier
to read.
-test $VN = $VC || {
- echo 2 GIT_VERSION = $VN
- echo GIT_VERSION = $VN $GVF
-}
-
-
+test $VN = $VC exit
+echo 2 GIT_VERSION = $VN
+echo GIT_VERSION = $VN $GVF
The point of this part is if the version file is already up to
date, do not rewrite it only to smudge the mtime to confuse make,
so it would be much easier to read to write it like so:
if test $VN != $VC
then
... two echoes ...
fi
compared to exiting in the middle which is harder to follow,
especially if we consider that we may have to grow the remaining two
lines in the future.
--
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 0/3] stop storing trailing slash in dir-hash
This series alters name-hash so that it no longer stores the (redundant) trailing slash with index_state.dir_hash entries. As an intentional side-effect, the series fixes [1] in a cleaner way (suggested by Junio [2]) than either [3] (680be044 in master) or [4]. As noted by Peff [5], this change is at a fairly fundamental level, so care has been taken to ensure that all tests still pass (thus it at least does not break anything covered by the tests). [1]: http://thread.gmane.org/gmane.comp.version-control.git/232727 [2]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232813 [3]: http://thread.gmane.org/gmane.comp.version-control.git/232796 [4]: http://thread.gmane.org/gmane.comp.version-control.git/232833 [5]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232822 Eric Sunshine (3): name-hash: refactor polymorphic index_name_exists() name-hash: stop storing trailing '/' on paths in index_state.dir_hash dir: revert work-around for retired dangerous behavior cache.h | 2 ++ dir.c| 25 +++-- name-hash.c | 54 -- read-cache.c | 2 +- 4 files changed, 38 insertions(+), 45 deletions(-) -- 1.8.4.457.g424cb08 -- 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] name-hash: refactor polymorphic index_name_exists()
Depending upon the absence or presence of a trailing '/' on the incoming
pathname, index_name_exists() checks either if a file is present in the
index or if a directory is represented within the index. Each caller
explicitly chooses the mode of operation by adding or removing a
trailing '/' before invoking index_name_exists().
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
Splitting index searching into two functions relieves callers of the
artificial burden of having to add or remove a slash to select the mode
of operation; instead they just call the desired function. A subsequent
patch will take advantage of this benefit in order to eliminate the
requirement that the incoming pathname for a directory search must have
a trailing slash.
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
cache.h | 2 ++
dir.c| 7 ---
name-hash.c | 47 ---
read-cache.c | 2 +-
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/cache.h b/cache.h
index 9ef778a..03ec24c 100644
--- a/cache.h
+++ b/cache.h
@@ -314,6 +314,7 @@ extern void free_name_hash(struct index_state *istate);
#define refresh_cache(flags) refresh_index(the_index, (flags), NULL, NULL,
NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(the_index, (ce), (st),
(options))
#define ce_modified(ce, st, options) ie_modified(the_index, (ce), (st),
(options))
+#define cache_dir_exists(name, namelen) index_dir_exists(the_index, (name),
(namelen))
#define cache_name_exists(name, namelen, igncase)
index_name_exists(the_index, (name), (namelen), (igncase))
#define cache_name_is_other(name, namelen) index_name_is_other(the_index,
(name), (namelen))
#define resolve_undo_clear() resolve_undo_clear_index(the_index)
@@ -463,6 +464,7 @@ extern int write_index(struct index_state *, int newfd);
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
+extern struct cache_entry *index_dir_exists(struct index_state *istate, const
char *name, int namelen);
extern struct cache_entry *index_name_exists(struct index_state *istate, const
char *name, int namelen, int igncase);
extern int index_name_pos(const struct index_state *, const char *name, int
namelen);
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
diff --git a/dir.c b/dir.c
index b439ff0..0080673 100644
--- a/dir.c
+++ b/dir.c
@@ -860,7 +860,8 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if ((len == 0 || pathname[len - 1] != '/')
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir-entries, dir-nr+1, dir-alloc);
@@ -885,11 +886,11 @@ enum exist_status {
/*
* Do not use the alphabetically sorted index to look up
* the directory name; instead, use the case insensitive
- * name hash.
+ * directory hash.
*/
static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
{
- const struct cache_entry *ce = cache_name_exists(dirname, len + 1,
ignore_case);
+ const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
unsigned char endchar;
if (!ce)
diff --git a/name-hash.c b/name-hash.c
index 617c86c..5b01554 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -222,11 +222,35 @@ static int same_name(const struct cache_entry *ce, const
char *name, int namelen
return slow_same_name(name, namelen, ce-name, len);
}
+struct cache_entry *index_dir_exists(struct index_state *istate, const char
*name, int namelen)
+{
+ struct cache_entry *ce;
+ struct dir_entry *dir;
+
+ lazy_init_name_hash(istate);
+ dir = find_dir_entry(istate, name, namelen);
+ if (dir dir-nr)
+ return dir-ce;
+
+ /*
+* It might be a submodule. Unlike plain directories, which are stored
+* in the dir-hash, submodules are stored in the name-hash, so check
+* there, as well.
+*/
+ ce = index_name_exists(istate, name, namelen - 1, 1);
+ if (ce S_ISGITLINK(ce-ce_mode))
+ return ce;
+
+ return NULL;
+}
+
struct cache_entry *index_name_exists(struct index_state *istate, const char
*name, int namelen, int icase)
{
unsigned int hash = hash_name(name, namelen);
struct cache_entry *ce;
+ assert(namelen == 0 || name[namelen - 1] != '/');
+
lazy_init_name_hash(istate);
ce = lookup_hash(hash, istate-name_hash);
@@ -237,29 +261,6 @@ struct cache_entry *index_name_exists(struct index_state
[PATCH 2/3] name-hash: stop storing trailing '/' on paths in index_state.dir_hash
When 5102c617 (Add case insensitivity support for directories when using
git status, 2010-10-03) added directories to the name-hash there was
only a single hash table in which both real cache entries and leading
directory prefixes were registered. To distinguish between the two types
of entries, directories were stored with a trailing '/'.
2092678c (name-hash.c: fix endless loop with core.ignorecase=true,
2013-02-28), however, moved directories to a separate hash table
(index_state.dir_hash) but retained the now-redundant trailing '/', thus
callers still bear the burden of ensuring its presence before searching
via index_dir_exists(). Eliminate this redundancy by storing paths in
the dir-hash without the trailing '/'.
An important benefit of this change is that it eliminates undocumented
and dangerous behavior of dir.c:directory_exists_in_index_icase() in
which it assumes not only that it can validly access one character
beyond the end of its incoming directory argument, but also that that
character will unconditionally be a '/'. This perilous behavior was
tolerated because the string passed in by its lone caller always had a
'/' in that position, however, things broke [1] when 2eac2a4c (ls-files
-k: a directory only can be killed if the index has a non-directory,
2013-08-15) added a new caller which failed to respect the undocumented
assumption.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/232727
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
dir.c| 2 +-
name-hash.c | 9 +
read-cache.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 0080673..69d04a0 100644
--- a/dir.c
+++ b/dir.c
@@ -890,7 +890,7 @@ enum exist_status {
*/
static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
{
- const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
+ const struct cache_entry *ce = cache_dir_exists(dirname, len);
unsigned char endchar;
if (!ce)
diff --git a/name-hash.c b/name-hash.c
index 5b01554..2bae75d 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -58,9 +58,9 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
{
/*
* Throw each directory component in the hash for quick lookup
-* during a git status. Directory components are stored with their
+* during a git status. Directory components are stored without their
* closing slash. Despite submodules being a directory, they never
-* reach this point, because they are stored without a closing slash
+* reach this point, because they are stored
* in index_state.name_hash (as ordinary cache_entries).
*
* Note that the cache_entry stored with the dir_entry merely
@@ -78,6 +78,7 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
namelen--;
if (namelen = 0)
return NULL;
+ namelen--;
/* lookup existing entry for that directory */
dir = find_dir_entry(istate, ce-name, namelen);
@@ -97,7 +98,7 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
}
/* recursively add missing parent directories */
- dir-parent = hash_dir_entry(istate, ce, namelen - 1);
+ dir-parent = hash_dir_entry(istate, ce, namelen);
}
return dir;
}
@@ -237,7 +238,7 @@ struct cache_entry *index_dir_exists(struct index_state
*istate, const char *nam
* in the dir-hash, submodules are stored in the name-hash, so check
* there, as well.
*/
- ce = index_name_exists(istate, name, namelen - 1, 1);
+ ce = index_name_exists(istate, name, namelen, 1);
if (ce S_ISGITLINK(ce-ce_mode))
return ce;
diff --git a/read-cache.c b/read-cache.c
index a59644a..8990b61 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -643,7 +643,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
if (*ptr == '/') {
struct cache_entry *foundce;
++ptr;
- foundce = index_dir_exists(istate, ce-name,
ptr - ce-name);
+ foundce = index_dir_exists(istate, ce-name,
ptr - ce-name - 1);
if (foundce) {
memcpy((void *)startPtr, foundce-name
+ (startPtr - ce-name), ptr - startPtr);
startPtr = ptr;
--
1.8.4.457.g424cb08
--
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 3/3] dir: revert work-around for retired dangerous behavior
directory_exists_in_index_icase() dangerously assumed that it could
access one character beyond the end of its directory argument, and that
that character would unconditionally be '/'. 2eac2a4c (ls-files -k: a
directory only can be killed if the index has a non-directory,
2013-08-15) added a caller which did not respect this undocumented
assumption, and 680be044 (dir.c::test_one_path(): work around
directory_exists_in_index_icase() breakage, 2013-08-23) added a
work-around which temporarily appends a '/' before invoking
directory_exists_in_index_icase().
Since the dangerous behavior of directory_exists_in_index_icase() has
been eliminated, the work-around is now redundant, so retire it (but not
the tests added by the same commit).
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
dir.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index 69d04a0..ef95160 100644
--- a/dir.c
+++ b/dir.c
@@ -1161,21 +1161,9 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
*/
if ((dir-flags DIR_COLLECT_KILLED_ONLY)
(dtype == DT_DIR)
- !has_path_in_index) {
- /*
-* NEEDSWORK: directory_exists_in_index_icase()
-* assumes that one byte past the given path is
-* readable and has '/', which needs to be fixed, but
-* until then, work it around in the caller.
-*/
- strbuf_addch(path, '/');
- if (directory_exists_in_index(path-buf, path-len - 1) ==
- index_nonexistent) {
- strbuf_setlen(path, path-len - 1);
- return path_none;
- }
- strbuf_setlen(path, path-len - 1);
- }
+ !has_path_in_index
+ (directory_exists_in_index(path-buf, path-len) ==
index_nonexistent))
+ return path_none;
exclude = is_excluded(dir, path-buf, dtype);
--
1.8.4.457.g424cb08
--
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: Re-Transmission of blobs?
-Original Message- From: Josef Wolf Sent: Friday, September 13, 2013 6:23 On Thu, Sep 12, 2013 at 08:06:35PM +, Pyeron, Jason J CTR (US) wrote: Yes, but it is those awfully slow connections (slower that the looping issue) which happen to always drop while cloning from our office. And the round trip should be mitigated by http-keep-alives. [ ... ] But, again if the connection drops, we have already lost the delta advantage. I would think the scenario would go like this: git clone url://blah/blah [fail] cd blah git clone --resume #uses normal methods I am using the mythical --resume, where it would fetch packs and indexes. [fail] while ! git clone --resume --HitItWithAStick replace clone with fetch for that use case too Last time I checked, cloning could not be resumed: http://git.661346.n2.nabble.com/git-clone-and-unreliable-links-td7570652.html If you're on a slow/unreliable link, you've lost. That is kind of the point. It should be possible. -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- 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 v2 1/2] version-gen: cleanup
On Fri, Sep 13, 2013 at 1:10 AM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
+describe () {
+ VN=$(git describe --match v[0-9]* --abbrev=7 HEAD 2/dev/null) ||
return 1
case $VN in
+ *$LF*)
+ return 1
+ ;;
v[0-9]*)
git update-index -q --refresh
test -z $(git diff-index --name-only HEAD --) ||
+ VN=$VN-dirty
+ return 0
+ ;;
esac
+}
+
+# First see if there is a version file (included in release tarballs),
+# then try 'git describe', then default.
+if test -f version
+then
+ VN=$(cat version) || VN=$DEF_VER
+elif test -d ${GIT_DIR:-.git} -o -f .git describe
then
Makes sense; using a helper function makes the primary logic easier
to read.
-test $VN = $VC || {
- echo 2 GIT_VERSION = $VN
- echo GIT_VERSION = $VN $GVF
-}
-
-
+test $VN = $VC exit
+echo 2 GIT_VERSION = $VN
+echo GIT_VERSION = $VN $GVF
The point of this part is if the version file is already up to
date, do not rewrite it only to smudge the mtime to confuse make,
so it would be much easier to read to write it like so:
if test $VN != $VC
then
... two echoes ...
fi
compared to exiting in the middle which is harder to follow,
especially if we consider that we may have to grow the remaining two
lines in the future.
No, it's much easier to follow, the code clearly says if the version
is up to date, there's nothing else to do.
--
Felipe Contreras
--
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: Re-Transmission of blobs?
On Fri, Sep 13, 2013 at 3:06 AM, Pyeron, Jason J CTR (US) jason.j.pyeron@mail.mil wrote: But, again if the connection drops, we have already lost the delta advantage. I would think the scenario would go like this: git clone url://blah/blah [fail] cd blah git clone --resume #uses normal methods [fail] while ! git clone --resume --HitItWithAStick replace clone with fetch for that use case too Sorry if I missed something in this thread. But I think we could stablize the transferred pack so that --resume works. The sender constructs exactly the same pack as in the first git clone then it starts sending from the offset given by the client. For that to work, the first git clone must also be git clone --resume. I started working on that but now my focus is pack v4, so that has to wait. -- 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] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote: I'm not too happy with the wording either. As I see it, even on MinGW runtime version 4.0 it's not true that string.h has _only_ inline definition of strcasecmp; there's also #define strncasecmp _strnicmp which effectively provides a non-inline definition of strncasecmp aka _strnicmp. I do not get this part. Sure, string.h would have definitions of things other than strcasecmp, such as strncasecmp. So what? Sorry, I mixed up strcasecmp and strncasecmp. Does it effectively provide a non-inline definition of strcasecmp? Yes, if __NO_INLINE__ is defined string.h provides non-inline definition of both strcasecmp and strncasecmp by defining them to _stricmp and _strnicmp respectively. Perhaps the real issue is that the header file does not give an equivalent those who want to take the address of strcasecmp will get the address of _stricmp instead macro, e.g. #define strcasecmp _stricmp or something? Now it's you who puzzles me, because the header file *does* have exactly the macro that you suggest. Anyway, I think Peff's reply to my other mail summed it up nicely. I will come up with another patch. -- Sebastian Schuberth -- 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] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 12, 2013 at 10:29 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: I think there are basically three classes of solution: 1. Declare __NO_INLINE__ everywhere. I'd worry this might affect other environments, who would then not inline and lose performance (but since it's a non-standard macro, we don't really know what it will do in other places; possibly nothing). 2. Declare __NO_INLINE__ on mingw. Similar to above, but we know it only affects mingw, and we know the meaning of NO_INLINE there. 3. Try to impact only the uses as a function pointer (e.g., by using a wrapper function as suggested in the thread). Your patch does (1), I believe. Junio's patch does (3), but is a maintenance burden in that any new callsites will need to remember to do the same trick. Well, if by everywhere in (1) you mean on all platforms then you're right. But my patch does not define __NO_INLINE__ globally, but only at the time string.h / strings.h is included. Afterwards __NO_INLINE__ is undefined. In that sense, __NO_INLINE__ is not defined everywhere. Agreed. If that #define __NO_INLINE__ does not appear in the common part of our header files like git-compat-util.h but is limited to somewhere in compat/, that would be the perfect outcome. It's not that easy to move the definition of __NO_INLINE__ into compat/ because git-compat-util.h includes string.h / strings.h before anything of compat/. More over, defining __NO_INLINE__ in somewhere in compat/ would not limit its definition to the string.h / strings.h headers only. So how about something like this on top of my original patch: --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,12 +85,16 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#ifdef __MINGW32__ #define __NO_INLINE__ /* do not inline strcasecmp() */ +#endif #include string.h +#ifdef __MINGW32__ +#undef __NO_INLINE__ +#endif #ifdef HAVE_STRINGS_H #include strings.h /* for strcasecmp() */ #endif -#undef __NO_INLINE__ #ifdef WIN32 /* Both MinGW and MSVC */ #ifndef _WIN32_WINNT -- Sebastian Schuberth -- 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/4] pack v4: add v4_size to struct delta_base_cache_entry
On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote:
The intention is to store flat v4 trees in delta base cache to avoid
repeatedly expanding copy sequences in v4 trees. When the user needs
to unpack a v4 tree and the tree is found in the cache, the tree will
be converted back to canonical format. Future tree_desc interface may
skip canonical format and read v4 trees directly.
For that to work we need to keep track of v4 tree size after all copy
sequences are expanded, which is the purpose of this new field.
Hmmm I think this is going in a wrong direction.
If we want a cache for pv4 objects, it would be best to keep it separate
from the current delta cache in sha1_file.c for code maintainability
reasons. I'd suggest creating another cache in packv4-parse.c instead.
Yet, pavkv4 tree walking shouldn't need a cache since there is nothing
to expand in the end. Entries should be advanced one by one as they are
needed. Granted when converting back to a canonical object we need all
of them, but eventually this shouldn't be the main mode of operation.
However I can see that, as you say, the same base object is repeatedly
referenced. This means that we need to parse it over and over again
just to find the right offset where the needed entries start. We
probably end up skipping over the first entries in a tree object
multiple times. And that would be true even when the core code learns
to walk pv4 trees directly.
So here's the beginning of a tree offset cache to mitigate this problem.
It is incomplete as the cache function is not implemented, etc. But
that should give you the idea.
diff --git a/packv4-parse.c b/packv4-parse.c
index ae3e6a5..39b4f31 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -339,9 +339,9 @@ void *pv4_get_commit(struct packed_git *p, struct
pack_window **w_curs,
return dst;
}
-static int copy_canonical_tree_entries(struct packed_git *p, off_t offset,
- unsigned int start, unsigned int count,
- unsigned char **dstp, unsigned long
*sizep)
+static off_t copy_canonical_tree_entries(struct packed_git *p, off_t offset,
+ unsigned int start, unsigned int count,
+ unsigned char **dstp, unsigned long *sizep)
{
void *data;
const unsigned char *from, *end;
@@ -369,13 +369,19 @@ static int copy_canonical_tree_entries(struct packed_git
*p, off_t offset,
if (end - from *sizep) {
free(data);
- return -1;
+ return 0;
}
memcpy(*dstp, from, end - from);
*dstp += end - from;
*sizep -= end - from;
free(data);
- return 0;
+
+
+ /*
+* We won't cache this tree's current offset so let's just
+* indicate success with a non-zero return value.
+*/
+ return 1;
}
static int tree_entry_prefix(unsigned char *buf, unsigned long size,
@@ -404,39 +410,56 @@ static int tree_entry_prefix(unsigned char *buf, unsigned
long size,
return len;
}
-static int decode_entries(struct packed_git *p, struct pack_window **w_curs,
- off_t offset, unsigned int start, unsigned int count,
- unsigned char **dstp, unsigned long *sizep, int hdr)
+static off_t decode_entries(struct packed_git *p, struct pack_window **w_curs,
+ off_t offset, unsigned int start, unsigned int count,
+ unsigned char **dstp, unsigned long *sizep, int hdr)
{
- unsigned long avail;
- unsigned int nb_entries;
+ unsigned long avail = 0;
const unsigned char *src, *scp;
off_t copy_objoffset = 0;
- src = use_pack(p, w_curs, offset, avail);
- scp = src;
-
if (hdr) {
+ unsigned int nb_entries;
+
+ src = use_pack(p, w_curs, offset, avail);
+ scp = src;
+
/* we need to skip over the object header */
while (*scp 128)
if (++scp - src = avail - 20)
- return -1;
+ return 0;
+
/* is this a canonical tree object? */
if ((*scp 0xf) == OBJ_TREE)
return copy_canonical_tree_entries(p, offset,
start, count,
dstp, sizep);
+
/* let's still make sure this is actually a pv4 tree */
if ((*scp++ 0xf) != OBJ_PV4_TREE)
- return -1;
- }
+ return 0;
+
+ nb_entries = decode_varint(scp);
+ if (scp == src || start nb_entries ||
+ count nb_entries - start)
+ return 0;
+
+ /*
+* If this is a partial
Re: [PATCH 1/2] relative_path should honor dos_drive_prefix
On 13.09.13 06:55, Jiang Xin wrote: 2013/9/13 Junio C Hamano gits...@pobox.com: For systems that need POSIX escape hatch for Apollo Domain ;-), we would need a bit more work. When both path1 and path2 begin with a double-dash, we would need to check if they match up to the next slash, so that - //host1/usr/src and //host1/usr/lib share the same root and the former can be made to ../src relative to the latter; - //host1/usr/src and //host2/usr/lib are of separate roots. or something. But how could we know which platform supports network pathnames and needs such implementation. Similar to the has_dos_drive_prefix: For Windows/Mingw we do like this mingw.h #define has_dos_drive_prefix(path) (isalpha(*(path)) (path)[1] == ':') And all other platforms defines has_dos_drive_prefix() to be 0 here git-compat-util.h #ifndef has_dos_drive_prefix #define has_dos_drive_prefix(path) 0 #endif mingw.h: #define has_unc_path_prefix(path) ((path)[0] == '/' (path)[1] == '/') (Or may be) #define has_unc_path_prefix(path) (is_dir_sep((path)[0]) is_dir_sep((path)[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: [PATCH 2/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre n...@fluxnic.net wrote: On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote: The intention is to store flat v4 trees in delta base cache to avoid repeatedly expanding copy sequences in v4 trees. When the user needs to unpack a v4 tree and the tree is found in the cache, the tree will be converted back to canonical format. Future tree_desc interface may skip canonical format and read v4 trees directly. For that to work we need to keep track of v4 tree size after all copy sequences are expanded, which is the purpose of this new field. Hmmm I think this is going in a wrong direction. Good thing you caught me early. I was planning to implement a better version of this on the weekend. And you are not wrong about code maintainability, unpack_entry() so far looks very close to a real mess. Yet, pavkv4 tree walking shouldn't need a cache since there is nothing to expand in the end. Entries should be advanced one by one as they are needed. Granted when converting back to a canonical object we need all of them, but eventually this shouldn't be the main mode of operation. There's another case where one of the base tree is not v4 (the packer is inefficient, like my index-pack --fix-thin). For trees with leading zeros in entry mode, we can just do a lossy conversion to v4, but I wonder if there is a case where we can't even convert to v4 and the v4 treewalk interface has to fall back to canonical format.. I guess that can't happen. However I can see that, as you say, the same base object is repeatedly referenced. This means that we need to parse it over and over again just to find the right offset where the needed entries start. We probably end up skipping over the first entries in a tree object multiple times. And that would be true even when the core code learns to walk pv4 trees directly. So here's the beginning of a tree offset cache to mitigate this problem. It is incomplete as the cache function is not implemented, etc. But that should give you the idea. Thanks. I'll have a closer look and maybe complete your patch. -- 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] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth sschube...@gmail.com writes: On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote: I'm not too happy with the wording either. As I see it, even on MinGW runtime version 4.0 it's not true that string.h has _only_ inline definition of strcasecmp; there's also #define strncasecmp _strnicmp which effectively provides a non-inline definition of strncasecmp aka _strnicmp. I do not get this part. Sure, string.h would have definitions of things other than strcasecmp, such as strncasecmp. So what? Sorry, I mixed up strcasecmp and strncasecmp. OK. Does it effectively provide a non-inline definition of strcasecmp? Yes, if __NO_INLINE__ is defined string.h provides non-inline definition of both strcasecmp and strncasecmp by defining them to _stricmp and _strnicmp respectively. Perhaps the real issue is that the header file does not give an equivalent those who want to take the address of strcasecmp will get the address of _stricmp instead macro, e.g. #define strcasecmp _stricmp or something? Now it's you who puzzles me, because the header file *does* have exactly the macro that you suggest. Then why does your platform have problem with the code that takes the address of strcasecmp and stores it in the variable? It is not me, but your platform that is puzzling us. There is something else going on, like you do not have that #define enabled under some condition, or something silly like that. -- 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] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth sschube...@gmail.com writes: Well, if by everywhere in (1) you mean on all platforms then you're right. But my patch does not define __NO_INLINE__ globally, but only at the time string.h / strings.h is included. Afterwards __NO_INLINE__ is undefined. In that sense, __NO_INLINE__ is not defined everywhere. Which means people who do want to see that macro defined will be broken after that section of the header file which unconditionally undefs it, right? That is exactly why that change should not appear in the platform neutral part of the header file. --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,12 +85,16 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#ifdef __MINGW32__ #define __NO_INLINE__ /* do not inline strcasecmp() */ +#endif #include string.h +#ifdef __MINGW32__ +#undef __NO_INLINE__ +#endif That is certainly better than the unconditional one, but I wonder if it is an option to add compat/mingw/string.h without doing the above, though. That header file can do the no-inline dance before including the real thing with #include_next, and nobody else would notice, no? #ifdef __NO_INLINE__ #define __NO_INLINE_WAS_THERE 1 #else #define __NO_INLINE__ #define __NO_INLINE_WAS_THERE 0 #endif #include_next string.h #if !__NO_INLINE_WAS_THERE #undef __NO_INLINE__ #endif or something like that. That of course assumes nobody compiles for _MINGW32_ with a compiler that does not understrand #include_next and I do not know if that restriction is a showstopper or not. #ifdef HAVE_STRINGS_H #include strings.h /* for strcasecmp() */ #endif -#undef __NO_INLINE__ #ifdef WIN32 /* Both MinGW and MSVC */ #ifndef _WIN32_WINNT -- 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: Removing all notes containing a specific string
On Fri, Sep 13, 2013 at 10:12 AM, Johan Herland jo...@herland.net wrote: On Fri, Sep 13, 2013 at 8:51 AM, Francis Moreau francis.m...@gmail.com wrote: Hello, I'd like to know if that's possible to parse all notes to detect a special string and if it's the case, remove the note like git-notes remove would do. Hi, There's no built-in command/option to do this, but the following shell one-liner should do the job: git grep -l $mystring refs/notes/commits | cut -d':' -f2 | tr -d '/' | xargs git notes remove Looks magic to me but does the trick Thanks ! -- Francis -- 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: Fwd: Fwd: git-daemon access-hook race condition
For now I'm trying to do the following: access-hook.bash has: delayed-notify.bash $@ delayed-notify.bash has: sleep 10 ... curl ... I'm expecting access-hook to spawn new process and return without waiting for it to finish to let the service to do its job. But when i do push - it sleeps for 10 seconds anyway. Am i missing something obvious here? Any help is much appreciated! Thanks, Eugene I found a following solution to make it happen while waiting for somebody to be generous enough to take on the --post-service-hook (unfortunately i'm not a C guy): It is using 'at' command. The access-hook script has: echo delayed-notify.bash $@ | at now while delayed-notify.bash has: sleep 10 curl ... This is not perfect and in certain situations can still fail because the delay is not long enough but this will at least resolve 90% of issues. I hope that might be helpful for someone. Thanks, Eugene -- 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 v2 2/2] version-gen: avoid messing the version
Felipe Contreras felipe.contre...@gmail.com writes:
If the version is 'v1.8.4-rc1' that is the version, and there's no need
to change it to anything else, like 'v1.8.4.rc1'.
If RedHat, or somebody else, needs a specific version, they can use the
'version' file, like everybody else.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
I already explained to you why this is a bad change.
When we say we try to avoid regressions, we really mean it.
Before coming up with a change to pay Paul by robbing Peter, we must
make an honest effort to see if there is a way to pay Paul without
robbing anybody.
This change forces existing users who depend on how dashes are
mangled into dots to change their tooling.
We do occasionally make deliberate regressions that force existing
users to change the way they work, but only when there is no other
way, and when the benefit of the change far outweighs the cost of
such an adjustment, and with careful planning to ease the pain of
transition. The updates to git add and git push planned for 2.0
fall into that category.
There has to be a benefit that far outweighs the inconvenience this
patch imposes on existing users, but I do not see there is any. If
somebody needs a specific version, they can use the 'version' file
does not justify it at all; it equally applies to those who want to
use a version name with a dash.
Besides, the patch does not even do what it claims to do; if the
version is v1.8.4-rc1, what you get out of the updated code is
1.8.4-rc1, still losing the leading v.
I'd be more receptive if the patch were like this instead, though.
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index b444c18..c6d78ec 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,12 +3,16 @@
GVF=GIT-VERSION-FILE
DEF_VER=v1.8.4.GIT
+READ_RAW_VERSION=
LF='
'
# First see if there is a version file (included in release tarballs),
# then try git-describe, then default.
-if test -f version
+if test -f raw-version VN=$(cat raw-version)
+then
+ READ_RAW_VERSION=yes
+elif test -f version
then
VN=$(cat version) || VN=$DEF_VER
elif test -d ${GIT_DIR:-.git} -o -f .git
@@ -26,7 +30,10 @@ else
VN=$DEF_VER
fi
-VN=$(expr $VN : v*'\(.*\)')
+if test -z $READ_RAW_VERSION
+then
+ VN=$(expr $VN : v*'\(.*\)')
+fi
if test -r $GVF
then
--
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] pack-objects: no crc check when the cached version is used
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Current code makes pack-objects always do check_pack_crc() in
unpack_entry() even if right after that we find out there's a cached
version and pack access is not needed. Swap two code blocks, search
for cached version first, then check crc.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Interesting.
This is only triggered inside pack-objects, which would read a lot
of data from existing packs, and the overhead for looking up the
entry from the revindex, faulting in the actual packdata, and
computing and comparing the crc would not be trivial, especially as
the cost is incurred over many objects we need to untangle in the
delta chain. If you have interesting numbers to show how much this
improves the performance, I am curious to see it.
Good spotting ;-)
sha1_file.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 8c2d1ed..4955724 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2126,6 +2126,16 @@ void *unpack_entry(struct packed_git *p, off_t
obj_offset,
int i;
struct delta_base_cache_entry *ent;
+ ent = get_delta_base_cache_entry(p, curpos);
+ if (eq_delta_base_cache_entry(ent, p, curpos)) {
+ type = ent-type;
+ data = ent-data;
+ size = ent-size;
+ clear_delta_base_cache_entry(ent);
+ base_from_cache = 1;
+ break;
+ }
+
if (do_check_packed_object_crc p-index_version 1) {
struct revindex_entry *revidx = find_pack_revindex(p,
obj_offset);
unsigned long len = revidx[1].offset - obj_offset;
@@ -2140,16 +2150,6 @@ void *unpack_entry(struct packed_git *p, off_t
obj_offset,
}
}
- ent = get_delta_base_cache_entry(p, curpos);
- if (eq_delta_base_cache_entry(ent, p, curpos)) {
- type = ent-type;
- data = ent-data;
- size = ent-size;
- clear_delta_base_cache_entry(ent);
- base_from_cache = 1;
- break;
- }
-
type = unpack_object_header(p, w_curs, curpos, size);
if (type != OBJ_OFS_DELTA type != OBJ_REF_DELTA)
break;
--
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] name-hash: refactor polymorphic index_name_exists()
Eric Sunshine sunsh...@sunshineco.com writes:
Depending upon the absence or presence of a trailing '/' on the incoming
pathname, index_name_exists() checks either if a file is present in the
index or if a directory is represented within the index. Each caller
explicitly chooses the mode of operation by adding or removing a
trailing '/' before invoking index_name_exists().
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
Splitting index searching into two functions relieves callers of the
artificial burden of having to add or remove a slash to select the mode
of operation; instead they just call the desired function. A subsequent
patch will take advantage of this benefit in order to eliminate the
requirement that the incoming pathname for a directory search must have
a trailing slash.
Good thinking. Thanks for working on this; I agree with the general
direction this series is going.
I however wonder if it would be a good idea to rename the other one
to {cache|index}_file_exists(), and even keep the *_name_exists()
variant that switches between the two based on the trailing slash,
to avoid breaking other topics in flight that may have added new
callsites to *_name_exists(). Otherwise the new callsites will feed
a path with a trailing slash to *_name_exists() and get a false
result, no?
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
cache.h | 2 ++
dir.c| 7 ---
name-hash.c | 47 ---
read-cache.c | 2 +-
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/cache.h b/cache.h
index 9ef778a..03ec24c 100644
--- a/cache.h
+++ b/cache.h
@@ -314,6 +314,7 @@ extern void free_name_hash(struct index_state *istate);
#define refresh_cache(flags) refresh_index(the_index, (flags), NULL, NULL,
NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(the_index, (ce), (st),
(options))
#define ce_modified(ce, st, options) ie_modified(the_index, (ce), (st),
(options))
+#define cache_dir_exists(name, namelen) index_dir_exists(the_index, (name),
(namelen))
#define cache_name_exists(name, namelen, igncase)
index_name_exists(the_index, (name), (namelen), (igncase))
#define cache_name_is_other(name, namelen) index_name_is_other(the_index,
(name), (namelen))
#define resolve_undo_clear() resolve_undo_clear_index(the_index)
@@ -463,6 +464,7 @@ extern int write_index(struct index_state *, int newfd);
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
+extern struct cache_entry *index_dir_exists(struct index_state *istate,
const char *name, int namelen);
extern struct cache_entry *index_name_exists(struct index_state *istate,
const char *name, int namelen, int igncase);
extern int index_name_pos(const struct index_state *, const char *name, int
namelen);
#define ADD_CACHE_OK_TO_ADD 1/* Ok to add */
diff --git a/dir.c b/dir.c
index b439ff0..0080673 100644
--- a/dir.c
+++ b/dir.c
@@ -860,7 +860,8 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if ((len == 0 || pathname[len - 1] != '/')
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir-entries, dir-nr+1, dir-alloc);
@@ -885,11 +886,11 @@ enum exist_status {
/*
* Do not use the alphabetically sorted index to look up
* the directory name; instead, use the case insensitive
- * name hash.
+ * directory hash.
*/
static enum exist_status directory_exists_in_index_icase(const char
*dirname, int len)
{
- const struct cache_entry *ce = cache_name_exists(dirname, len + 1,
ignore_case);
+ const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
unsigned char endchar;
if (!ce)
diff --git a/name-hash.c b/name-hash.c
index 617c86c..5b01554 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -222,11 +222,35 @@ static int same_name(const struct cache_entry *ce,
const char *name, int namelen
return slow_same_name(name, namelen, ce-name, len);
}
+struct cache_entry *index_dir_exists(struct index_state *istate, const char
*name, int namelen)
+{
+ struct cache_entry *ce;
+ struct dir_entry *dir;
+
+ lazy_init_name_hash(istate);
+ dir = find_dir_entry(istate, name, namelen);
+ if (dir dir-nr)
+ return dir-ce;
+
+ /*
+ * It might be a submodule. Unlike plain directories, which are stored
+ * in the dir-hash, submodules are stored in the name-hash, so
Re: [PATCH 1/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 2:40 PM, Junio C Hamano gits...@pobox.com wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
Depending upon the absence or presence of a trailing '/' on the incoming
pathname, index_name_exists() checks either if a file is present in the
index or if a directory is represented within the index. Each caller
explicitly chooses the mode of operation by adding or removing a
trailing '/' before invoking index_name_exists().
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
Splitting index searching into two functions relieves callers of the
artificial burden of having to add or remove a slash to select the mode
of operation; instead they just call the desired function. A subsequent
patch will take advantage of this benefit in order to eliminate the
requirement that the incoming pathname for a directory search must have
a trailing slash.
Good thinking. Thanks for working on this; I agree with the general
direction this series is going.
I however wonder if it would be a good idea to rename the other one
to {cache|index}_file_exists(), and even keep the *_name_exists()
variant that switches between the two based on the trailing slash,
to avoid breaking other topics in flight that may have added new
callsites to *_name_exists(). Otherwise the new callsites will feed
a path with a trailing slash to *_name_exists() and get a false
result, no?
An assert(no trailing /) was added to index_name_exists() precisely
for the purpose of catching cases of code incorrectly calling
index_name_exists() to search for a directory. No existing code in
'master' trips the assertion (at least according to the tests),
however, the assertion may be annoying for topics in flight which do.
Other than plopping these patches atop 'pu' and seeing if anything
breaks, what would be the git way of detecting in-flight topics
which add callers of index_name_exists()? (Excuse my git ignorance.)
--
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] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 4:26 PM, Junio C Hamano gits...@pobox.com wrote: Perhaps the real issue is that the header file does not give an equivalent those who want to take the address of strcasecmp will get the address of _stricmp instead macro, e.g. #define strcasecmp _stricmp or something? Now it's you who puzzles me, because the header file *does* have exactly the macro that you suggest. Then why does your platform have problem with the code that takes the address of strcasecmp and stores it in the variable? It is not me, but your platform that is puzzling us. There is something else going on, like you do not have that #define enabled under some condition, or something silly like that. Exactly. That define is only enabled if __NO_INLINE__ is defined. Which is what my patch is all about: Define __NO_INLINE__ so that we get #define strcasecmp _stricmp. -- Sebastian Schuberth -- 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 v2 1/3] test: use unambigous leading path (/foo) for mingw
Jiang Xin worldhello@gmail.com writes: In test cases for relative_path, path with one leading character (such as /a, /x) may be recogonized as a:/ or x:/ if there is such doc drive on MINGW platform. Use an umambigous leading path /foo instead. DOS drive, you mean? Are they really spelled as /a or /x (not e.g. //a or something)? Just double-checking. Signed-off-by: Jiang Xin worldhello@gmail.com --- t/t0060-path-utils.sh | 56 +-- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 3a48de2..82a6f21 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -190,33 +190,33 @@ test_expect_success SYMLINKS 'real path works on symlinks' ' test $sym = $(test-path-utils real_path $dir2/syml) ' -relative_path /a/b/c//a/b/ c/ -relative_path /a/b/c//a/bc/ -relative_path /a//b//c/ //a/b// c/ POSIX -relative_path /a/b /a/b./ -relative_path /a/b/ /a/b./ -relative_path /a /a/b../ -relative_path / /a/b/ ../../ -relative_path /a/c /a/b/ ../c -relative_path /a/c /a/b../c -relative_path /x/y /a/b/ ../../x/y -relative_path /a/b empty /a/b -relative_path /a/b null/a/b -relative_path a/b/c/ a/b/c/ -relative_path a/b/c/ a/b c/ -relative_path a/b//c a//bc -relative_path a/b/ a/b/./ -relative_path a/b/ a/b ./ -relative_path a a/b ../ -relative_path x/ya/b ../../x/y -relative_path a/ca/b ../c -relative_path a/bempty a/b -relative_path a/bnulla/b -relative_path empty /a/b./ -relative_path empty empty ./ -relative_path empty null./ -relative_path null empty ./ -relative_path null null./ -relative_path null /a/b./ +relative_path /foo/a/b/c//foo/a/b/ c/ +relative_path /foo/a/b/c//foo/a/bc/ +relative_path /foo/a//b//c/ //foo/a/b// c/ POSIX +relative_path /foo/a/b /foo/a/b./ +relative_path /foo/a/b/ /foo/a/b./ +relative_path /foo/a /foo/a/b../ +relative_path / /foo/a/b/ ../../../ +relative_path /foo/a/c /foo/a/b/ ../c +relative_path /foo/a/c /foo/a/b../c +relative_path /foo/x/y /foo/a/b/ ../../x/y +relative_path /foo/a/b empty /foo/a/b +relative_path /foo/a/b null/foo/a/b +relative_path foo/a/b/c/ foo/a/b/c/ +relative_path foo/a/b/c/ foo/a/b c/ +relative_path foo/a/b//c foo/a//bc +relative_path foo/a/b/ foo/a/b/./ +relative_path foo/a/b/ foo/a/b ./ +relative_path foo/a foo/a/b ../ +relative_path foo/x/yfoo/a/b ../../x/y +relative_path foo/a/cfoo/a/b ../c +relative_path foo/a/bempty foo/a/b +relative_path foo/a/bnullfoo/a/b +relative_path empty /foo/a/b./ +relative_path empty empty ./ +relative_path empty null./ +relative_path null empty ./ +relative_path null null./ +relative_path null /foo/a/b./ 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
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 4:37 PM, Junio C Hamano gits...@pobox.com wrote: Which means people who do want to see that macro defined will be broken after that section of the header file which unconditionally undefs it, right? Right, but luckily you've fixed that in our proposed patch :-) That is certainly better than the unconditional one, but I wonder if it is an option to add compat/mingw/string.h without doing the above, though. I don't like the idea of introducing a compat/mingw/string.h because of two reasons: You would have to add a conditional to include that string.h instead of the system one anyway, so we could just as well keep the conditional in git-compat-util.h along with the logic. And I don't like the include_next GCC-ism, especially as I was planning to take a look at compiling Git with LLVM / clang under Windows. So how about this: --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,6 +85,25 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#ifdef __MINGW32__ +#ifdef __NO_INLINE__ +#define __NO_INLINE_ALREADY_DEFINED +#else +#define __NO_INLINE__ /* do not inline strcasecmp() */ +#endif +#endif +#include string.h +#ifdef __MINGW32__ +#ifdef __NO_INLINE_ALREADY_DEFINED +#undef __NO_INLINE_ALREADY_DEFINED +#else +#undef __NO_INLINE__ +#endif +#endif +#ifdef HAVE_STRINGS_H +#include strings.h /* for strcasecmp() */ +#endif + #ifdef WIN32 /* Both MinGW and MSVC */ #define _WIN32_WINNT 0x0502 #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ @@ -99,10 +118,6 @@ #include stddef.h #include stdlib.h #include stdarg.h -#include string.h -#ifdef HAVE_STRINGS_H -#include strings.h /* for strcasecmp() */ -#endif #include errno.h #include limits.h #ifdef NEEDS_SYS_PARAM_H -- Sebastian Schuberth -- 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] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 12:53 PM, Sebastian Schuberth sschube...@gmail.com wrote: +#ifdef __MINGW32__ +#ifdef __NO_INLINE__ Why do you want to push this insane workaround for a clear Mingw bug? Please have mingw just fix the nasty bug, and the git patch with the trivial wrapper looks much simpler than just saying don't inline anything and that crazy block of nasty mingw magic #defines/. And then document loudly that the wrapper is due to the mingw bug. Linus -- 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] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth sschube...@gmail.com writes: I don't like the idea of introducing a compat/mingw/string.h because of two reasons: You would have to add a conditional to include that string.h instead of the system one anyway, With -Icompat/mingw passed to the compiler, which is a bog-standard technique we already use to supply headers the system forgot to supply or override buggy headers the system is shipped with, you do not have to change any #include string.h. Am I mistaken? -- 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] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 9:56 PM, Linus Torvalds torva...@linux-foundation.org wrote: +#ifdef __MINGW32__ +#ifdef __NO_INLINE__ Why do you want to push this insane workaround for a clear Mingw bug? To be frank, because Git is picking up patches much quicker than MinGW does, and I want a solution ASAP. Although I of course agree that fixing the real issue upstream in MinGW is the better solution. Please have mingw just fix the nasty bug, and the git patch with the I'll try to come up with a MinGW patch in parallel. trivial wrapper looks much simpler than just saying don't inline anything and that crazy block of nasty mingw magic #defines/. It may look simpler, but as outlines in this thread it's less maintainable because you need to remember to use the wrapper. And people tend to forget that no matter how loudly you document that. If we can make the code more fool proof we IMHO should do so. -- Sebastian Schuberth -- 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] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano gits...@pobox.com wrote: I don't like the idea of introducing a compat/mingw/string.h because of two reasons: You would have to add a conditional to include that string.h instead of the system one anyway, With -Icompat/mingw passed to the compiler, which is a bog-standard technique we already use to supply headers the system forgot to supply or override buggy headers the system is shipped with, you do not have to change any #include string.h. Am I mistaken? Ah, that would work I guess, but you'd still need the include_next. -- Sebastian Schuberth -- 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] pack-objects: no crc check when the cached version is used
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Current code makes pack-objects always do check_pack_crc() in unpack_entry() even if right after that we find out there's a cached version and pack access is not needed. Swap two code blocks, search for cached version first, then check crc. [...] Interesting. This is only triggered inside pack-objects, which would read a lot of data from existing packs, and the overhead for looking up the entry from the revindex, faulting in the actual packdata, and computing and comparing the crc would not be trivial, especially as the cost is incurred over many objects we need to untangle in the delta chain. If you have interesting numbers to show how much this improves the performance, I am curious to see it. I can't see anything wrong with the patch, but then I haven't stared too hard. (It seems that my conversion around abe601b (sha1_file: remove recursion in unpack_entry, 2013-03-27) was faithful on this point, the problem has existed for longer than that.) I tried the perf script below, but at least for the git repo the only thing I can see is noise. --- 8 --- t/perf/p5300-pack-object.sh --- 8 --- #!/bin/sh test_description=Tests object packing performance . ./perf-lib.sh test_perf_default_repo test_perf 'pack-objects on commits in HEAD' ' git rev-list HEAD | git pack-objects --stdout /dev/null ' test_perf 'pack-objects on all of HEAD' ' git rev-list --objects HEAD | git pack-objects --stdout /dev/null ' 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 V2 2/3] config doc: update dot-repository notes
branch.name.remote can be set to '.' (period) as the repository path (URL) as part of the remote name dwimmery. Tell the reader. Such relative paths are not 'special'. Correct the branch.name.merge note. Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/config.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 599ca52..da63043 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -718,6 +718,8 @@ branch.name.remote:: overridden by `branch.name.pushremote`. If no remote is configured, or if you are not on any branch, it defaults to `origin` for fetching and `remote.pushdefault` for pushing. + Additionally, `.` (a period) is the current local repository + (a dot-repository), see `branch.name.merge`'s final note below. branch.name.pushremote:: When on branch name, it overrides `branch.name.remote` for @@ -743,8 +745,8 @@ branch.name.merge:: Specify multiple values to get an octopus merge. If you wish to setup 'git pull' so that it merges into name from another branch in the local repository, you can point - branch.name.merge to the desired branch, and use the special setting - `.` (a period) for branch.name.remote. + branch.name.merge to the desired branch, and use the relative path + setting `.` (a period) for branch.name.remote. branch.name.mergeoptions:: Sets default options for merging into branch name. The syntax and -- 1.8.1.msysgit.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
[PATCH V2 3/3] doc: command line interface (cli) dot-repository dwimmery
The Git cli will accept dot '.' (period) as the relative path to the current repository. Explain this action. Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/gitcli.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt index 7d54b77..b065c0e 100644 --- a/Documentation/gitcli.txt +++ b/Documentation/gitcli.txt @@ -58,6 +58,10 @@ the paths in the index that match the pattern to be checked out to your working tree. After running `git add hello.c; rm hello.c`, you will _not_ see `hello.c` in your working tree with the former, but with the latter you will. ++ +Just as the filesystem '.' (period) refers to the current directory, +using a '.' as a repository name in Git (a dot-repository) is a relative +path for your current repository. Here are the rules regarding the flags that you should follow when you are scripting Git: -- 1.8.1.msysgit.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
[PATCH V2 1/3] Doc URLs: relative paths imply the dot-respository
Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/urls.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/urls.txt b/Documentation/urls.txt index 9ccb246..5350a63 100644 --- a/Documentation/urls.txt +++ b/Documentation/urls.txt @@ -55,6 +55,13 @@ These two syntaxes are mostly equivalent, except the former implies --local option. endif::git-clone[] +Relative paths are relative to the `$GIT_DIR`, thus the path: + +- '.' + +is the current repository and acts as if it were a repository +named `'.'`. + When Git doesn't know how to handle a certain transport protocol, it attempts to use the 'remote-transport' remote helper, if one exists. To explicitly request a remote helper, the following syntax -- 1.8.1.msysgit.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
[PATCH V2 0/3] Extend dot repository documentation
This is an update to my patch series on 19 May documenting the dot repository notation. The earlier threads start at: $gmane/224870/ [0/2] Extend dot repository documentation $gmane/224868/ [1/2] config doc: add dot-repository note $gmane/224869/ [2/2] doc: command line interface (cli) dot-repository dwimmery The base documentation is now the URLs section (first patch) as suggested by Jonathan, with a clarification in config(1) branch.name.remote, and finally a note in cli(7). The patches can be squashed together if appropriate. Philip -- Philip Oakley (3): Doc URLs: relative paths imply the dot-respository config doc: update dot-repository notes doc: command line interface (cli) dot-repository dwimmery Documentation/config.txt | 6 -- Documentation/gitcli.txt | 4 Documentation/urls.txt | 7 +++ 3 files changed, 15 insertions(+), 2 deletions(-) -- 1.8.1.msysgit.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: [PATCH 1/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 3:41 PM, Junio C Hamano gits...@pobox.com wrote:
On Fri, Sep 13, 2013 at 12:29 PM, Eric Sunshine sunsh...@sunshineco.com
wrote:
On Fri, Sep 13, 2013 at 2:40 PM, Junio C Hamano gits...@pobox.com wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
Good thinking. Thanks for working on this; I agree with the general
direction this series is going.
I however wonder if it would be a good idea to rename the other one
to {cache|index}_file_exists(), and even keep the *_name_exists()
variant that switches between the two based on the trailing slash,
to avoid breaking other topics in flight that may have added new
callsites to *_name_exists(). Otherwise the new callsites will feed
a path with a trailing slash to *_name_exists() and get a false
result, no?
An assert(no trailing /) was added to index_name_exists() precisely
for the purpose of catching cases of code incorrectly calling
index_name_exists() to search for a directory. No existing code in
'master' trips the assertion (at least according to the tests),
however, the assertion may be annoying for topics in flight which do.
Other than plopping these patches atop 'pu' and seeing if anything
breaks, what would be the git way of detecting in-flight topics
which add callers of index_name_exists()? (Excuse my git ignorance.)
I would do a quick
$ git diff master..pu | grep '^+.*_name_exists'
which would be more conservative (it would also show an invocation
moved from one place to another).
There appear to be no topics in flight which add new
index_name_exists() callers (which is not to say that no new callers
will be introduced before this topic lands in 'master').
I also plopped the patches atop 'pu' and there are no new tests
failures on Linux or Mac OS X, so the series does not seem to break
anything in flight. (There are a few test failures on 'pu' on Mac OS
X, but they are not related to this series.)
Given the above. How should I proceed? Do you still feel that it is
advisable to keep an index_name_exists() around for compatibility
reasons in case any new callers are introduced? Regardless of that
answer, do you want index_name_exists() renamed to
index_file_exists()?
--
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] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth sschube...@gmail.com writes: On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano gits...@pobox.com wrote: I don't like the idea of introducing a compat/mingw/string.h because of two reasons: You would have to add a conditional to include that string.h instead of the system one anyway, With -Icompat/mingw passed to the compiler, which is a bog-standard technique we already use to supply headers the system forgot to supply or override buggy headers the system is shipped with, you do not have to change any #include string.h. Am I mistaken? Ah, that would work I guess, but you'd still need the include_next. You can explicitly include the system header from your compatibility layer, i.e. === compat/mingw/string.h === #define __NO_INLINE__ #ifdef SYSTEM_STRING_H_HEADER #include SYSTEM_STRING_H_HEADER #else #include_next string.h #endif and then in config.mak.uname, do something like this: ifneq (,$(findstring MINGW,$(uname_S))) ifndef SYSTEM_STRING_H_HEADER SYSTEM_STRING_H_HEADER = C:\\llvm\include\string.h endif COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER) endif People who have the system header file at different paths can further override SYSTEM_STRING_H_HEADER in their config.mak. That would help compilers targetting mingw that do not support #include_next without spreading the damage to other people's systems, I think. -- 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] name-hash: refactor polymorphic index_name_exists()
Eric Sunshine sunsh...@sunshineco.com writes: Given the above. How should I proceed? Do you still feel that it is advisable to keep an index_name_exists() around for compatibility reasons in case any new callers are introduced? Regardless of that answer, do you want index_name_exists() renamed to index_file_exists()? Renaming *_name_exists() to *_file_exists() without keeping a compatibility one will force new topics to be rebased on this series. Alternatively we could merge them to 'pu' (and later 'next' and 'master') with evil merges to adjust the change in the semantics of the called function. That increases the risk of accidental breakages, I think. It is safer to keep index_name_exists() around with the older semantics, if we can, and rename your file only one to a different name. That way, even if a new topic still uses index_name_exists() expecting the traditional behaviour, it will not break immediately and we do not need to risk evil merges making mistakes. Later, we can git grep _name_exists to spot them and convert such old-style calls to either directory only or file only variants after this series and these follow-on topics hit 'master' (and we do not know at this point in what order that happens). Hmm? -- 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
What's cooking in git.git (Sep 2013, #04; Fri, 13)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The third batch of topics are now in 'master'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * jc/commit-is-spelled-with-two-ems (2013-09-05) 2 commits (merged to 'next' on 2013-09-05 at 982aef2) + typofix: cherry is spelled with two ars + typofix: commit is spelled with two ems * jc/pager-configuration-doc (2013-08-29) 1 commit (merged to 'next' on 2013-09-05 at 3169083) + config: rewrite core.pager documentation It was unclear in the documentation how various configurations and environment variables determine which pager is eventually used. * jk/config-int-range-check (2013-09-09) 5 commits (merged to 'next' on 2013-09-09 at 9ab779d) + git-config: always treat --int as 64-bit internally + config: make numeric parsing errors more clear + config: set errno in numeric git_parse_* functions + config: properly range-check integer values + config: factor out integer parsing from range checks git config did not provide a way to set or access numbers larger than a native int on the platform; it now provides 64-bit signed integers on all platforms. * mm/fast-import-feature-doc (2013-08-25) 1 commit (merged to 'next' on 2013-09-05 at 83802e2) + Documentation/fast-import: clarify summary for `feature` command * mm/mediawiki-dumb-push-fix (2013-09-03) 4 commits (merged to 'next' on 2013-09-05 at f8313f4) + git-remote-mediawiki: no need to update private ref in non-dumb push + git-remote-mediawiki: use no-private-update capability on dumb push + transport-helper: add no-private-update capability + git-remote-mediawiki: add test and check Makefile targets * mm/remote-helpers-doc (2013-08-26) 1 commit (merged to 'next' on 2013-09-05 at c181b35) + Documentation/remote-helpers: document common use-case for private ref * mn/doc-pack-heu-remove-dead-pastebin (2013-08-23) 1 commit (merged to 'next' on 2013-09-05 at 5caecec) + remove dead pastebin link from pack-heuristics document -- [New Topics] * jc/url-match (2013-09-12) 1 commit (merged to 'next' on 2013-09-13 at 7b94f8e) + urlmatch.c: recompute pointer after append_normalized_escapes While normalizing a URL, we forgot that the buffer that holds it could be relocated when it grows, which was a brown-paper-bag bug that can lead to a crash introduced on 'master' post 1.8.4 release. Will merge to 'master' in the fourth batch. * jx/relative-path-regression-fix (2013-09-13) 3 commits - Use simpler relative_path when set_git_dir - relative_path should honor dos_drive_prefix - test: use unambigous leading path (/foo) for mingw (this branch uses jx/clean-interactive.) * nd/unpack-entry-optim-in-pack-objects (2013-09-13) 1 commit - pack-objects: no crc check when the cached version is used The codepath to use data from packfiles that is only exercised in pack-objects unnecessarily checked crc checksum of the pack data, even when it ends up using in-core copy that it got by reading from the pack (at which point the checksum was validated). Will merge to 'next'. -- [Stalled] * jc/ref-excludes (2013-09-03) 2 commits - document --exclude option - revision: introduce --exclude=glob to tame wildcards People often wished a way to tell git log --branches (and git log --remotes --not --branches) to exclude some local branches from the expansion of --branches (similarly for --tags, --all and --glob=pattern). Now they have one. Needs a matching change to rev-parse. * rv/send-email-cache-generated-mid (2013-08-21) 2 commits - git-send-email: Cache generated message-ids, use them when prompting - git-send-email: add optional 'choices' parameter to the ask sub * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten refs/remotes/origin/HEAD - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, rev-parse --abbrev-ref remotes/origin/HEAD ought to show origin, not origin/HEAD, which is fixed with this series (if it is a
Re: [PATCH 1/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 6:16 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: Given the above. How should I proceed? Do you still feel that it is advisable to keep an index_name_exists() around for compatibility reasons in case any new callers are introduced? Regardless of that answer, do you want index_name_exists() renamed to index_file_exists()? Renaming *_name_exists() to *_file_exists() without keeping a compatibility one will force new topics to be rebased on this series. Alternatively we could merge them to 'pu' (and later 'next' and 'master') with evil merges to adjust the change in the semantics of the called function. That increases the risk of accidental breakages, I think. It is safer to keep index_name_exists() around with the older semantics, if we can, and rename your file only one to a different name. That way, even if a new topic still uses index_name_exists() expecting the traditional behaviour, it will not break immediately and we do not need to risk evil merges making mistakes. Later, we can git grep _name_exists to spot them and convert such old-style calls to either directory only or file only variants after this series and these follow-on topics hit 'master' (and we do not know at this point in what order that happens). Thanks. That's what I needed to know. I'll re-roll with the suggested changes. (And, I'm looking into the Mac-only test breakages not related to this patch series.) -- 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 v2] sequencer: trivial cleanup
Consider that the return values of allow_empty() could either be negative, zero, or one. However, there is no reason to be overtly conservative about it: we might as well return positive values as well since the callsite has no problems with it. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- sequencer.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index 351548f..ae25b5b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -463,13 +463,7 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit) if (opts-keep_redundant_commits) return 1; - empty_commit = is_original_commit_empty(commit); - if (empty_commit 0) - return empty_commit; - if (!empty_commit) - return 0; - else - return 1; + return is_original_commit_empty(commit); } static int do_pick_commit(struct commit *commit, struct replay_opts *opts) -- 1.8.4.299.gb3e7d24.dirty -- 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 V2 1/3] Doc URLs: relative paths imply the dot-respository
Philip Oakley philipoak...@iee.org writes: Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/urls.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/urls.txt b/Documentation/urls.txt index 9ccb246..5350a63 100644 --- a/Documentation/urls.txt +++ b/Documentation/urls.txt @@ -55,6 +55,13 @@ These two syntaxes are mostly equivalent, except the former implies --local option. endif::git-clone[] +Relative paths are relative to the `$GIT_DIR`, thus the path: Is it? git init src dst cd src git commit --allow-empty -m initial cd ../dst git fetch ../src HEAD:refs/heads/copy would work, but if it is relative to $GIT_DIR, the last one would need to be written as git fetch ../../src HEAD:refs/heads/copy wouldn't it? + +- '.' + +is the current repository and acts as if it were a repository +named `'.'`. + When Git doesn't know how to handle a certain transport protocol, it attempts to use the 'remote-transport' remote helper, if one exists. To explicitly request a remote helper, the following syntax -- 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 V2 2/3] config doc: update dot-repository notes
Philip Oakley philipoak...@iee.org writes: branch.name.remote can be set to '.' (period) as the repository path (URL) as part of the remote name dwimmery. Tell the reader. Such relative paths are not 'special'. Correct the branch.name.merge note. Looks good. It naturally follows that this is also valid: [branch master] merge = refs/heads/master remote = git://git.kernel.org/pub/scm/git/git.git and running git pull while on your 'master'. This is because branch.name.remote usually is spelled with a nickname that refers to the [remote nickname] section, but it does not have to be; it can use a URL to refer to the remote repository. Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/config.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 599ca52..da63043 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -718,6 +718,8 @@ branch.name.remote:: overridden by `branch.name.pushremote`. If no remote is configured, or if you are not on any branch, it defaults to `origin` for fetching and `remote.pushdefault` for pushing. + Additionally, `.` (a period) is the current local repository + (a dot-repository), see `branch.name.merge`'s final note below. branch.name.pushremote:: When on branch name, it overrides `branch.name.remote` for @@ -743,8 +745,8 @@ branch.name.merge:: Specify multiple values to get an octopus merge. If you wish to setup 'git pull' so that it merges into name from another branch in the local repository, you can point - branch.name.merge to the desired branch, and use the special setting - `.` (a period) for branch.name.remote. + branch.name.merge to the desired branch, and use the relative path + setting `.` (a period) for branch.name.remote. branch.name.mergeoptions:: Sets default options for merging into branch name. The syntax and -- 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 V2 3/3] doc: command line interface (cli) dot-repository dwimmery
Philip Oakley philipoak...@iee.org writes: The Git cli will accept dot '.' (period) as the relative path to the current repository. Explain this action. Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/gitcli.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt index 7d54b77..b065c0e 100644 --- a/Documentation/gitcli.txt +++ b/Documentation/gitcli.txt @@ -58,6 +58,10 @@ the paths in the index that match the pattern to be checked out to your working tree. After running `git add hello.c; rm hello.c`, you will _not_ see `hello.c` in your working tree with the former, but with the latter you will. ++ +Just as the filesystem '.' (period) refers to the current directory, +using a '.' as a repository name in Git (a dot-repository) is a relative +path for your current repository. Looks good to me. 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] git-compat-util: Avoid strcasecmp() being inlined
Junio C Hamano gits...@pobox.com writes: You can explicitly include the system header from your compatibility layer, i.e. ... and then in config.mak.uname, do something like this: ... COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER) You need to have one level of quoting to keep from being eaten; it should be sufficient to see how SHA1_HEADER that is included in cache.h is handled and imitate it. -- 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] pack-objects: no crc check when the cached version is used
On Sat, Sep 14, 2013 at 4:26 AM, Thomas Rast tr...@inf.ethz.ch wrote: Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Current code makes pack-objects always do check_pack_crc() in unpack_entry() even if right after that we find out there's a cached version and pack access is not needed. Swap two code blocks, search for cached version first, then check crc. [...] Interesting. This is only triggered inside pack-objects, which would read a lot of data from existing packs, and the overhead for looking up the entry from the revindex, faulting in the actual packdata, and computing and comparing the crc would not be trivial, especially as the cost is incurred over many objects we need to untangle in the delta chain. If you have interesting numbers to show how much this improves the performance, I am curious to see it. No I don't have any timing numbers. I just updated the code to see how many times crc is checked and how many times we find a cached version after crc is checked. The numbers with git.git are 353535 and 113257 respectively. IOW we could reduce the number of crc checks by 30%. I can't see anything wrong with the patch, but then I haven't stared too hard. (It seems that my conversion around abe601b (sha1_file: remove recursion in unpack_entry, 2013-03-27) was faithful on this point, the problem has existed for longer than that.) I tried the perf script below, but at least for the git repo the only thing I can see is noise. --stdout does not set do_check_packed_object_crc, you need to run pack-objects without --stdout (i.e. the real use case is repack) --- 8 --- t/perf/p5300-pack-object.sh --- 8 --- #!/bin/sh test_description=Tests object packing performance . ./perf-lib.sh test_perf_default_repo test_perf 'pack-objects on commits in HEAD' ' git rev-list HEAD | git pack-objects --stdout /dev/null ' test_perf 'pack-objects on all of HEAD' ' git rev-list --objects HEAD | git pack-objects --stdout /dev/null ' test_done -- 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 2/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, 13 Sep 2013, Duy Nguyen wrote: On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre n...@fluxnic.net wrote: On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote: The intention is to store flat v4 trees in delta base cache to avoid repeatedly expanding copy sequences in v4 trees. When the user needs to unpack a v4 tree and the tree is found in the cache, the tree will be converted back to canonical format. Future tree_desc interface may skip canonical format and read v4 trees directly. For that to work we need to keep track of v4 tree size after all copy sequences are expanded, which is the purpose of this new field. Hmmm I think this is going in a wrong direction. Good thing you caught me early. I was planning to implement a better version of this on the weekend. And you are not wrong about code maintainability, unpack_entry() so far looks very close to a real mess. Yes. We might have to break it into sub-functions. However this has the potential to recurse rather deeply so it is necessary to limit its stack footprint as well. Yet, pavkv4 tree walking shouldn't need a cache since there is nothing to expand in the end. Entries should be advanced one by one as they are needed. Granted when converting back to a canonical object we need all of them, but eventually this shouldn't be the main mode of operation. There's another case where one of the base tree is not v4 (the packer is inefficient, like my index-pack --fix-thin). For trees with leading zeros in entry mode, we can just do a lossy conversion to v4, but I wonder if there is a case where we can't even convert to v4 and the v4 treewalk interface has to fall back to canonical format.. I guess that can't happen. Well... There is little gain in converting loose tree objects into pv4 format so there will always be a place for the canolical tree parser. Eventually the base trees added by index-pack should be pv4 trees. The only case where this might not work is for zero padded mode trees and we don't have to optimize for that i.e. a fallback to the canonical tree format will be good enough. However I can see that, as you say, the same base object is repeatedly referenced. This means that we need to parse it over and over again just to find the right offset where the needed entries start. We probably end up skipping over the first entries in a tree object multiple times. And that would be true even when the core code learns to walk pv4 trees directly. So here's the beginning of a tree offset cache to mitigate this problem. It is incomplete as the cache function is not implemented, etc. But that should give you the idea. Thanks. I'll have a closer look and maybe complete your patch. I've committed an almost final patch and pushed it out. It is disabled right now due to some bug I've not found yet. But you can have a look. Nicolas
Re: [PATCH] pack-objects: no crc check when the cached version is used
On Sat, 14 Sep 2013, Duy Nguyen wrote: On Sat, Sep 14, 2013 at 4:26 AM, Thomas Rast tr...@inf.ethz.ch wrote: I tried the perf script below, but at least for the git repo the only thing I can see is noise. --stdout does not set do_check_packed_object_crc, you need to run pack-objects without --stdout (i.e. the real use case is repack) And for those who might wonder why, you can have a look at the description for commit 0e8189e2708b. This was probably one of my best commit logs ever. ;-) This commit also provides a hint about the cost of over-checking the CRC. Nicolas -- 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/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, 13 Sep 2013, Nicolas Pitre wrote: On Fri, 13 Sep 2013, Duy Nguyen wrote: On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre n...@fluxnic.net wrote: However I can see that, as you say, the same base object is repeatedly referenced. This means that we need to parse it over and over again just to find the right offset where the needed entries start. We probably end up skipping over the first entries in a tree object multiple times. And that would be true even when the core code learns to walk pv4 trees directly. So here's the beginning of a tree offset cache to mitigate this problem. It is incomplete as the cache function is not implemented, etc. But that should give you the idea. Thanks. I'll have a closer look and maybe complete your patch. I've committed an almost final patch and pushed it out. It is disabled right now due to some bug I've not found yet. But you can have a look. Found the bug. The cache is currently updated by the caller. The caller may ask for a copy of 2 entries from a base object, but that base object may itself copy those objects from somewhere else in a larger chunk. Let's consider this example: tree A -- 0 (0) copy 2 entries from tree B starting at entry 0 1 (2) copy 1 entry from tree B starting at entry 3 tree B -- 0 (0) copy 6 entries from tree C starting at entry 0 1 (6) entry foo.txt 2 (7) entry bar.txt Right now, the code calls decode_entries() to decode 2 entries from tree B but those entries are part of a copy from tree C. When that call returns, the cache is updated as if tree B entry #2 would start at offset 1 but this is wrong because offset 0 in tree B covers 6 entries and therefore offset 1 is for entry #6. So this needs a rethink. I won't be able to work on this for a few days. Nicolas -- 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] t7508: avoid non-portable sed expression
2556b996 (status: disable display of '#' comment prefix by default;
2013-09-06) introduced tests which fail on Mac OS X due to unportable
use of \t (for TAB) in a sed expression. POSIX [1][2] also disallows
it. Fix this.
[1]:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html#tag_20_116_13_02
[2]:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_02
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
This applies against 'next'.
t/t7508-status.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 7d52dbf..6fb59f3 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -61,7 +61,8 @@ test_expect_success 'status (1)' '
'
strip_comments () {
- sed s/^\# //; s/^\#$//; s/^#\t/\t/ $1 $1.tmp
+ tab=' '
+ sed s/^\# //; s/^\#$//; s/^#$tab/$tab/ $1 $1.tmp
rm $1 mv $1.tmp $1
}
--
1.8.4.535.g7b94f8e
--
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 v2 2/2] version-gen: avoid messing the version
On Fri, Sep 13, 2013 at 1:16 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: If the version is 'v1.8.4-rc1' that is the version, and there's no need to change it to anything else, like 'v1.8.4.rc1'. If RedHat, or somebody else, needs a specific version, they can use the 'version' file, like everybody else. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- I already explained to you why this is a bad change. No, you did not. All you did is throw a non sequitur argument which I already exposed as such. When we say we try to avoid regressions, we really mean it. Maybe by regressions you mean progress. Before coming up with a change to pay Paul by robbing Peter, we must make an honest effort to see if there is a way to pay Paul without robbing anybody. There is, because Peter, the RedHat maintainer, can do exactly the same that Alice does; use the 'version' file. In fact, Peter, cannot possibly use Git's version because of this: % rpmdev-vercmp git-1.8.4 git-1.8.4.rc1 git-1.8.4 git-1.8.4.rc1 % rpmdev-vercmp git-1.8.4 git-1.8.4-rc1 git-1.8.4 git-1.8.4-rc1 % rpmdev-vercmp git-1.8.4 git-1.8.4~rc1 git-1.8.4 git-1.8.4~rc1 Fedora's guideline[1] is to use a format like git-1.8.4-1.rc1, then the final release would be git-1.8.4-2, in other words, the '.rc1' is mostly informative, but that has nothing to do with Git's internal version (git-1.8.4.rc1). openSUSE's guideline[1] prefers to use git-1.8.4~rc1. Either way, none of them could possibly use git-1.8.4.rc1, because that's greater than git-1.8.4. This change forces existing users who depend on how dashes are mangled into dots to change their tooling. No, it does not. You just say that, and you assume because you said it, it must be true; it's not. First of all, nor RedHat, nor any other RPM-based distribution needs this any more, because as a simple search demonstrates, they don't use release candidates any more. http://www.rpmfind.net/linux/rpm2html/search.php?query=gitsubmit=Search+... Essentially, Peter is a figment of your imagination. The closest thing to Peter 1) doesn't use release candidates, and 2) if he did, he wouldn't use a version like git-1.8.4.rc1. We do occasionally make deliberate regressions that force existing users to change the way they work, but only when there is no other way, and when the benefit of the change far outweighs the cost of such an adjustment, and with careful planning to ease the pain of transition. The updates to git add and git push planned for 2.0 fall into that category. There has to be a benefit that far outweighs the inconvenience this patch imposes on existing users, but I do not see there is any. If somebody needs a specific version, they can use the 'version' file does not justify it at all; it equally applies to those who want to use a version name with a dash. Besides, the patch does not even do what it claims to do; if the version is v1.8.4-rc1, what you get out of the updated code is 1.8.4-rc1, still losing the leading v. Wrong. % git tag -m '' v1.8.5-rc1 % ./GIT-VERSION-GEN GIT_VERSION = 1.8.5-rc1-dirty [1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease [2] http://en.opensuse.org/openSUSE:Package_naming_guidelines -- Felipe Contreras -- 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
