Re: Nike Air Max TN
Trøst af skoen er høflighed på de teknologisk avancerede Nike Air Max Cushioning system. Denne plan er placeret under sko på er bedst til at absorbere stød genereret i virkningen. Dette er ikke overført på runner såvel som konklusion resultatet er virkelig en sko, Nike Free 5.0 Sko Billigt http://www.2014skotilbud.com/nike-free der giver en behagelig udflugt. nbsp skoen også passer meget godt mange på grund af metoden exceptionel snøring som effektivt en større, der er fremstillet i løbere. Øverst er lavet ud let og åndbar komponenter, som giver til en stor pasning af praktiske møde. Der er også fremragende trækkraft skyldige på BRS tusind ydersål. -- View this message in context: http://git.661346.n2.nabble.com/Nike-Air-Max-TN-tp7604110p7604111.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack: add `repack.honorpackkeep` config var
On Tue, Jan 28, 2014 at 01:21:43AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: The git-repack command always passes `--honor-pack-keep` to pack-objects. This has traditionally been a good thing, as we do not want to duplicate those objects in a new pack, and we are not going to delete the old pack. ... Note that this option just disables the pack-objects behavior. We still leave packs with a .keep in place, as we do not necessarily know that we have duplicated all of their objects. Signed-off-by: Jeff King p...@peff.net --- Intended for the jk/pack-bitmap topic. Two comments. Sorry, this one slipped through the cracks. Here's a re-roll addressing your comments. - It seems that this adds a configuration variable that cannot be countermanded from the command line. It has to come with either a very good justification in the documentation describing why it is a bad idea to even allow overriding from the command line in a repository that sets it, or a command line option to let the users override it. I personally prefer the latter, because that will be one less thing for users to remember (i.e. usually you can override the configured default from the command line, but this variable cannot be because of these very good reasons). It was less it is a bad idea to override and more I cannot conceive of any good reason to override. And since you can always use git -c, I didn't think it was worth cluttering repack's options. However, I suppose if one were explicitly bitmapping a single invocation with `git repack -adb`, it might make sense to use it on the command line. Fixed in the re-roll. - In the context of pack-objects, the name --honor-pack-keep makes sense; it is understood that pack-objects will _not_ remove kept packfile, so honoring can only mean do not attempt to pick objects out of kept packs to add to the pack being generated. and there is no room for --no-honor-pack-keep to be mistaken as you canremove the ones marked to be kept after saving the still-used objects in it away. But does the same name make sense in the context of repack? I think the distinction you are making is to capture the second second from the docs: If set to false, include objects in `.keep` files when repacking via `git repack`. Note that we still do not delete `.keep` packs after `pack-objects` finishes. The best name I could come up with is --pack-keep-objects, since that is literally what it is doing. I'm not wild about the name because it is easy to read keep as a verb (and pack as a noun). I think it's OK, but suggestions are welcome. -- 8 -- From: Vicent Marti tan...@gmail.com Subject: repack: add `repack.packKeepObjects` config var The git-repack command always passes `--honor-pack-keep` to pack-objects. This has traditionally been a good thing, as we do not want to duplicate those objects in a new pack, and we are not going to delete the old pack. However, when bitmaps are in use, it is important for a full repack to include all reachable objects, even if they may be duplicated in a .keep pack. Otherwise, we cannot generate the bitmaps, as the on-disk format requires the set of objects in the pack to be fully closed. Even if the repository does not generally have .keep files, a simultaneous push could cause a race condition in which a .keep file exists at the moment of a repack. The repack may try to include those objects in one of two situations: 1. The pushed .keep pack contains objects that were already in the repository (e.g., blobs due to a revert of an old commit). 2. Receive-pack updates the refs, making the objects reachable, but before it removes the .keep file, the repack runs. In either case, we may prefer to duplicate some objects in the new, full pack, and let the next repack (after the .keep file is cleaned up) take care of removing them. This patch introduces an option to disable the `--honor-pack-keep` option. It is not triggered by default, even when pack.writeBitmaps is turned on, because its use depends on your overall packing strategy and use of .keep files. Note that this option just disables the pack-objects behavior. We still leave packs with a .keep in place, as we do not necessarily know that we have duplicated all of their objects. Signed-off-by: Jeff King p...@peff.net --- I added a test, too, mostly to make sure I didn't bungle the option, since it's negated from its former name. Documentation/config.txt | 5 + Documentation/git-repack.txt | 8 builtin/repack.c | 10 +- t/t7700-repack.sh| 16 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index becbade..8ad081e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2136,6 +2136,11 @@ repack.usedeltabaseoffset:: false and
Re: Nike Air Max TN
Trøst af skoen er høflighed på de teknologisk avancerede Nike Air Max Cushioning system. Denne plan er placeret under sko på er bedst til at absorbere stød genereret i virkningen. Dette er ikke overført på runner såvel som konklusion resultatet er virkelig en sko, Nike Free 5.0 Sko Billigt http://www.2014skotilbud.com/nike-free der giver en behagelig udflugt. nbsp skoen også passer meget godt mange på grund af metoden exceptionel snøring som effektivt en større, der er fremstillet i løbere. Øverst er lavet ud let og åndbar komponenter, som giver til en stor pasning af praktiske møde. Der er også fremragende trækkraft skyldige på BRS tusind ydersål. -- View this message in context: http://git.661346.n2.nabble.com/Nike-Air-Max-TN-tp7604109p7604112.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] Add docstrings for lookup_replace_object() and do_lookup_replace_object()
On 02/21/2014 07:21 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h | 16
1 file changed, 16 insertions(+)
diff --git a/cache.h b/cache.h
index dc040fb..0ecd1c8 100644
--- a/cache.h
+++ b/cache.h
@@ -788,13 +788,29 @@ static inline void *read_sha1_file(const unsigned char
*sha1, enum object_type *
{
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
}
+
+/*
+ * If a replacement for object sha1 has been set up, return the
+ * replacement object's name (replaced recursively, if necessary).
+ * The return value is either sha1 or a pointer to a
+ * permanently-allocated value. This function always respects replace
+ * references, regardless of the value of check_replace_refs.
+ */
extern const unsigned char *do_lookup_replace_object(const unsigned char
*sha1);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name. This function is similar to do_lookup_replace_object(),
+ * except that it when object replacement is suppressed, it always
+ * returns its argument unchanged.
+ */
static inline const unsigned char *lookup_replace_object(const unsigned
char *sha1)
{
if (!read_replace_refs)
return sha1;
return do_lookup_replace_object(sha1);
}
+
static inline const unsigned char *lookup_replace_object_extended(const
unsigned char *sha1, unsigned flag)
{
if (!(flag LOOKUP_REPLACE_OBJECT))
The above description is good, but after reading ecef (inline
lookup_replace_object() calls, 2011-05-15) that introduced this
ugliness, I have to wonder if do_lookup_replace(), which nobody
except lookup_replace_object() ever calls, is better removed from
the public API, making lookup_replace_object() an extern definition.
We do name functions that are purely helpers that are internal
implementation detals of the API as do_blah, but exporting that
kind of name as if that is part of the API people are expected to
call feels very wrong.
I assume that the current design was to avoid the overhead of a function
call in the case that no replace references exist. If we're willing to
eat that cost, then sure, we should bury do_lookup_replace_object() in
the implementation file.
Unless you say otherwise, I will work that change into my patch series.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
`git stash pop` UX Problem
Hi there, I'm fairly new to git and I wanted to ask about a certain behavior that I want to fix myself (if you agree with me that it is a misbehavior)... since I've never contributed to open source and it'll be an important step for me to start and get something done. In general, whenever something a user should do, git always tells. So, for example, when things go wrong with a merge, you have the option to abort. When you are doing a rebase, git tells you to do git commit --amend, and then git rebase --continue... and so on. The point is: Because of this, git is expected to always instruct you on what to do next in a multilevel operation, or instructing you what to do when an operation has gone wrong. Now comes the problem. When you do a git stash pop, and a merge conflict happens, git correctly tells you to fix the problems and then git add to resolve the conflict. But once that happens, and the internal status of git tells you that there are no more problems (I have a prompt that tells me git's internal status), the operation is not culminated by dropping the stash reference, which what normally happens automatically after a git stash pop. This has actually confused me for a lot of time, till I ran into a git committer and asked him, and only then were I 100% confident that I did nothing wrong and it is indeed a UX problem. I wasted a lot of time to know why the operation is not completed as expected (since I trusted that git just does the right thing), and it turned out that it is git's fault. If this is accepted, please reply to this email and tell me to start working on it. I've read the Documenation/SubmittingPatches guidelines, but I'll appreciate also telling me where to base my change. My guess is maint, since it's a bug in the sense of UX. Thanks and sorry for the long email. -- Best Regards, Omar Othman -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] make commit --verbose work with --no-status
On Mon, Feb 24, 2014 at 12:24:42PM +0800, Tay Ray Chuan wrote:
What happens here when there is an alternate status format _and_
--verbose is used? If I say git commit --porcelain it should imply
--dry-run. But git commit --porcelain --verbose no longer does so
after your patch.
I should have just left the --dry-run inference alone, like this.
--8--
@@ -1141,7 +1146,12 @@ static int parse_and_validate_options
if (all argc 0)
die(_(Paths with -a does not make sense.));
- if (status_format != STATUS_FORMAT_DEFAULT)
+ if (verbose !include_status) {
+ include_status = 1;
+ status_format = STATUS_FORMAT_NONE;
+ } else if (status_format != STATUS_FORMAT_DEFAULT)
dry_run = 1;
return argc;
Hrm, not quite, because the way include_status works is weird. If I turn
it off in the config, like this:
git config commit.status false
then asking explicitly for a status format should still dry-run and show
it:
git commit --porcelain
IOW, include_status is only respected when we are generating the actual
commit. So I think you need something more like:
if (status_format == STATUS_FORMAT_DEFAULT) {
if (verbose !include_status) {
include_status = 1;
status_format = STATUS_FORMAT_NONE;
}
} else
dry_run = 1;
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] Document some functions defined in object.c
Nicolas, thanks for the very fast feedback! On 02/21/2014 06:33 PM, Nicolas Pitre wrote: On Fri, 21 Feb 2014, Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Minor nits below. --- object.c | 23 ++- object.h | 7 +++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/object.c b/object.c index 584f7ac..c34e225 100644 --- a/object.c +++ b/object.c @@ -43,14 +43,26 @@ int type_from_string(const char *str) die(invalid object type \%s\, str); } +/* + * Return a numerical hash value between 0 and n-1 for the object with + * the specified sha1. n must be a power of 2. + * + * Since the sha1 is essentially random, we just take the required + * bits from the first sizeof(unsigned int) bytes of sha1. This might be improved a little. The only reason for the sizeof() is actually to copy those bits into a properly aligned integer. Some architectures have alignment restrictions that incure a significant cost when integer operations are performed on unaligned data whereas sha1 pointers don't have any particular alignment requirements. Once upon a time this used to simply be: return *(unsigned int *)sha1 (n - 1); The memcpy is there only to avoid unaligned accesses. I understand all that; it's clear that the old code is not correct C, isn't it? And ISTM that the use of memcpy() is an implementation detail that is not relevant to callers and so not needed in the docstring. So I suggest that your note be added as comments within the function; what do you think? Contrariwise, I thought about it again and believe that it *is* important for the docstring to mention explicitly that the return value is architecture-dependent (it depends on endianness and possibly sizeof(unsigned int)). Presumably the function is only used within one Git invocation, so this isn't a problem, but we should warn callers. Alternatively, we could stick a call to ntohl() in the function to make the return value consistent, but this would cost a little bit on little-endian computers. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
`git stash pop` UX Problem
Hi there, I'm fairly new to git and I wanted to ask about a certain behavior that I want to fix myself (if you agree with me that it is a misbehavior)... since I've never contributed to open source and it'll be an important step for me to start and get something done. In general, whenever something a user should do, git always tells. So, for example, when things go wrong with a merge, you have the option to abort. When you are doing a rebase, git tells you to do git commit --amend, and then git rebase --continue... and so on. The point is: Because of this, git is expected to always instruct you on what to do next in a multilevel operation, or instructing you what to do when an operation has gone wrong. Now comes the problem. When you do a git stash pop, and a merge conflict happens, git correctly tells you to fix the problems and then git add to resolve the conflict. But once that happens, and the internal status of git tells you that there are no more problems (I have a prompt that tells me git's internal status), the operation is not culminated by dropping the stash reference, which what normally happens automatically after a git stash pop. This has actually confused me for a lot of time, till I ran into a git committer and asked him, and only then were I 100% confident that I did nothing wrong and it is indeed a UX problem. I wasted a lot of time to know why the operation is not completed as expected (since I trusted that git just does the right thing), and it turned out that it is git's fault. If this is accepted, please reply to this email and tell me to start working on it. I've read the Documenation/SubmittingPatches guidelines, but I'll appreciate also telling me where to base my change. My guess is maint, since it's a bug in the sense of UX. Thanks and sorry for the long email. -- 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] remote: handle pushremote config in any order order
On Mon, Feb 24, 2014 at 12:10:04AM -0500, Jack Nagel wrote:
There seems to be a difference in the behavior of git push depending
on whether remote.pushdefault is defined before or after
branch.name.pushremote in .git/config.
[...]
I would expect the order that things are defined in the config file to
have no effect on the behavior of git push.
Yes, with a few exceptions, we usually try to make the ordering in the
config file irrelevant. This is a bug. The patch below should fix it.
-- 8 --
Subject: remote: handle pushremote config in any order
The remote we push can be defined either by
remote.pushdefault or by branch.*.pushremote for the current
branch. The order in which they appear in the config file
should not matter to precedence (which should be to prefer
the branch-specific config).
The current code parses the config linearly and uses a
single string to store both values, overwriting any
previous value. Thus, config like:
[branch master]
pushremote = foo
[remote]
pushdefault = bar
erroneously ends up pushing to bar from the master branch.
We can fix this by storing both values and resolving the
correct value after all config is read.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 7 ++-
t/t5516-fetch-push.sh | 12
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index e41251e..7232a33 100644
--- a/remote.c
+++ b/remote.c
@@ -49,6 +49,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *branch_pushremote_name;
static const char *pushremote_name;
static int explicit_default_remote_name;
@@ -352,7 +353,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
}
} else if (!strcmp(subkey, .pushremote)) {
if (branch == current_branch)
- if (git_config_string(pushremote_name, key,
value))
+ if (git_config_string(branch_pushremote_name,
key, value))
return -1;
} else if (!strcmp(subkey, .merge)) {
if (!value)
@@ -492,6 +493,10 @@ static void read_config(void)
make_branch(head_ref + strlen(refs/heads/), 0);
}
git_config(handle_config, NULL);
+ if (branch_pushremote_name) {
+ free(pushremote_name);
+ pushremote_name = branch_pushremote_name;
+ }
alias_all_urls();
}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 926e7f6..1309c4d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -536,6 +536,18 @@ test_expect_success 'push with config branch.*.pushremote'
'
check_push_result down_repo $the_commit heads/master
'
+test_expect_success 'branch.*.pushremote config order is irrelevant' '
+ mk_test one_repo heads/master
+ mk_test two_repo heads/master
+ test_config remote.one.url one_repo
+ test_config remote.two.url two_repo
+ test_config branch.master.pushremote two_repo
+ test_config remote.pushdefault one_repo
+ git push
+ check_push_result one_repo $the_first_commit heads/master
+ check_push_result two_repo $the_commit heads/master
+'
+
test_expect_success 'push with dry-run' '
mk_test testrepo heads/master
--
1.8.5.2.500.g8060133
--
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: Cygwin + git log = no pager!
On Mon, Feb 24, 2014 at 08:55:41PM +1300, Chris Packham wrote: Thanks for the response. I did set this environment variable in my .bashrc like so: export GIT_PAGER=less However after I do a 'git log' it is just spitting it out all at once and not entering the pager. Um OK that was the obvious thing to try. There is also the config variable core.pager but GIT_PAGER should take precedence. Presumably we are actually running what's in GIT_PAGER, but we can double-check with: GIT_PAGER='echo custom pager' git log You can also try: GIT_TRACE=1 git log which should describe the pager being run. Could something be setting the environment variable LESS? Reading the git-config man page for core.pager git wants to set LESS to FRSX but if it is already set git takes that as an indication that you don't want to set LESS automatically. We can also double-check the LESS setting in the executed pager: GIT_PAGER='echo LESS=$LESS' git log If we are running less, and it is using FRSX, I'd suspect some kind of terminal weirdness with less itself. With -F, less will quit if the whole output can be displayed; it's possible it thinks the screen is bigger than it is. Trying: GIT_PAGER=less LESS=RSX git log might help. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] Add docstrings for lookup_replace_object() and do_lookup_replace_object()
On Fri, Feb 21, 2014 at 5:32 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h | 16
1 file changed, 16 insertions(+)
diff --git a/cache.h b/cache.h
index dc040fb..0ecd1c8 100644
--- a/cache.h
+++ b/cache.h
@@ -788,13 +788,29 @@ static inline void *read_sha1_file(const unsigned char
*sha1, enum object_type *
{
return read_sha1_file_extended(sha1, type, size,
LOOKUP_REPLACE_OBJECT);
}
+
+/*
+ * If a replacement for object sha1 has been set up, return the
+ * replacement object's name (replaced recursively, if necessary).
+ * The return value is either sha1 or a pointer to a
+ * permanently-allocated value. This function always respects replace
+ * references, regardless of the value of check_replace_refs.
Here you talk about check_replace_refs ...
+ */
extern const unsigned char *do_lookup_replace_object(const unsigned char
*sha1);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name. This function is similar to do_lookup_replace_object(),
+ * except that it when object replacement is suppressed, it always
+ * returns its argument unchanged.
+ */
static inline const unsigned char *lookup_replace_object(const unsigned char
*sha1)
{
if (!read_replace_refs)
... but here read_replace_refs is used.
return sha1;
return do_lookup_replace_object(sha1);
}
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git gc --aggressive led to about 40 times slower git log --raw
I used to repack older part of history manually with a deeper depth, mark the result with the .keep bit, and then repack the whole thing again to have the remainder in a shallower depth. Something like: git rev-list --objects v1.5.3 | git pack-objects --depth=128 --delta-base-offset pack would give me the first pack (in real life, I would use a larger window size like 4096), and then after placing the resulting .pack and .idx files along with a .keep file in .git/objects/pack/, running git repack -a -d to pack the rest. I'm curious, after these repacking, how do you guys publish these packs? git push? if yes, on what criteria does the remote repo know which pack it should fetch? Or maybe it's only a local operation and thus you cannot do it on the remote without ssh access? Philippe -- 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/6] Add docstrings for lookup_replace_object() and do_lookup_replace_object()
On 02/24/2014 10:24 AM, Christian Couder wrote:
On Fri, Feb 21, 2014 at 5:32 PM, Michael Haggerty mhag...@alum.mit.edu
wrote:
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h | 16
1 file changed, 16 insertions(+)
diff --git a/cache.h b/cache.h
index dc040fb..0ecd1c8 100644
--- a/cache.h
+++ b/cache.h
@@ -788,13 +788,29 @@ static inline void *read_sha1_file(const unsigned char
*sha1, enum object_type *
{
return read_sha1_file_extended(sha1, type, size,
LOOKUP_REPLACE_OBJECT);
}
+
+/*
+ * If a replacement for object sha1 has been set up, return the
+ * replacement object's name (replaced recursively, if necessary).
+ * The return value is either sha1 or a pointer to a
+ * permanently-allocated value. This function always respects replace
+ * references, regardless of the value of check_replace_refs.
Here you talk about check_replace_refs ...
+ */
extern const unsigned char *do_lookup_replace_object(const unsigned char
*sha1);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name. This function is similar to do_lookup_replace_object(),
+ * except that it when object replacement is suppressed, it always
+ * returns its argument unchanged.
+ */
static inline const unsigned char *lookup_replace_object(const unsigned
char *sha1)
{
if (!read_replace_refs)
... but here read_replace_refs is used.
return sha1;
return do_lookup_replace_object(sha1);
}
You're right; thanks for noticing. I originally implemented this patch
on top of mh/replace-refs-variable-rename but then separated them after
all, in the hopes that the latter would be straightforward enough to be
merged quickly, before conflicting patch series appear.
Junio, what would be easiest for you? I suggest that I rebase this
patch series back on top of mh/replace-refs-variable-rename when re-rolling.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Supporting non-blob notes
Johan Herland jo...@herland.net wrote on 02/24/2014 02:29:10: On Wed, Feb 19, 2014 at 12:10 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Feb 18, 2014 at 9:46 PM, Johan Herland jo...@herland.net wrote: On Mon, Feb 17, 2014 at 11:48 AM, yann.dir...@bertin.fr wrote: The recent git-note -C changes commit type? thread ( http://thread.gmane.org/gmane.comp.version-control.git/241950 ) looks like a good occasion to discuss possible uses of non-blob notes. The use-case we're thinking about is the storage of testrun logs as notes (think: being able to justify that a given set of tests were successfully run on a given revision). I think this is a good use of notes, and organizing the testrun logs into a tree of files seems like a natural way to proceed. Notes from the previous attempt to store trees as notes (something to watch out maybe, when you do it again) http://article.gmane.org/gmane.comp.version-control.git/197712 Thanks for that link. It is good to see that these issues have been considered/discussed previously. Yes, it sheds some useful light on the problem, thanks. I've been thinking about this for a while now, and I find myself agreeing more and more with Junio's argument in the linked thread. I think notes are fundamentally - like file contents from Git's POV - an unstructured stream of bytes. Any real structure in a git note is imposed by the surrounding application/context, and having Git impose its own object model onto the contents of notes would likely be an unnecessary distraction. OTOH, it looks like a good idea to allow the surrounding application/context to benefit from existing infrastructure. I identified so far: (i) diffing/grepping trees (ii) efficiency of indexing through notes fanout (iii) reachability (iv) content packing In Yann's example, the testrun logs are probably best structured as a hierarchy of files, but that does not necessarily mean that they MUST be stored as a Git tree object (with accompanying sub-trees and blobs). For example, one could imagine many different solutions for storing the testrun logs: (a) Storing the logs statically on some server, and putting the corresponding URL in a notes blob. Reachability is manual/on-demand (be retrieving the URL). Would require to redo (ii) and (iv) in a way that does not impait (i) (b) Storing the logs in a .tar.gz archive, and adding that archive as a blob note. Reachability is implicit/automatic (by unpacking the archive). Interferes with (i) and (iv), ie. does not allow to benefit from similarity between the contents of (unpacked) notes. (c) Storing the logs on some ref in an external repo, and putting the repo URL + ref in a notes blob. Reachability is manual/on-demand (by cloning/fetching the repo). (d) Storing the logs on some ref/commit in the same repo, and putting the ref/commit name in a notes blob. Reachability depends on the application/user to sync the ref/commit along with the notes. Better than (a), but still does not address (ii). And indeed, my intent was to let the notes live in a separate fork repo, so ordinary users need not fetch the testrun contents systematically with the code. (e) Storing the logs in a commit, putting the commit name in a blob note, and then creating/rewriting the notes history to include the commit in its ancestry. Reachability is automatic (i.e.follows the notes), but the application must control/manipulate the notes history. And finally, that one does address all points in my case. Whichever of these (or other) solutions is most appropriate depends on the particular application/context, and (from Git's perspective), none of them are inherently superior to any of the other. Even the question of whether testrun logs should or should not be reachable by default, depends on the surrounding application/context. Wouldn't it make sense to mention these possibilities in the git-notes manpage, to help people use the mechanism as intended ? Now, the intention of Yann's RFC is to store the testrun logs directly in a notes _tree_. This is not too different from alternative (e) above, in that reachability is automatic. However, instead of having the surrounding application manipulate the notes history to ensure reachability, the RFC would rather teach Git's notes code to accomodate the (likely rather special) case of having a note that is BOTH structured like (or at least easily mapped to) a Git tree object, AND that should be automatically reachable. Incidently, proposal (e) would allow the use of commits, although doing so would probably cause problems, not all of the children of the commit used as annotation having the same relationship to their parent. Are you suggesting using a slightly different mechanism than the parent relationship ? Even though there is a certain elegance to storing such a tree object
Re: [RFC/PATCH] Supporting non-blob notes
On Mon, Feb 24, 2014 at 11:27 AM, ydir...@free.fr wrote: Johan Herland jo...@herland.net wrote on 02/24/2014 02:29:10: I've been thinking about this for a while now, and I find myself agreeing more and more with Junio's argument in the linked thread. I think notes are fundamentally - like file contents from Git's POV - an unstructured stream of bytes. Any real structure in a git note is imposed by the surrounding application/context, and having Git impose its own object model onto the contents of notes would likely be an unnecessary distraction. OTOH, it looks like a good idea to allow the surrounding application/context to benefit from existing infrastructure. I identified so far: (i) diffing/grepping trees (ii) efficiency of indexing through notes fanout All of my proposed alternatives store some sort of reference to the real data in a notes object; even when using a tree object directly as a note, the notes tree itself only stores a SHA1 reference to the tree object. As such, all alternatives (a) through (e) (even including your RFC) benefit from indexing through the notes fanout, and I'm not sure what is gained by attaching the real data more directly to the notes. In all of (a) through (e), the lookup of a specific commit's testrun logs always start with doing a lookup of the notes associated with a given commit. Once that is done, the remainder of the work is about resolving that reference and retrieving the associated resource, Whether the consists of loading an HTTP URL, fetching a remote Git repo, or looking up a local tree object is ultimately an implementation detail, and does not affect the indexing itself. (iii) reachability (iv) content packing These four criteria/requirements apply to your specific use case, but they do not necessarily apply to _all_ use cases. I can easily imagine a slightly different scenario: For example, a company setting with highly-available internal servers, and where testrun logs are primarily interesting to a small subset of users (e.g. most developers only look at them very occasionally). Now assume there is already a (third-party) system in place for archiving and indexing the testrun logs (i.e. providing (i), (ii) and (iv)), and direct reachability (iii) is not desired as including the testrun logs in the repo would add nothing but bloat for most users. In this scenario, simply adding a note with the appropriate URL to the third-party service would be a sufficient and preferable solution. In Yann's example, the testrun logs are probably best structured as a hierarchy of files, but that does not necessarily mean that they MUST be stored as a Git tree object (with accompanying sub-trees and blobs). For example, one could imagine many different solutions for storing the testrun logs: (a) Storing the logs statically on some server, and putting the corresponding URL in a notes blob. Reachability is manual/on-demand (be retrieving the URL). Would require to redo (ii) and (iv) in a way that does not impait (i) (b) Storing the logs in a .tar.gz archive, and adding that archive as a blob note. Reachability is implicit/automatic (by unpacking the archive). Interferes with (i) and (iv), ie. does not allow to benefit from similarity between the contents of (unpacked) notes. (c) Storing the logs on some ref in an external repo, and putting the repo URL + ref in a notes blob. Reachability is manual/on-demand (by cloning/fetching the repo). (d) Storing the logs on some ref/commit in the same repo, and putting the ref/commit name in a notes blob. Reachability depends on the application/user to sync the ref/commit along with the notes. Better than (a), but still does not address (ii). And indeed, my intent was to let the notes live in a separate fork repo, so ordinary users need not fetch the testrun contents systematically with the code. Just to clarify, my alternatives (except for (e) below) were not intended to satisfy the exact criteria for your use case, but only to demonstrate that there exist a variety of solutions for a variety of slightly different problems. When we consider adding significant complexity to the notes code, we must justify that with real and tangible benefits, not only for your exact use case, but preferably also for a larger group of related use cases. So far I don't see how allowing the direct use of tree objects as notes benefit more than your specific use case... (e) Storing the logs in a commit, putting the commit name in a blob note, and then creating/rewriting the notes history to include the commit in its ancestry. Reachability is automatic (i.e.follows the notes), but the application must control/manipulate the notes history. And finally, that one does address all points in my case. Whichever of these (or other) solutions is most appropriate depends on the particular application/context, and (from Git's perspective), none of them are inherently superior to any of the other. Even the
Re: `git stash pop` UX Problem
Omar:
On Mon, Feb 24, 2014 at 3:32 AM, Omar Othman omar.oth...@booking.com wrote:
In general, whenever something a user should do, git always tells. So, for
example, when things go wrong with a merge, you have the option to abort.
When you are doing a rebase, git tells you to do git commit --amend, and
then git rebase --continue... and so on.
The point is: Because of this, git is expected to always instruct you on
what to do next in a multilevel operation, or instructing you what to do
when an operation has gone wrong.
Now comes the problem. When you do a git stash pop, and a merge conflict
happens, git correctly tells you to fix the problems and then git add to
resolve the conflict. But once that happens, and the internal status of git
tells you that there are no more problems (I have a prompt that tells me
git's internal status), the operation is not culminated by dropping the
stash reference, which what normally happens automatically after a git stash
pop. This has actually confused me for a lot of time, till I ran into a git
committer and asked him, and only then were I 100% confident that I did
nothing wrong and it is indeed a UX problem. I wasted a lot of time to know
why the operation is not completed as expected (since I trusted that git
just does the right thing), and it turned out that it is git's fault.
If this is accepted, please reply to this email and tell me to start working
on it. I've read the Documenation/SubmittingPatches guidelines, but I'll
appreciate also telling me where to base my change. My guess is maint, since
it's a bug in the sense of UX.
Unlike a merge, when you pop a stash that history is lost. If you
screw up the merge and the stash is dropped then there's generally no
reliable way to get it back. I think that it's correct behavior for
the stash to not be dropped if the merge conflicts. The user is
expected to manually drop the stash when they're done with it. It's
been a while since I've relied much on the stash (commits and branches
are more powerful to work with) so I'm not really familiar with what
help the UI gives when a conflict occurs now. Git's UI never really
expects the user to be negligent. It does help to hint to you what is
needed, but for the most part it still expects you to know what you're
doing and does what you say, not what you mean.
If there's any change that should be made it should be purely
providing more detailed instructions to the user about how to deal
with it. Either resolve the merge conflicts and git-add the
conflicting files, or use git-reset to either reset the index
(unstaging files nad clear) or reset index and working tree back to
HEAD. In general, I almost always git-reset after a git-stash pop
because I'm probably not ready to commit those changes yet and
generally want to still see those changes with git diff (without
--staged). Or perhaps just direct them to the appropriate sections of
the man pages.
I'm not really in favor of dumbing down Git in any way and I think
that any step in that direction would be for the worst... Software
should do what you say, not what you mean, because it's impossible to
reliably guess what you meant. When a git-stash pop operation fails
that might make the user rethink popping that stash. That's why it
becomes a manual operation to drop it if still desired. And unlike
git-reset --continue, which is explicitly the user saying it is fixed
and I accept the consequences, let's move on, there is no such option
to git-stash to acknowledge that the merge conflicts have been
resolved and you no longer need that stash (aside from git-stash drop,
of course). It's not a UI problem. It's maybe a documentation problem,
but again I'm not familiar with the current state of that.
/not a git dev...yet
Regards,
--
Brandon McCaig bamcc...@gmail.com bamcc...@castopulence.org
Castopulence Software https://www.castopulence.org/
Blog http://www.bamccaig.com/
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
--
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 02/19] combine-diff: move changed-paths scanning logic into its own function
Move code for finding paths for which diff(commit,parent_i) is not-empty
for all parents to separate function - at present we have generic (and
slow) code for this job, which translates 1 n-parent problem to n
1-parent problems and then intersect results, and will be adding another
limited, but faster, paths scanning implementation in the next patch.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
combine-diff.c | 80 ++
1 file changed, 53 insertions(+), 27 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 68d2e53..1732dfd 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1301,6 +1301,51 @@ static const char *path_path(void *obj)
return path-path;
}
+
+/* find set of paths that every parent touches */
+static struct combine_diff_path *find_paths(const unsigned char *sha1,
+ const struct sha1_array *parents, struct diff_options *opt)
+{
+ struct combine_diff_path *paths = NULL;
+ int i, num_parent = parents-nr;
+
+ int output_format = opt-output_format;
+ const char *orderfile = opt-orderfile;
+
+ opt-output_format = DIFF_FORMAT_NO_OUTPUT;
+ /* tell diff_tree to emit paths in sorted (=tree) order */
+ opt-orderfile = NULL;
+
+ for (i = 0; i num_parent; i++) {
+ /*
+* show stat against the first parent even when doing
+* combined diff.
+*/
+ int stat_opt = (output_format
+ (DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT));
+ if (i == 0 stat_opt)
+ opt-output_format = stat_opt;
+ else
+ opt-output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_tree_sha1(parents-sha1[i], sha1, , opt);
+ diffcore_std(opt);
+ paths = intersect_paths(paths, i, num_parent);
+
+ /* if showing diff, show it in requested order */
+ if (opt-output_format != DIFF_FORMAT_NO_OUTPUT
+ orderfile) {
+ diffcore_order(orderfile);
+ }
+
+ diff_flush(opt);
+ }
+
+ opt-output_format = output_format;
+ opt-orderfile = orderfile;
+ return paths;
+}
+
+
void diff_tree_combined(const unsigned char *sha1,
const struct sha1_array *parents,
int dense,
@@ -1308,7 +1353,7 @@ void diff_tree_combined(const unsigned char *sha1,
{
struct diff_options *opt = rev-diffopt;
struct diff_options diffopts;
- struct combine_diff_path *p, *paths = NULL;
+ struct combine_diff_path *p, *paths;
int i, num_paths, needsep, show_log_first, num_parent = parents-nr;
/* nothing to do, if no parents */
@@ -1327,35 +1372,16 @@ void diff_tree_combined(const unsigned char *sha1,
diffopts = *opt;
copy_pathspec(diffopts.pathspec, opt-pathspec);
- diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
DIFF_OPT_SET(diffopts, RECURSIVE);
DIFF_OPT_CLR(diffopts, ALLOW_EXTERNAL);
- /* tell diff_tree to emit paths in sorted (=tree) order */
- diffopts.orderfile = NULL;
- /* find set of paths that everybody touches */
- for (i = 0; i num_parent; i++) {
- /* show stat against the first parent even
-* when doing combined diff.
-*/
- int stat_opt = (opt-output_format
- (DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT));
- if (i == 0 stat_opt)
- diffopts.output_format = stat_opt;
- else
- diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
- diff_tree_sha1(parents-sha1[i], sha1, , diffopts);
- diffcore_std(diffopts);
- paths = intersect_paths(paths, i, num_parent);
-
- /* if showing diff, show it in requested order */
- if (diffopts.output_format != DIFF_FORMAT_NO_OUTPUT
- opt-orderfile) {
- diffcore_order(opt-orderfile);
- }
-
- diff_flush(diffopts);
- }
+ /* find set of paths that everybody touches
+*
+* NOTE find_paths() also handles --stat, as it computes
+* diff(sha1,parent_i) for all i to do the job, specifically
+* for parent0.
+*/
+ paths = find_paths(sha1, parents, diffopts);
/* find out number of surviving paths */
for (num_paths = 0, p = paths; p; p = p-next)
--
1.9.rc1.181.g641f458
--
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 00/19] Multiparent diff tree-walker + combine-diff speedup
Hello up there.
Here go combine-diff speedup patches in form of first reworking diff
tree-walker to work in general case - when a commit have several parents, not
only one - we are traversing all 1+nparent trees in parallel.
Then we are taking advantage of the new diff tree-walker for speeding up
combine-diff, which for linux.git results in ~14 times speedup.
This is the second posting for the whole series - sent here patches should go
instead of already-in-pu ks/diff-tree-more and ks/tree-diff-nway into
ks/tree-diff-nway - patches are related and seeing them all at once is more
logical to me.
I've tried to do my homework based on review feedback and the changes compared
to v1 are:
- fixed last-minute thinko/bug last time introduced on my side (sorry) with
opt-pathchange manipulation in __diff_tree_sha1() - we were forgetting to
restore opt-pathchange, which led to incorrect log -c (merges _and_ plain
diff-tree) output;
This time, I've verified several times, log output stays really the same.
- direct use of alloca() changed to portability wrappers xalloca/xalloca_free
which gracefully degrade to xmalloc/free on systems, where alloca is not
available (see new patch 17).
- i = 0; do { ... } while (++i nparent) is back to usual looping
for (i = 0; i nparent; ++), as I've re-measured timings and the
difference is negligible.
( Initially, when I was fighting for every cycle it made sense, but real
no-slowdown turned out to be related to avoiding mallocs, load trees in
correct
order and reducing register pressure. )
- S_IFXMIN_NEQ definition moved out to cache.h, to have all modes registry in
one place;
- diff_tree() becomes static (new patch 13), as nobody is using it outside
tree-diff.c (and is later renamed to __diff_tree_sha1);
- p0 - first_parent; corrected comments about how emit_diff_first_parent_only
behaves;
not changed:
- low-level helpers are still named with __ prefix as, imho, that is the best
convention to name such helpers, without sacrificing signal/noise ratio. All
of them are now static though.
Signoffs were left intact, if a patch was already applied to pu with one, and
had not changed.
Please apply and thanks,
Kirill
P.S. Sorry for the delay - I was very busy.
Kirill Smelkov (19):
combine-diff: move show_log_first logic/action out of paths scanning
combine-diff: move changed-paths scanning logic into its own function
tree-diff: no need to manually verify that there is no mode change for a path
tree-diff: no need to pass match to skip_uninteresting()
tree-diff: show_tree() is not needed
tree-diff: consolidate code for emitting diffs and recursion in one place
tree-diff: don't assume compare_tree_entry() returns -1,0,1
tree-diff: move all action-taking code out of compare_tree_entry()
tree-diff: rename compare_tree_entry - tree_entry_pathcmp
tree-diff: show_path prototype is not needed anymore
tree-diff: simplify tree_entry_pathcmp
tree-diff: remove special-case diff-emitting code for empty-tree cases
tree-diff: diff_tree() should now be static
tree-diff: rework diff_tree interface to be sha1 based
tree-diff: no need to call full diff_tree_sha1 from show_path()
tree-diff: reuse base str(buf) memory on sub-tree recursion
Portable alloca for Git
tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
combine-diff: speed it up, by using multiparent diff tree-walker directly
Makefile | 6 +
cache.h | 15 ++
combine-diff.c| 170 +++---
config.mak.uname | 10 +-
configure.ac | 8 +
diff.c| 2 +
diff.h| 12 +-
git-compat-util.h | 8 +
tree-diff.c | 666 +++---
9 files changed, 724 insertions(+), 173 deletions(-)
--
1.9.rc1.181.g641f458
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/19] combine-diff: move show_log_first logic/action out of paths scanning
Judging from sample outputs and tests nothing changes in diff -c output,
and this change will help later patches, when we'll be refactoring paths
scanning into its own function with several variants - the
show_log_first logic / code will stay common to all of them.
NOTE: only now we have to take care to explicitly not show anything if
parents array is empty, as in fact there are some clients in Git code,
which calls diff_tree_combined() in such a way.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
combine-diff.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 24ca7e2..68d2e53 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1311,6 +1311,20 @@ void diff_tree_combined(const unsigned char *sha1,
struct combine_diff_path *p, *paths = NULL;
int i, num_paths, needsep, show_log_first, num_parent = parents-nr;
+ /* nothing to do, if no parents */
+ if (!num_parent)
+ return;
+
+ show_log_first = !!rev-loginfo !rev-no_commit_id;
+ needsep = 0;
+ if (show_log_first) {
+ show_log(rev);
+
+ if (rev-verbose_header opt-output_format)
+ printf(%s%c, diff_line_prefix(opt),
+ opt-line_termination);
+ }
+
diffopts = *opt;
copy_pathspec(diffopts.pathspec, opt-pathspec);
diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -1319,8 +1333,6 @@ void diff_tree_combined(const unsigned char *sha1,
/* tell diff_tree to emit paths in sorted (=tree) order */
diffopts.orderfile = NULL;
- show_log_first = !!rev-loginfo !rev-no_commit_id;
- needsep = 0;
/* find set of paths that everybody touches */
for (i = 0; i num_parent; i++) {
/* show stat against the first parent even
@@ -1336,14 +1348,6 @@ void diff_tree_combined(const unsigned char *sha1,
diffcore_std(diffopts);
paths = intersect_paths(paths, i, num_parent);
- if (show_log_first i == 0) {
- show_log(rev);
-
- if (rev-verbose_header opt-output_format)
- printf(%s%c, diff_line_prefix(opt),
- opt-line_termination);
- }
-
/* if showing diff, show it in requested order */
if (diffopts.output_format != DIFF_FORMAT_NO_OUTPUT
opt-orderfile) {
--
1.9.rc1.181.g641f458
--
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 13/19] tree-diff: diff_tree() should now be static
We reworked all its users to use the functionality through
diff_tree_sha1 variant in recent patches (see tree-diff: allow
diff_tree_sha1 to accept NULL sha1 and what comes next).
diff_tree() is now not used outside tree-diff.c - make it static.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
( new patch )
diff.h | 2 --
tree-diff.c | 4 ++--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/diff.h b/diff.h
index e79f3b3..5d7b9f7 100644
--- a/diff.h
+++ b/diff.h
@@ -189,8 +189,6 @@ const char *diff_line_prefix(struct diff_options *);
extern const char mime_boundary_leader[];
-extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
-const char *base, struct diff_options *opt);
extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
const char *base, struct diff_options *opt);
extern int diff_root_tree_sha1(const unsigned char *new, const char *base,
diff --git a/tree-diff.c b/tree-diff.c
index 2fd6d0e..b99622c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -137,8 +137,8 @@ static void skip_uninteresting(struct tree_desc *t, struct
strbuf *base,
}
}
-int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
- const char *base_str, struct diff_options *opt)
+static int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
+const char *base_str, struct diff_options *opt)
{
struct strbuf base;
int baselen = strlen(base_str);
--
1.9.rc1.181.g641f458
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/19] tree-diff: rename compare_tree_entry - tree_entry_pathcmp
Since previous commit, this function does not compare entry hashes, and
mode are compared fully outside of it. So what it does is compare entry
names and DIR bit in modes. Reflect this in its name.
Add documentation stating the semantics, and move the note about
files/dirs comparison to it.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 6207372..3345534 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -9,7 +9,14 @@
static void show_path(struct strbuf *base, struct diff_options *opt,
struct tree_desc *t1, struct tree_desc *t2);
-static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2)
+/*
+ * Compare two tree entries, taking into account only path/S_ISDIR(mode),
+ * but not their sha1's.
+ *
+ * NOTE files and directories *always* compare differently, even when having
+ * the same name - thanks to base_name_compare().
+ */
+static int tree_entry_pathcmp(struct tree_desc *t1, struct tree_desc *t2)
{
unsigned mode1, mode2;
const char *path1, *path2;
@@ -22,10 +29,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2)
pathlen1 = tree_entry_len(t1-entry);
pathlen2 = tree_entry_len(t2-entry);
- /*
-* NOTE files and directories *always* compare differently,
-* even when having the same name.
-*/
cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
return cmp;
}
@@ -169,7 +172,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
continue;
}
- cmp = compare_tree_entry(t1, t2);
+ cmp = tree_entry_pathcmp(t1, t2);
/* t1 = t2 */
if (cmp == 0) {
--
1.9.rc1.181.g641f458
--
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 11/19] tree-diff: simplify tree_entry_pathcmp
Since an earlier Finally switch over tree descriptors to contain a
pre-parsed entry, we can safely access all tree_desc-entry fields
directly instead of first extracting them through
tree_entry_extract.
Use it. The code generated stays the same - only it now visually looks
cleaner.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 20a4fda..cf96ad7 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -15,18 +15,13 @@
*/
static int tree_entry_pathcmp(struct tree_desc *t1, struct tree_desc *t2)
{
- unsigned mode1, mode2;
- const char *path1, *path2;
- const unsigned char *sha1, *sha2;
- int cmp, pathlen1, pathlen2;
+ struct name_entry *e1, *e2;
+ int cmp;
- sha1 = tree_entry_extract(t1, path1, mode1);
- sha2 = tree_entry_extract(t2, path2, mode2);
-
- pathlen1 = tree_entry_len(t1-entry);
- pathlen2 = tree_entry_len(t2-entry);
-
- cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
+ e1 = t1-entry;
+ e2 = t2-entry;
+ cmp = base_name_compare(e1-path, tree_entry_len(e1), e1-mode,
+ e2-path, tree_entry_len(e2), e2-mode);
return cmp;
}
--
1.9.rc1.181.g641f458
--
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 06/19] tree-diff: consolidate code for emitting diffs and recursion in one place
Currently both compare_tree_entry() and show_path() invoke opt diff
callbacks (opt-add_remove() and opt-change()), and also they both have
code which decides whether to recurse into sub-tree, and whether to emit
a tree as separate entry if DIFF_OPT_TREE_IN_RECURSIVE is set.
I.e. we have code duplication and logic scattered on two places.
Let's consolidate it - all diff emmiting code and recurion logic moves
to show_entry, which is now named as show_path, because it shows diff
for a path, based on up to two tree entries, with actual diff emitting
code being kept in new helper emit_diff() for clarity.
What we have as the result, is that compare_tree_entry is now free from
code with logic for diff generation, and also performance is not
affected as timings for
`git log --raw --no-abbrev --no-renames`
for navy.git and `linux.git v3.10..v3.11`, just like in previous patch,
stay the same.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 115
1 file changed, 84 insertions(+), 31 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 2ad7788..a5b9ff9 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -6,8 +6,8 @@
#include diffcore.h
#include tree.h
-static void show_entry(struct diff_options *opt, const char *prefix,
- struct tree_desc *desc, struct strbuf *base);
+static void show_path(struct strbuf *base, struct diff_options *opt,
+ struct tree_desc *t1, struct tree_desc *t2);
static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
struct strbuf *base, struct diff_options *opt)
@@ -16,7 +16,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
const char *path1, *path2;
const unsigned char *sha1, *sha2;
int cmp, pathlen1, pathlen2;
- int old_baselen = base-len;
sha1 = tree_entry_extract(t1, path1, mode1);
sha2 = tree_entry_extract(t2, path2, mode2);
@@ -30,51 +29,105 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
*/
cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
if (cmp 0) {
- show_entry(opt, -, t1, base);
+ show_path(base, opt, t1, /*t2=*/NULL);
return -1;
}
if (cmp 0) {
- show_entry(opt, +, t2, base);
+ show_path(base, opt, /*t1=*/NULL, t2);
return 1;
}
if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) !hashcmp(sha1, sha2)
mode1 == mode2)
return 0;
- strbuf_add(base, path1, pathlen1);
- if (DIFF_OPT_TST(opt, RECURSIVE) S_ISDIR(mode1)) {
- if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
- opt-change(opt, mode1, mode2,
- sha1, sha2, 1, 1, base-buf, 0, 0);
- }
- strbuf_addch(base, '/');
- diff_tree_sha1(sha1, sha2, base-buf, opt);
- } else {
- opt-change(opt, mode1, mode2, sha1, sha2, 1, 1, base-buf, 0,
0);
- }
- strbuf_setlen(base, old_baselen);
+ show_path(base, opt, t1, t2);
return 0;
}
-/* An entry went away or appeared */
-static void show_entry(struct diff_options *opt, const char *prefix,
- struct tree_desc *desc, struct strbuf *base)
+
+/* convert path, t1/t2 - opt-diff_*() callbacks */
+static void emit_diff(struct diff_options *opt, struct strbuf *path,
+ struct tree_desc *t1, struct tree_desc *t2)
+{
+ unsigned int mode1 = t1 ? t1-entry.mode : 0;
+ unsigned int mode2 = t2 ? t2-entry.mode : 0;
+
+ if (mode1 mode2) {
+ opt-change(opt, mode1, mode2, t1-entry.sha1, t2-entry.sha1,
+ 1, 1, path-buf, 0, 0);
+ }
+ else {
+ const unsigned char *sha1;
+ unsigned int mode;
+ int addremove;
+
+ if (mode2) {
+ addremove = '+';
+ sha1 = t2-entry.sha1;
+ mode = mode2;
+ }
+ else {
+ addremove = '-';
+ sha1 = t1-entry.sha1;
+ mode = mode1;
+ }
+
+ opt-add_remove(opt, addremove, mode, sha1, 1, path-buf, 0);
+ }
+}
+
+
+/* new path should be added to diff
+ *
+ * 3 cases on how/when it should be called and behaves:
+ *
+ * !t1, t2- path added, parent lacks it
+ * t1, !t2- path removed from parent
+ * t1, t2- path modified
+ */
+static void show_path(struct strbuf *base, struct diff_options *opt,
+ struct tree_desc *t1, struct tree_desc *t2)
{
unsigned mode;
const char *path;
-
[PATCH 12/19] tree-diff: remove special-case diff-emitting code for empty-tree cases
via teaching tree_entry_pathcmp() how to compare empty tree descriptors:
While walking trees, we iterate their entries from lowest to highest in
sort order, so empty tree means all entries were already went over.
If we artificially assign +infinity value to such tree entry, it will
go after all usual entries, and through the usual driver loop we will be
taking the same actions, which were hand-coded for special cases, i.e.
t1 empty, t2 non-empty
pathcmp(+∞, t2) - +1
show_path(/*t1=*/NULL, t2); /* = t1 t2 case in main loop */
t1 non-empty, t2-empty
pathcmp(t1, +∞) - -1
show_path(t1, /*t2=*/NULL); /* = t1 t2 case in main loop */
Right now we never go to when compared tree descriptors are infinity, as
this condition is checked in the loop beginning as finishing criteria,
but will do in the future, when there will be several parents iterated
simultaneously, and some pair of them would run to the end.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index cf96ad7..2fd6d0e 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -12,12 +12,19 @@
*
* NOTE files and directories *always* compare differently, even when having
* the same name - thanks to base_name_compare().
+ *
+ * NOTE empty (=invalid) descriptor(s) take part in comparison as +infty.
*/
static int tree_entry_pathcmp(struct tree_desc *t1, struct tree_desc *t2)
{
struct name_entry *e1, *e2;
int cmp;
+ if (!t1-size)
+ return t2-size ? +1 /* +∞ c */ : 0 /* +∞ = +∞ */;
+ else if (!t2-size)
+ return -1; /* c +∞ */
+
e1 = t1-entry;
e2 = t2-entry;
cmp = base_name_compare(e1-path, tree_entry_len(e1), e1-mode,
@@ -151,18 +158,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
skip_uninteresting(t1, base, opt);
skip_uninteresting(t2, base, opt);
}
- if (!t1-size) {
- if (!t2-size)
- break;
- show_path(base, opt, /*t1=*/NULL, t2);
- update_tree_entry(t2);
- continue;
- }
- if (!t2-size) {
- show_path(base, opt, t1, /*t2=*/NULL);
- update_tree_entry(t1);
- continue;
- }
+ if (!t1-size !t2-size)
+ break;
cmp = tree_entry_pathcmp(t1, t2);
--
1.9.rc1.181.g641f458
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Brandon McCaig bamcc...@gmail.com writes: Unlike a merge, when you pop a stash that history is lost. If you screw up the merge and the stash is dropped then there's generally no reliable way to get it back. I think that it's correct behavior for the stash to not be dropped if the merge conflicts. Agreed. If there's any change that should be made it should be purely providing more detailed instructions to the user about how to deal with it. Yes, there may be room for improvement, but that does not seem so easy. Today, we have: $ git stash pop Auto-merging foo.txt CONFLICT (content): Merge conflict in foo.txt $ git status On branch master Unmerged paths: (use git reset HEAD file... to unstage) (use git add file... to mark resolution) both modified: foo.txt = The advices shown here are OK. Then: $ git add foo.txt $ git status On branch master Changes to be committed: (use git reset HEAD file... to unstage) modified: foo.txt = here, git status could have hinted the user you may now run 'git stash drop' if you are satisfied with your merge. An obvious issue is that at this point Git has no way to know that you just did a git stash pop. But that could be solved by leaving a file around like .git/stash-pop-ongoing. Now, the real question is: when would Git stop showing this advice. I don't see a real way to answer this, and I'd rather avoid doing just a guess. One easy thing to do OTOH would be to show a hint at the end of git stash pop's output, like $ git stash pop Auto-merging foo.txt CONFLICT (content): Merge conflict in foo.txt 'stash pop' failed. Please, resolve the conflicts manually. The stash was not dropped in case you need to restart the operation. When you are done resolving the merge, you may run the following to drop the stash: git stash drop or so (I couldn't find a concise yet accurate wording). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 04/19] tree-diff: no need to pass match to skip_uninteresting()
It is neither used there as input, nor the output written through it, is
used outside.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 5810b00..a8c2aec 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -109,13 +109,14 @@ static void show_entry(struct diff_options *opt, const
char *prefix,
}
static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
- struct diff_options *opt,
- enum interesting *match)
+ struct diff_options *opt)
{
+ enum interesting match;
+
while (t-size) {
- *match = tree_entry_interesting(t-entry, base, 0,
opt-pathspec);
- if (*match) {
- if (*match == all_entries_not_interesting)
+ match = tree_entry_interesting(t-entry, base, 0,
opt-pathspec);
+ if (match) {
+ if (match == all_entries_not_interesting)
t-size = 0;
break;
}
@@ -128,8 +129,6 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
{
struct strbuf base;
int baselen = strlen(base_str);
- enum interesting t1_match = entry_not_interesting;
- enum interesting t2_match = entry_not_interesting;
/* Enable recursion indefinitely */
opt-pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
@@ -141,8 +140,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
if (diff_can_quit_early(opt))
break;
if (opt-pathspec.nr) {
- skip_uninteresting(t1, base, opt, t1_match);
- skip_uninteresting(t2, base, opt, t2_match);
+ skip_uninteresting(t1, base, opt);
+ skip_uninteresting(t2, base, opt);
}
if (!t1-size) {
if (!t2-size)
--
1.9.rc1.181.g641f458
--
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 19/19] combine-diff: speed it up, by using multiparent diff tree-walker directly
As was recently shown in combine-diff: optimize
combine_diff_path sets intersection, combine-diff runs very slowly. In
that commit we optimized paths sets intersection, but that accounted
only for ~ 25% of the slowness, and as my tracing showed, for linux.git
v3.10..v3.11, for merges a lot of time is spent computing
diff(commit,commit^2) just to only then intersect that huge diff to
almost small set of files from diff(commit,commit^1).
In previous commit, we described the problem in more details, and
reworked the diff tree-walker to be general one - i.e. to work in
multiple parent case too. Now is the time to take advantage of it for
finding paths for combine diff.
The implementation is straightforward - if we know, we can get generated
diff paths directly, and at present that means no diff filtering or
rename/copy detection was requested(*), we can call multiparent tree-walker
directly and get ready paths.
(*) because e.g. at present, all diffcore transformations work on
diff_filepair queues, but in the future, that limitation can be
lifted, if filters would operate directly on combine_diff_paths.
Timings for `git log --raw --no-abbrev --no-renames` without `-c` (git log)
and with `-c` (git log -c) and with `-c --merges` (git log -c --merges)
before and after the patch are as follows:
linux.git v3.10..v3.11
log log -c log -c --merges
before 1.9s16.4s 15.2s
after 1.9s 2.4s 1.1s
The result stayed the same.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
combine-diff.c | 88 ++
diff.c | 1 +
2 files changed, 84 insertions(+), 5 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 1732dfd..12764fb 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1303,7 +1303,7 @@ static const char *path_path(void *obj)
/* find set of paths that every parent touches */
-static struct combine_diff_path *find_paths(const unsigned char *sha1,
+static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,
const struct sha1_array *parents, struct diff_options *opt)
{
struct combine_diff_path *paths = NULL;
@@ -1316,6 +1316,7 @@ static struct combine_diff_path *find_paths(const
unsigned char *sha1,
/* tell diff_tree to emit paths in sorted (=tree) order */
opt-orderfile = NULL;
+ /* D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn) (wrt paths) */
for (i = 0; i num_parent; i++) {
/*
* show stat against the first parent even when doing
@@ -1346,6 +1347,35 @@ static struct combine_diff_path *find_paths(const
unsigned char *sha1,
}
+/*
+ * find set of paths that everybody touches, assuming diff is run without
+ * rename/copy detection, etc, comparing all trees simultaneously (= faster).
+ */
+static struct combine_diff_path *find_paths_multitree(
+ const unsigned char *sha1, const struct sha1_array *parents,
+ struct diff_options *opt)
+{
+ int i, nparent = parents-nr;
+ const unsigned char **parents_sha1;
+ struct combine_diff_path paths_head;
+ struct strbuf base;
+
+ parents_sha1 = xmalloc(nparent * sizeof(parents_sha1[0]));
+ for (i = 0; i nparent; i++)
+ parents_sha1[i] = parents-sha1[i];
+
+ /* fake list head, so worker can assume it is non-NULL */
+ paths_head.next = NULL;
+
+ strbuf_init(base, PATH_MAX);
+ diff_tree_paths(paths_head, sha1, parents_sha1, nparent, base, opt);
+
+ strbuf_release(base);
+ free(parents_sha1);
+ return paths_head.next;
+}
+
+
void diff_tree_combined(const unsigned char *sha1,
const struct sha1_array *parents,
int dense,
@@ -1355,6 +1385,7 @@ void diff_tree_combined(const unsigned char *sha1,
struct diff_options diffopts;
struct combine_diff_path *p, *paths;
int i, num_paths, needsep, show_log_first, num_parent = parents-nr;
+ int need_generic_pathscan;
/* nothing to do, if no parents */
if (!num_parent)
@@ -1377,11 +1408,58 @@ void diff_tree_combined(const unsigned char *sha1,
/* find set of paths that everybody touches
*
-* NOTE find_paths() also handles --stat, as it computes
-* diff(sha1,parent_i) for all i to do the job, specifically
-* for parent0.
+* NOTE
+*
+* Diffcore transformations are bound to diff_filespec and logic
+* comparing two entries - i.e. they do not apply directly to combine
+* diff.
+*
+* If some of such transformations is requested - we launch generic
+* path scanning, which works significantly slower compared to
+* simultaneous all-trees-in-one-go scan in find_paths_multitree().
+*
+* TODO some of the
[PATCH 15/19] tree-diff: no need to call full diff_tree_sha1 from show_path()
As described in previous commit, when recursing into sub-trees, we can
use lower-level tree walker, since its interface is now sha1 based.
The change is ok, because diff_tree_sha1() only invokes
__diff_tree_sha1(), and also, if base is empty, try_to_follow_renames().
But base is not empty here, as we have added a path and '/' before
recursing.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index f90acf5..aea0297 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -114,8 +114,8 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
- diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
- t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+ __diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
+t2 ? t2-entry.sha1 : NULL, base-buf, opt);
}
strbuf_setlen(base, old_baselen);
--
1.9.rc1.181.g641f458
--
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 17/19] Portable alloca for Git
In the next patch we'll have to use alloca() for performance reasons,
but since alloca is non-standardized and is not portable, let's have a
trick with compatibility wrappers:
1. at configure time, determine, do we have working alloca() through
alloca.h, and define
#define HAVE_ALLOCA_H
if yes.
2. in code
#ifdef HAVE_ALLOCA_H
# include alloca.h
# define xalloca(size) (alloca(size))
# define xalloca_free(p)do {} while(0)
#else
# define xalloca(size) (xmalloc(size))
# define xalloca_free(p)(free(p))
#endif
and use it like
func() {
p = xalloca(size);
...
xalloca_free(p);
}
This way, for systems, where alloca is available, we'll have optimal
on-stack allocations with fast executions. On the other hand, on
systems, where alloca is not available, this gracefully fallbacks to
xmalloc/free.
Both autoconf and config.mak.uname configurations were updated. For
autoconf, we are not bothering considering cases, when no alloca.h is
available, but alloca() works some other way - its simply alloca.h is
available and works or not, everything else is deep legacy.
For config.mak.uname, I've tried to make my almost-sure guess for where
alloca() is available, but since I only have access to Linux it is the
only change I can be sure about myself, with relevant to other changed
systems people Cc'ed.
NOTE
SunOS and Windows had explicit -DHAVE_ALLOCA_H in their configurations.
I've changed that to now-common HAVE_ALLOCA_H=YesPlease which should be
correct.
Cc: Brandon Casey draf...@gmail.com
Cc: Marius Storm-Olsen msto...@gmail.com
Cc: Johannes Sixt j...@kdbg.org
Cc: Johannes Schindelin johannes.schinde...@gmx.de
Cc: Ramsay Jones ram...@ramsay1.demon.co.uk
Cc: Gerrit Pape p...@smarden.org
Cc: Petr Salinger petr.salin...@seznam.cz
Cc: Jonathan Nieder jrnie...@gmail.com
Cc: Thomas Schwinge tschwi...@gnu.org
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
( new patch )
Makefile | 6 ++
config.mak.uname | 10 --
configure.ac | 8
git-compat-util.h | 8
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index dddaf4f..0334806 100644
--- a/Makefile
+++ b/Makefile
@@ -30,6 +30,8 @@ all::
# Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
# /foo/bar/include and /foo/bar/lib directories.
#
+# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header.
+#
# Define NO_CURL if you do not have libcurl installed. git-http-fetch and
# git-http-push are not built, and you cannot use http:// and https://
# transports (neither smart nor dumb).
@@ -1099,6 +1101,10 @@ ifdef USE_LIBPCRE
EXTLIBS += -lpcre
endif
+ifdef HAVE_ALLOCA_H
+ BASIC_CFLAGS += -DHAVE_ALLOCA_H
+endif
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..71602ee 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -28,6 +28,7 @@ ifeq ($(uname_S),OSF1)
NO_NSEC = YesPlease
endif
ifeq ($(uname_S),Linux)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -35,6 +36,7 @@ ifeq ($(uname_S),Linux)
HAVE_DEV_TTY = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -103,6 +105,7 @@ ifeq ($(uname_S),SunOS)
NEEDS_NSL = YesPlease
SHELL_PATH = /bin/bash
SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
+ HAVE_ALLOCA_H = YesPlease
NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_MKDTEMP = YesPlease
@@ -146,7 +149,7 @@ ifeq ($(uname_S),SunOS)
endif
INSTALL = /usr/ucb/install
TAR = gtar
- BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__ -DHAVE_ALLOCA_H
+ BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
endif
ifeq ($(uname_O),Cygwin)
ifeq ($(shell expr $(uname_R) : '1\.[1-6]\.'),4)
@@ -166,6 +169,7 @@ ifeq ($(uname_O),Cygwin)
else
NO_REGEX = UnfortunatelyYes
endif
+ HAVE_ALLOCA_H = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
@@ -239,6 +243,7 @@ ifeq ($(uname_S),AIX)
endif
ifeq ($(uname_S),GNU)
# GNU/Hurd
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -316,6 +321,7 @@ endif
ifeq ($(uname_S),Windows)
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
+ HAVE_ALLOCA_H = YesPlease
NO_PREAD = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
@@ -363,7 +369,7 @@ ifeq ($(uname_S),Windows)
COMPAT_OBJS = compat/msvc.o compat/winansi.o \
[PATCH 03/19] tree-diff: no need to manually verify that there is no mode change for a path
Because if there is, such two tree entries would never be compared as
equal - the code in base_name_compare() explicitly compares modes, if
there is a change for dir bit, even for equal paths, entries would
compare as different.
The code I'm removing here is from 2005 April 262e82b4 (Fix diff-tree
recursion), which pre-dates base_name_compare() introduction in 958ba6c9
(Introduce base_name_compare() helper function) by a month.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 11c3550..5810b00 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -23,6 +23,11 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
pathlen1 = tree_entry_len(t1-entry);
pathlen2 = tree_entry_len(t2-entry);
+
+ /*
+* NOTE files and directories *always* compare differently,
+* even when having the same name.
+*/
cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
if (cmp 0) {
show_entry(opt, -, t1, base);
@@ -35,16 +40,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) !hashcmp(sha1, sha2)
mode1 == mode2)
return 0;
- /*
-* If the filemode has changed to/from a directory from/to a regular
-* file, we need to consider it a remove and an add.
-*/
- if (S_ISDIR(mode1) != S_ISDIR(mode2)) {
- show_entry(opt, -, t1, base);
- show_entry(opt, +, t2, base);
- return 0;
- }
-
strbuf_add(base, path1, pathlen1);
if (DIFF_OPT_TST(opt, RECURSIVE) S_ISDIR(mode1)) {
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
--
1.9.rc1.181.g641f458
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/19] tree-diff: show_tree() is not needed
We don't need special code for showing added/removed subtree, because we
can do the same via diff_tree_sha1, just passing NULL for absent tree.
And compared to show_tree(), which was calling show_entry() for every
tree entry, that would lead to the same show_entry() callings:
show_tree(t):
for e in t.entries:
show_entry(e)
diff_tree_sha1(NULL, new): /* the same applies to (old, NULL) */
diff_tree(t1=NULL, t2)
...
if (!t1-size)
show_entry(t2)
...
and possible overhead is negligible, since after the patch, timing for
`git log --raw --no-abbrev --no-renames`
for navy.git and `linux.git v3.10..v3.11` is practically the same.
So let's say goodbye to show_tree() - it removes some code, but also,
and what is important, consolidates more code for showing/recursing into
trees into one place.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
( re-posting without change )
tree-diff.c | 35 +++
1 file changed, 3 insertions(+), 32 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index a8c2aec..2ad7788 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -55,25 +55,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
return 0;
}
-/* A whole sub-tree went away or appeared */
-static void show_tree(struct diff_options *opt, const char *prefix,
- struct tree_desc *desc, struct strbuf *base)
-{
- enum interesting match = entry_not_interesting;
- for (; desc-size; update_tree_entry(desc)) {
- if (match != all_entries_interesting) {
- match = tree_entry_interesting(desc-entry, base, 0,
- opt-pathspec);
- if (match == all_entries_not_interesting)
- break;
- if (match == entry_not_interesting)
- continue;
- }
- show_entry(opt, prefix, desc, base);
- }
-}
-
-/* A file entry went away or appeared */
+/* An entry went away or appeared */
static void show_entry(struct diff_options *opt, const char *prefix,
struct tree_desc *desc, struct strbuf *base)
{
@@ -85,23 +67,12 @@ static void show_entry(struct diff_options *opt, const char
*prefix,
strbuf_add(base, path, pathlen);
if (DIFF_OPT_TST(opt, RECURSIVE) S_ISDIR(mode)) {
- enum object_type type;
- struct tree_desc inner;
- void *tree;
- unsigned long size;
-
- tree = read_sha1_file(sha1, type, size);
- if (!tree || type != OBJ_TREE)
- die(corrupt tree sha %s, sha1_to_hex(sha1));
-
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
opt-add_remove(opt, *prefix, mode, sha1, 1, base-buf,
0);
strbuf_addch(base, '/');
-
- init_tree_desc(inner, tree, size);
- show_tree(opt, prefix, inner, base);
- free(tree);
+ diff_tree_sha1(*prefix == '-' ? sha1 : NULL,
+ *prefix == '+' ? sha1 : NULL, base-buf, opt);
} else
opt-add_remove(opt, prefix[0], mode, sha1, 1, base-buf, 0);
--
1.9.rc1.181.g641f458
--
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 18/19] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
Previously diff_tree(), which is now named __diff_tree_sha1(), was generating diff_filepair(s) for two trees t1 and t2, and that was usually used for a commit as t1=HEAD~, and t2=HEAD - i.e. to see changes a commit introduces. In Git, however, we have fundamentally built flexibility in that a commit can have many parents - 1 for a plain commit, 2 for a simple merge, but also more than 2 for merging several heads at once. For merges there is a so called combine-diff, which shows diff, a merge introduces by itself, omitting changes done by any parent. That works through first finding paths, that are different to all parents, and then showing generalized diff, with separate columns for +/- for each parent. The code lives in combine-diff.c . There is an impedance mismatch, however, in that a commit could generally have any number of parents, and that while diffing trees, we divide cases for 2-tree diffs and more-than-2-tree diffs. I mean there is no special casing for multiple parents commits in e.g. revision-walker . That impedance mismatch *hurts* *performance* *badly* for generating combined diffs - in combine-diff: optimize combine_diff_path sets intersection I've already removed some slowness from it, but from the timings provided there, it could be seen, that combined diffs still cost more than an order of magnitude more cpu time, compared to diff for usual commits, and that would only be an optimistic estimate, if we take into account that for e.g. linux.git there is only one merge for several dozens of plain commits. That slowness comes from the fact that currently, while generating combined diff, a lot of time is spent computing diff(commit,commit^2) just to only then intersect that huge diff to almost small set of files from diff(commit,commit^1). That's because at present, to compute combine-diff, for first finding paths, that every parent touches, we use the following combine-diff property/definition: D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn) (w.r.t. paths) where D(A,P1...Pn) is combined diff between commit A, and parents Pi and D(A,Pi) is usual two-tree diff Pi..A So if any of that D(A,Pi) is huge, tracting 1 n-parent combine-diff as n 1-parent diffs and intersecting results will be slow. And usually, for linux.git and other topic-based workflows, that D(A,P2) is huge, because, if merge-base of A and P2, is several dozens of merges (from A, via first parent) below, that D(A,P2) will be diffing sum of merges from several subsystems to 1 subsystem. The solution is to avoid computing n 1-parent diffs, and to find changed-to-all-parents paths via scanning A's and all Pi's trees simultaneously, at each step comparing their entries, and based on that comparison, populate paths result, and deduce we could *skip* *recursing* into subdirectories, if at least for 1 parent, sha1 of that dir tree is the same as in A. That would save us from doing significant amount of needless work. Such approach is very similar to what diff_tree() does, only there we deal with scanning only 2 trees simultaneously, and for n+1 tree, the logic is a bit more complex: D(A,X1...Xn) calculation scheme --- D(A,X1...Xn) = D(A,X1) ^ ... ^ D(A,Xn) (regarding resulting paths set) D(A,Xj) - diff between A..Xj D(A,X1...Xn)- combined diff from A to parents X1,...,Xn We start from all trees, which are sorted, and compare their entries in lock-step: A X1 Xn - -- |a| |x1| |xn| |-| |--| ... |--| i = argmin(x1...xn) | | | | | | |-| |--| |--| |.| |. | |. | . .. . .. at any time there could be 3 cases: 1) a xi; 2) a xi; 3) a = xi. Schematic deduction of what every case means, and what to do, follows: 1) a xi - ∀j a ∉ Xj - +a ∈ D(A,Xj) - D += +a; a↓ 2) a xi 2.1) ∃j: xj xi - -xi ∉ D(A,Xj) - D += ø; ∀ xk=xi xk↓ 2.2) ∀j xj = xi - xj ∉ A - -xj ∈ D(A,Xj) - D += -xi; ∀j xj↓ 3) a = xi 3.1) ∃j: xj xi - +a ∈ D(A,Xj) - only xk=xi remains to investigate 3.2) xj = xi - investigate δ(a,xj) | | v 3.1+3.2) looking at δ(a,xk) ∀k: xk=xi - if all != ø - ⎧δ(a,xk) - if xk=xi - D += ⎨ ⎩+a - if xkxi in any case a↓ ∀ xk=xi xk↓ ~ For comparison, here is how diff_tree() works: D(A,B) calculation scheme - A B - - |a| |b|a b - a ∉ B - D(A,B) += +aa↓ |-| |-|a b - b ∉ A - D(A,B) += -bb↓ | | | |a = b - investigate δ(a,b)a↓ b↓ |-| |-| |.| |.| . . . . This patch generalizes diff tree-walker to work with arbitrary number of parents as described above - i.e. now there is a resulting tree t, and some parents trees tp[i] i=[0..nparent). The generalization builds on
[PATCH 10/19] tree-diff: show_path prototype is not needed anymore
We moved all action-taking code below show_path() in recent HEAD~~ (tree-diff: move all action-taking code out of compare_tree_entry). Signed-off-by: Kirill Smelkov k...@mns.spb.ru Signed-off-by: Junio C Hamano gits...@pobox.com --- ( re-posting without change ) tree-diff.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tree-diff.c b/tree-diff.c index 3345534..20a4fda 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -6,9 +6,6 @@ #include diffcore.h #include tree.h -static void show_path(struct strbuf *base, struct diff_options *opt, - struct tree_desc *t1, struct tree_desc *t2); - /* * Compare two tree entries, taking into account only path/S_ISDIR(mode), * but not their sha1's. -- 1.9.rc1.181.g641f458 -- 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 08/19] tree-diff: move all action-taking code out of compare_tree_entry()
- let it do only comparison.
This way the code is cleaner and more structured - cmp function only
compares, and the driver takes action based on comparison result.
There should be no change in performance, as effectively, we just move
if series from on place into another, and merge it to was-already-there
same switch/if, so the result is maybe a little bit faster.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 5f7dbbf..6207372 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -9,8 +9,7 @@
static void show_path(struct strbuf *base, struct diff_options *opt,
struct tree_desc *t1, struct tree_desc *t2);
-static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
- struct strbuf *base, struct diff_options *opt)
+static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2)
{
unsigned mode1, mode2;
const char *path1, *path2;
@@ -28,19 +27,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct
tree_desc *t2,
* even when having the same name.
*/
cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
- if (cmp 0) {
- show_path(base, opt, t1, /*t2=*/NULL);
- return -1;
- }
- if (cmp 0) {
- show_path(base, opt, /*t1=*/NULL, t2);
- return 1;
- }
- if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) !hashcmp(sha1, sha2)
mode1 == mode2)
- return 0;
-
- show_path(base, opt, t1, t2);
- return 0;
+ return cmp;
}
@@ -161,6 +148,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
strbuf_add(base, base_str, baselen);
for (;;) {
+ int cmp;
+
if (diff_can_quit_early(opt))
break;
if (opt-pathspec.nr) {
@@ -180,21 +169,28 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
continue;
}
- cmp = compare_tree_entry(t1, t2, base, opt);
+ cmp = compare_tree_entry(t1, t2);
/* t1 = t2 */
if (cmp == 0) {
+ if (DIFF_OPT_TST(opt, FIND_COPIES_HARDER) ||
+ hashcmp(t1-entry.sha1, t2-entry.sha1) ||
+ (t1-entry.mode != t2-entry.mode))
+ show_path(base, opt, t1, t2);
+
update_tree_entry(t1);
update_tree_entry(t2);
}
/* t1 t2 */
else if (cmp 0) {
+ show_path(base, opt, t1, /*t2=*/NULL);
update_tree_entry(t1);
}
/* t1 t2 */
else {
+ show_path(base, opt, /*t1=*/NULL, t2);
update_tree_entry(t2);
}
}
--
1.9.rc1.181.g641f458
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/19] tree-diff: don't assume compare_tree_entry() returns -1,0,1
It does, but we'll be reworking it in the next patch after it won't, and
besides it is better to stick to standard
strcmp/memcmp/base_name_compare/etc... convention, where comparison
function returns 0, =0, 0
Regarding performance, comparing for 0, =0, 0 should be a little bit
faster, than switch, because it is just 1 test-without-immediate
instruction and then up to 3 conditional branches, and in switch you
have up to 3 tests with immediate and up to 3 conditional branches.
No worry, that update_tree_entry(t2) is duplicated for =0 and 0 - it
will be good after we'll be adding support for multiparent walker and
will stay that way.
=0 case goes first, because it happens more often in real diffs - i.e.
paths are the same.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index a5b9ff9..5f7dbbf 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -179,18 +179,24 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
update_tree_entry(t1);
continue;
}
- switch (compare_tree_entry(t1, t2, base, opt)) {
- case -1:
+
+ cmp = compare_tree_entry(t1, t2, base, opt);
+
+ /* t1 = t2 */
+ if (cmp == 0) {
update_tree_entry(t1);
- continue;
- case 0:
+ update_tree_entry(t2);
+ }
+
+ /* t1 t2 */
+ else if (cmp 0) {
update_tree_entry(t1);
- /* Fallthrough */
- case 1:
+ }
+
+ /* t1 t2 */
+ else {
update_tree_entry(t2);
- continue;
}
- die(git diff-tree: internal error);
}
strbuf_release(base);
--
1.9.rc1.181.g641f458
--
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 16/19] tree-diff: reuse base str(buf) memory on sub-tree recursion
instead of allocating it all the time for every subtree in
__diff_tree_sha1, let's allocate it once in diff_tree_sha1, and then all
callee just use it in stacking style, without memory allocations.
This should be faster, and for me this change gives the following
slight speedups for
git log --raw --no-abbrev --no-renames --format='%H'
navy.gitlinux.git v3.10..v3.11
before 0.618s 1.903s
after 0.611s 1.889s
speedup 1.1%0.7%
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
Changes since v1:
- don't need to touch diff.h, as the function we are changing became static.
tree-diff.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index aea0297..c76821d 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -115,7 +115,7 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
__diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
-t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+t2 ? t2-entry.sha1 : NULL, base, opt);
}
strbuf_setlen(base, old_baselen);
@@ -138,12 +138,10 @@ static void skip_uninteresting(struct tree_desc *t,
struct strbuf *base,
}
static int __diff_tree_sha1(const unsigned char *old, const unsigned char *new,
- const char *base_str, struct diff_options *opt)
+ struct strbuf *base, struct diff_options *opt)
{
struct tree_desc t1, t2;
void *t1tree, *t2tree;
- struct strbuf base;
- int baselen = strlen(base_str);
t1tree = fill_tree_descriptor(t1, old);
t2tree = fill_tree_descriptor(t2, new);
@@ -151,17 +149,14 @@ static int __diff_tree_sha1(const unsigned char *old,
const unsigned char *new,
/* Enable recursion indefinitely */
opt-pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
- strbuf_init(base, PATH_MAX);
- strbuf_add(base, base_str, baselen);
-
for (;;) {
int cmp;
if (diff_can_quit_early(opt))
break;
if (opt-pathspec.nr) {
- skip_uninteresting(t1, base, opt);
- skip_uninteresting(t2, base, opt);
+ skip_uninteresting(t1, base, opt);
+ skip_uninteresting(t2, base, opt);
}
if (!t1.size !t2.size)
break;
@@ -173,7 +168,7 @@ static int __diff_tree_sha1(const unsigned char *old, const
unsigned char *new,
if (DIFF_OPT_TST(opt, FIND_COPIES_HARDER) ||
hashcmp(t1.entry.sha1, t2.entry.sha1) ||
(t1.entry.mode != t2.entry.mode))
- show_path(base, opt, t1, t2);
+ show_path(base, opt, t1, t2);
update_tree_entry(t1);
update_tree_entry(t2);
@@ -181,18 +176,17 @@ static int __diff_tree_sha1(const unsigned char *old,
const unsigned char *new,
/* t1 t2 */
else if (cmp 0) {
- show_path(base, opt, t1, /*t2=*/NULL);
+ show_path(base, opt, t1, /*t2=*/NULL);
update_tree_entry(t1);
}
/* t1 t2 */
else {
- show_path(base, opt, /*t1=*/NULL, t2);
+ show_path(base, opt, /*t1=*/NULL, t2);
update_tree_entry(t2);
}
}
- strbuf_release(base);
free(t2tree);
free(t1tree);
return 0;
@@ -209,7 +203,7 @@ static inline int diff_might_be_rename(void)
!DIFF_FILE_VALID(diff_queued_diff.queue[0]-one);
}
-static void try_to_follow_renames(const unsigned char *old, const unsigned
char *new, const char *base, struct diff_options *opt)
+static void try_to_follow_renames(const unsigned char *old, const unsigned
char *new, struct strbuf *base, struct diff_options *opt)
{
struct diff_options diff_opts;
struct diff_queue_struct *q = diff_queued_diff;
@@ -306,13 +300,19 @@ static void try_to_follow_renames(const unsigned char
*old, const unsigned char
q-nr = 1;
}
-int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const
char *base, struct diff_options *opt)
+int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const
char *base_str, struct diff_options *opt)
{
+ struct strbuf base;
int retval;
- retval = __diff_tree_sha1(old, new, base, opt);
- if (!*base DIFF_OPT_TST(opt, FOLLOW_RENAMES)
diff_might_be_rename())
- try_to_follow_renames(old, new, base, opt);
+ strbuf_init(base,
Re: [PATCH v2] tag: support --sort=spec
Jeff King p...@peff.net writes:
On Sat, Feb 22, 2014 at 10:29:22AM +0700, Nguyễn Thái Ngọc Duy wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
The new prereq GNULINUX is an ugly workaround until people provide
strverscmp compat implementation. I hope that will happen soon as
strverscmp.c does not look very complex.
Should GNULINUX be called HAVE_STRVERSCMP in the Makefile?
Then this:
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -721,4 +721,11 @@ void warn_on_inaccessible(const char *path);
/* Get the passwd entry for the UID of the current process. */
struct passwd *xgetpwuid_self(void);
+#ifndef __GNU_LIBRARY__
+static inline int strverscmp(const char *s1, const char *s2)
+{
+die(strverscmp() not supported);
+}
+#endif
becomes #ifndef HAVE_STRVERSCMP, and this:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..5e8c39a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -771,6 +771,8 @@ case $(uname -s) in
;;
esac
+[ $(uname -o) = GNU/Linux ] test_set_prereq GNULINUX
+
can pick up the value from GIT-BUILD-OPTIONS as a prerequisite (see the
way we handle NO_PERL for an example). Though if we can just grab the
glibc version as a fallback, we can do away with that completely.
;-) I 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: difftool sends malformed path to exernal tool on Windows
David, Thanks for the helpful reply. As you suggested, I modified the .gitconfig file to have: [difftool test] cmd = echo \$LOCAL\ \$REMOTE\ and ran $ git difftool -t test An example of the the resulting console output is: C:/Users/Paul/AppData/Local/Temp/I8L2Bc_WriteTestParameters.vi Commands/StartAutomatedTest/WriteTestParameters.vi Paul -Original Message- From: David Aguilar [mailto:dav...@gmail.com] Sent: Friday, February 21, 2014 3:38 AM To: Paul Lotz Cc: git@vger.kernel.org Subject: Re: difftool sends malformed path to exernal tool on Windows On Mon, Feb 17, 2014 at 03:14:01PM -0700, Paul Lotz wrote: From the Git Bash command line, I enter $ git difftool and type ‘y’ when the file I want to difference appears. Git correctly calls the external diff tool (LVCompare.exe), but the path for the remote file Git passes to that tool is malformed (e.g., C:\/Users/Paul/AppData/Local/Temp/QCpqLa_calcLoadCellExcitation.vi). Obviously the \/ (backslash forwardslash) combination is incorrect. If this is the case then difftool is not the only one with this problem. We use the GIT_EXTERNAL_DIFF mechanism to run difftool under git diff, so it may be that the paths are mangled by git diff itself. I don't really know enough about msysgit to know for sure, though. What do you see if you create a dummy tool which just does echo? [difftool test] cmd = echo \$LOCAL\ \$REMOTE\ Then run: $ git difftool -t test For the record, I have successfully made calls to LVCompare.exe manually from a Windows command prompt directly (without Git). The relevant portion of the .gitconfig file is: [diff] tool = LVCompare [difftool LVCompare] cmd = 'C:/Program Files (x86)/National Instruments/Shared/LabVIEW Compare/LVCompare.exe' \$LOCAL\ \$REMOTE\ For the record, the operating system is Windows 8.1. Do any msysgit folks know whether GIT_EXTERNAL_DIFF is a known issue? -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)
Max Horn m...@quendi.de writes: On 21.02.2014, at 19:04, Junio C Hamano gits...@pobox.com wrote: Isn't it possible for some helpers to _do_ want to tell us that it did not have to force after all by _not_ saying forced update and overwrite -forced_update with zero? Yes to the first part, no to the last bit: Yes, a transport helper can (and frequently does) tell us that no force happened -- by not saying forced update. ... How do we tell helpers that do want to do so apart from other helpers that say forced update only when they noticed they are indeed forcing? I am not completely sure I even understand that bit? I think I phrased it too imprecisely. If nobody even knew about the forced update before hg helper, then they by definition do not wish to overwrite, of course. But I was worried if we are closing the door for this possible scenario: * the calling side sets ref-forced_update to true before invoking the helper, knowing that this update is not fast-forward; and * the helper does a magic (after all, we are talking with an external mechanism, which may be a different SCM like darcs) to rebase our change on top of the history that the other side already have, and makes it a fast-forward, non-forced push. Such a helper would want a way to say You may have thought that this does not fast-forward, but the push result ended up to be a fast-forward update, and if we wanted to support that, one thing we may need to allow it to do is to reset ref-forced_update to zero. But I think I was worried too much into the future---I agree that the code can stay as you proposed until such a remote-helper needs more support, because overwrite with zero is necessary but is probably not sufficient---it also may need to be able to tell us what the final resulting commit of the push is, for example. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)
Junio C Hamano gits...@pobox.com writes: But I think I was worried too much into the future---I agree that the code can stay as you proposed until such a remote-helper needs more support, because overwrite with zero is necessary but is probably not sufficient---it also may need to be able to tell us what the final resulting commit of the push is, for example. So, here is what I'll queue (with forged s-o-b). Thanks. -- 8 -- From: Max Horn m...@quendi.de Date: Fri, 21 Feb 2014 10:55:59 +0100 Subject: [PATCH] transport-helper.c: do not overwrite forced bit If the the transport helper says it was a forced update, then it is a forced update. It is however possible that an update is forced without the transport-helper knowing about it, namely because some higher up code had objections to the update and needed forcing in order to let it through to the transport helper. In other words, it does not necessarily mean the update was *not* forced, when the helper did not say forced update. Signed-off-by: Max Horn m...@quendi.de Signed-off-by: Junio C Hamano gits...@pobox.com --- transport-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index abe4c3c..705dce7 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -727,7 +727,7 @@ static int push_update_ref_status(struct strbuf *buf, } (*ref)-status = status; - (*ref)-forced_update = forced; + (*ref)-forced_update |= forced; (*ref)-remote_status = msg; return !(status == REF_STATUS_OK); } -- 1.9.0-291-g027825b -- 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 6/6] Document some functions defined in object.c
Michael Haggerty mhag...@alum.mit.edu writes: Contrariwise, I thought about it again and believe that it *is* important for the docstring to mention explicitly that the return value is architecture-dependent As it gives an internal hash value which should not leak to the outside world (e.g. stored in a file or sent over the wire later to be read by other platforms), I think that is a good idea. -- 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] demonstrate git-commit --dry-run exit code behaviour
Jeff King p...@peff.net writes: On Fri, Feb 21, 2014 at 12:21:13PM -0800, Junio C Hamano wrote: Tay Ray Chuan rcta...@gmail.com writes: In particular, show that --short and --porcelain, while implying --dry-run, do not return the same exit code as --dry-run. This is due to the wt_status.commitable flag being set only when a long status is requested. I am not sure if --short/--porcelain should even be accepted by git commit in the first place. It used to be that git status and git commit were the same program in a different guise and git status anything were merely a git commit --dry-run anything, but the recent push is in the direction of making them totally separate in the end-user's minds. So if we want a proper fix, I would actually think that these options should *error out* at the command line parser level, way before checking if there is anything to commit. I do not think they are any less useful than git commit --dry-run in the first place. If you want to ask what would happen if I ran commit with these arguments, you can get the answer in any of several formats (and --porcelain is the only machine-readable one). Hmph. I have never found commit --dry-run to be useful, but I assumed that somebody does. Same here, and I did not really consider commit --short was intentionally a valid short-hand for commit --dry-run --short, but its working as such was an accident, hence my comment. -- 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] commit: add --cleanup=scissors
Duy Nguyen pclo...@gmail.com writes:
On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote:
@@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_(Please enter the commit message for your
changes.
Lines starting\nwith '%c' will be ignored,
and an empty
message aborts the commit.\n),
comment_line_char);
+else if (cleanup_mode == CLEANUP_SCISSORS)
+wt_status_add_cut_line(s-fp);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
_(Please enter the commit message for your
changes.
This cut line does not cover the merge conflict message before it. The
following patch should be squashed in to move the cut line up in that
case.
I somehow thought that it was a policy decision we made in very
early days that these conflict messages are meant to be edited with
explanation of how they were resolved, not to be simply edited away?
The other stuff (commented out instructions and patch text) are
meant to aid humans while editing the log message, and stripping
away automatically after they are done editing like your patch does
is perfectly fine, but I find this change questionable.
-- 8 --
diff --git a/builtin/commit.c b/builtin/commit.c
index ea2912f..1033c50 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor include_status) {
char *ai_tmp, *ci_tmp;
- if (whence != FROM_COMMIT)
+ if (whence != FROM_COMMIT) {
+ if (cleanup_mode == CLEANUP_SCISSORS)
+ wt_status_add_cut_line(s-fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
? _(\n
@@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
git_path(whence == FROM_MERGE
? MERGE_HEAD
: CHERRY_PICK_HEAD));
+ }
fprintf(s-fp, \n);
if (cleanup_mode == CLEANUP_ALL)
@@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_(Please enter the commit message for your
changes.
Lines starting\nwith '%c' will be ignored,
and an empty
message aborts the commit.\n),
comment_line_char);
- else if (cleanup_mode == CLEANUP_SCISSORS)
+ else if (cleanup_mode == CLEANUP_SCISSORS whence ==
FROM_COMMIT)
wt_status_add_cut_line(s-fp);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
-- 8 --
--
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] index-pack: show chain length histogram as graph for better visual
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
definitely better than raw numbers but not really important
I would appreciate if it were an optional feature.
diff --git a/diff.c b/diff.c
index 8e4a6a9..ca2b92a 100644
--- a/diff.c
+++ b/diff.c
@@ -1489,7 +1489,8 @@ int print_stat_summary(FILE *fp, int files, int
insertions, int deletions)
return ret;
}
-static void show_stats(struct diffstat_t *data, struct diff_options *options)
+static void show_stats(struct diffstat_t *data, struct diff_options *options,
+int summary)
{
int i, len, add, del, adds = 0, dels = 0;
uintmax_t max_change = 0, max_len = 0;
@@ -1729,10 +1730,40 @@ static void show_stats(struct diffstat_t *data,
struct diff_options *options)
fprintf(options-file, %s ...\n, line_prefix);
extra_shown = 1;
}
+ if (!summary)
+ return;
Yuck.
fprintf(options-file, %s, line_prefix);
print_stat_summary(options-file, total_files, adds, dels);
}
+void show_histogram_graph(const char *label, unsigned long *data, int nr)
+{
+ struct diffstat_t diffstat;
+ struct diff_options options;
+ struct diffstat_file *files;
+ char buf[64];
+ int i;
+
+ memset(options, 0, sizeof(options));
+ options.file = stdout;
+ options.use_color = diff_use_color_default;
+ options.stat_width = -1;
+ diffstat.nr = nr;
+ diffstat.files = xmalloc(nr * sizeof(*diffstat.files));
Double yuck for an incomplete refactoring. Why should a generic
histogram-drawer know about *diff*-anything (and if this is still
a diff-specific histogram-drawer, verify-pack has no business
calling it)?
I like the idea very much, but not this particular execution.
+ files = xcalloc(nr, sizeof(*files));
+ for (i = 0; i nr; i++) {
+ diffstat.files[i] = files + i;
+ sprintf(buf, %s %d, label, i);
+ files[i].name = xstrdup(buf);
+ files[i].added = data[i];
+ }
+ show_stats(diffstat, options, 0);
--
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] diff: do not reuse_worktree_file for submodules
Thomas Rast t...@thomasrast.ch writes:
I spoke too soon; it breaks the test I wrote to cover this case, for a
reason that gives me a headache.
When we hit the conditional
- if (!one-sha1_valid ||
- reuse_worktree_file(name, one-sha1, 1)) {
+ if (!S_ISGITLINK(one-mode)
+ (!one-sha1_valid ||
+ reuse_worktree_file(name, one-sha1, 1))) {
sha1_valid=0 for the submodule on the worktree side of the diff. The
reason is that we start out with sha1_valid=0 and sha1=000..000 for the
worktree side of all dirty entries, which makes sense at that point. We
later set the sha1 by looking inside the submodule in
diff_fill_sha1_info(), but we never set sha1_valid. So the above
conditional will now trigger on the !one-sha1_valid arm, completely
defeating the change to reuse_worktree_file().
We can fix it like below, but it feels a bit wrong to me. Are
submodules the only case where it makes sense to set sha1_valid when we
fill the sha1?
The meaning of filespec-sha1_valid is Is it known that the
filespec-sha1 and filespec-mode field should be used?; I agree
that this feels wrong.
Which means that the previous one was wrong, and your original was
the right approach. I'll drop the update.
Thanks.
diff.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git i/diff.c w/diff.c
index dabf913..cf7281d 100644
--- i/diff.c
+++ w/diff.c
@@ -3081,6 +3082,8 @@ static void diff_fill_sha1_info(struct diff_filespec
*one)
die_errno(stat '%s', one-path);
if (index_path(one-sha1, one-path, st, 0))
die(cannot hash %s, one-path);
+ if (S_ISGITLINK(one-mode))
+ one-sha1_valid = 1;
}
}
else
--
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] clone: allow initial sparse checkouts
Robin H. Johnson robb...@gentoo.org writes: The only other clean alternative would be implementing ONLY --sparse-checkout-from, and letting uses use fds creatively: --sparse-checkout-from (echo X; echo Y) Not all POSIX shells have such an abomination that is process substitution. You can easily work it around by adopting the usual convention to use - to read from the standasrd input, though. (echo X; echo Y) | cmd --sparse-checkout-from - -- 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] difftool: support repositories with .git-files
David Aguilar dav...@gmail.com writes:
Modern versions of git submodule use .git-files to setup the
submodule directory. When run in a git submodule-created
repository git difftool --dir-diff dies with the following
error:
$ git difftool -d HEAD~
fatal: This operation must be run in a work tree
diff --raw --no-abbrev -z HEAD~: command returned error: 128
core.worktree is relative to the .git directory but the logic
in find_worktree() does not account for it.
Use `git rev-parse --show-toplevel` to find the worktree so that
the dir-diff feature works inside a submodule.
Reported-by: Gábor Lipták gabor.lip...@gmail.com
Helped-by: Jens Lehmann jens.lehm...@web.de
Helped-by: John Keeping j...@keeping.me.uk
Signed-off-by: David Aguilar dav...@gmail.com
---
Looks good; thanks.
git-difftool.perl | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index e57d3d1..18ca61e 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -39,24 +39,10 @@ USAGE
sub find_worktree
{
- my ($repo) = @_;
-
# Git-repository-wc_path() does not honor changes to the working
# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'
# config variable.
- my $worktree;
- my $env_worktree = $ENV{GIT_WORK_TREE};
- my $core_worktree = Git::config('core.worktree');
-
- if (defined($env_worktree) and (length($env_worktree) 0)) {
- $worktree = $env_worktree;
- } elsif (defined($core_worktree) and (length($core_worktree) 0)) {
- $worktree = $core_worktree;
- } else {
- $worktree = $repo-wc_path();
- }
-
- return $worktree;
+ return Git::command_oneline('rev-parse', '--show-toplevel');
}
sub print_tool_help
@@ -418,7 +404,7 @@ sub dir_diff
my $rc;
my $error = 0;
my $repo = Git-repository();
- my $workdir = find_worktree($repo);
+ my $workdir = find_worktree();
my ($a, $b, $tmpdir, @worktree) =
setup_dir_diff($repo, $workdir, $symlinks);
--
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] diffcore.h: be explicit about the signedness of is_binary
Richard Lowe richl...@richlowe.net writes:
Bitfields need to specify their signedness explicitly or the compiler is
free to default as it sees fit. With compilers that default 'unsigned'
(SUNWspro 12 seems to do this) the tri-state nature of is_binary
vanishes and all files are treated as binary.
Signed-off-by: Richard Lowe richl...@richlowe.net
---
Looks good; thanks.
diffcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diffcore.h b/diffcore.h
index 79de8cf..7c6f391 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -46,7 +46,7 @@ struct diff_filespec {
unsigned is_stdin : 1;
unsigned has_more_entries : 1; /* only appear in combined diff */
/* data should be considered binary; -1 means don't know yet */
- int is_binary : 2;
+ signed int is_binary : 2;
struct userdiff_driver *driver;
};
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)
Am 24.02.2014 um 18:06 schrieb Junio C Hamano gits...@pobox.com: Junio C Hamano gits...@pobox.com writes: But I think I was worried too much into the future---I agree that the code can stay as you proposed until such a remote-helper needs more support, because overwrite with zero is necessary but is probably not sufficient---it also may need to be able to tell us what the final resulting commit of the push is, for example. So, here is what I'll queue (with forged s-o-b). Thank you, I hereby declare the forged s-o-b as legit ;-) Thanks. -- 8 -- From: Max Horn m...@quendi.de Date: Fri, 21 Feb 2014 10:55:59 +0100 Subject: [PATCH] transport-helper.c: do not overwrite forced bit If the the transport helper says it was a forced update, then it is a forced update. It is however possible that an update is forced without the transport-helper knowing about it, namely because some higher up code had objections to the update and needed forcing in order to let it through to the transport helper. In other words, it does not necessarily mean the update was *not* forced, when the helper did not say forced update. Signed-off-by: Max Horn m...@quendi.de Signed-off-by: Junio C Hamano gits...@pobox.com --- transport-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index abe4c3c..705dce7 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -727,7 +727,7 @@ static int push_update_ref_status(struct strbuf *buf, } (*ref)-status = status; -(*ref)-forced_update = forced; +(*ref)-forced_update |= forced; (*ref)-remote_status = msg; return !(status == REF_STATUS_OK); } -- 1.9.0-291-g027825b -- 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] remote: handle pushremote config in any order order
Jeff King p...@peff.net writes:
Yes, with a few exceptions, we usually try to make the ordering in the
config file irrelevant. This is a bug. The patch below should fix it.
Looks good. Thanks.
-- 8 --
Subject: remote: handle pushremote config in any order
The remote we push can be defined either by
remote.pushdefault or by branch.*.pushremote for the current
branch. The order in which they appear in the config file
should not matter to precedence (which should be to prefer
the branch-specific config).
The current code parses the config linearly and uses a
single string to store both values, overwriting any
previous value. Thus, config like:
[branch master]
pushremote = foo
[remote]
pushdefault = bar
erroneously ends up pushing to bar from the master branch.
We can fix this by storing both values and resolving the
correct value after all config is read.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 7 ++-
t/t5516-fetch-push.sh | 12
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index e41251e..7232a33 100644
--- a/remote.c
+++ b/remote.c
@@ -49,6 +49,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *branch_pushremote_name;
static const char *pushremote_name;
static int explicit_default_remote_name;
@@ -352,7 +353,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
}
} else if (!strcmp(subkey, .pushremote)) {
if (branch == current_branch)
- if (git_config_string(pushremote_name, key,
value))
+ if (git_config_string(branch_pushremote_name,
key, value))
return -1;
} else if (!strcmp(subkey, .merge)) {
if (!value)
@@ -492,6 +493,10 @@ static void read_config(void)
make_branch(head_ref + strlen(refs/heads/), 0);
}
git_config(handle_config, NULL);
+ if (branch_pushremote_name) {
+ free(pushremote_name);
+ pushremote_name = branch_pushremote_name;
+ }
alias_all_urls();
}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 926e7f6..1309c4d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -536,6 +536,18 @@ test_expect_success 'push with config
branch.*.pushremote' '
check_push_result down_repo $the_commit heads/master
'
+test_expect_success 'branch.*.pushremote config order is irrelevant' '
+ mk_test one_repo heads/master
+ mk_test two_repo heads/master
+ test_config remote.one.url one_repo
+ test_config remote.two.url two_repo
+ test_config branch.master.pushremote two_repo
+ test_config remote.pushdefault one_repo
+ git push
+ check_push_result one_repo $the_first_commit heads/master
+ check_push_result two_repo $the_commit heads/master
+'
+
test_expect_success 'push with dry-run' '
mk_test testrepo heads/master
--
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 0/3] Easier access to index-v4
Thomas Gummerer t.gumme...@gmail.com writes: previous round was at $gmane/242198. Since then I've squashed the fixes suggested by Junio, added a test showing what should happen if an index file is present and GIT_INDEX_VERSION is set and fixed the typo found by Eric. Looks good; 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] stash doc: mention short form -k in save description
John Marshall j...@sanger.ac.uk writes: Document --keep-index's short form -k in both main synopsis and the save synopsis in the Options section. Signed-off-by: John Marshall j...@sanger.ac.uk --- Looks good; thanks. A very small documentation patch: I'd not read the main synopsis carefully (just skipped to the save details!) and didn't realise that --keep-index had a short form... Documentation/git-stash.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index db7e803..375213f 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -44,7 +44,7 @@ is also possible). OPTIONS --- -save [-p|--patch] [--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [message]:: +save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [message]:: Save your local modifications to a new 'stash', and run `git reset --hard` to revert them. The message part is optional and gives -- 1.8.3.4 (Apple Git-47) -- 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 0/6] Add a bunch of docstrings and make a few minor cleanups
Michael Haggerty mhag...@alum.mit.edu writes: I was reading around in the neighborhood of fsck, objects, and packs and I had the familiar and discouraging experience of having to read code all the way up and down the callstack to understand *anything*. Please let's all make more of an effort to document functions, especially things that are not obvious from the name and signature, like who owns the memory that is being referred to. This is my attempt to document a number of the functions that I was looking at based on what I inferred from my reading. It is also a selfish trick to get other people to double-check my understanding. I also fixed up a couple of small things that I noticed along the way: refactoring for understanding. Michael Haggerty (6): Add docstrings for lookup_replace_object() and do_lookup_replace_object() replace_object: use struct members instead of an array find_pack_entry(): document last_found_pack sha1_file_name(): declare to return a const string Document a bunch of functions defined in sha1_file.c Document some functions defined in object.c Queued 2, 3, 4, and 5, expecting 1 and 6 will be rerolled. Thanks. cache.h | 84 +--- http.c | 2 +- object.c | 23 +++- object.h | 7 + replace_object.c | 17 sha1_file.c | 66 6 files changed, 157 insertions(+), 42 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] Add docstrings for lookup_replace_object() and do_lookup_replace_object()
Michael Haggerty mhag...@alum.mit.edu writes: Junio, what would be easiest for you? I suggest that I rebase this patch series back on top of mh/replace-refs-variable-rename when re-rolling. Hmph, I suspect I do not care too deeply either way, as a mismerge would be fairly obvious (nobody should be adding any new string read_replace_refs or deleting existing ones and the linker would catch it even if I don't), but having on top of the patch that renames the variable would make sense. 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 5/6] Document a bunch of functions defined in sha1_file.c
Michael Haggerty wrote: -/* - * NOTE! This returns a statically allocated buffer, so you have to be - * careful about using it. Do an xstrdup() if you need to save the - * filename. - * - * Also note that this returns the location for creating. Reading - * SHA1 file can happen from any alternate directory listed in the - * DB_ENVIRONMENT environment variable if it is not found in - * the primary object database. - */ const char *sha1_file_name(const unsigned char *sha1) Has this changed? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack: add `repack.honorpackkeep` config var
Jeff King p...@peff.net writes: Sorry, this one slipped through the cracks. Here's a re-roll addressing your comments. ... - In the context of pack-objects, the name --honor-pack-keep makes sense; it is understood that pack-objects will _not_ remove kept packfile, so honoring can only mean do not attempt to pick objects out of kept packs to add to the pack being generated. and there is no room for --no-honor-pack-keep to be mistaken as you canremove the ones marked to be kept after saving the still-used objects in it away. But does the same name make sense in the context of repack? I think the distinction you are making is to capture the second second from the docs: If set to false, include objects in `.keep` files when repacking via `git repack`. Note that we still do not delete `.keep` packs after `pack-objects` finishes. The best name I could come up with is --pack-keep-objects, since that is literally what it is doing. I'm not wild about the name because it is easy to read keep as a verb (and pack as a noun). I think it's OK, but suggestions are welcome. pack-kept-objects 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
git am and mangled subject lines
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 git am already ignores the [PATCH X/Y] prefix that format-patch adds. Is it possible to get it to ignore any additional prefix that a bug tracker mangles into the subject line? i.e. bug #:? -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTC51PAAoJEI5FoCIzSKrw5YYH/R6t5fS71UAfFomsD/8HWHoI ve1tIyyruHeriOV8qttlNQGynuEXkI2IBMWJaB7jV5oK8d4OsVQZ/7Nfcxoj52SO JXDSs0MVDB2Ro2lHXRnQsaCy/TUm+ALWsNiTy0kYMTeC7Iqtri1T1l8gaG2rwRJh AGT1sgGssl9CvGFgDHJxRZ4WHSl/XrcjErZeJHz59hGIeJSeq2tJXjfNzNTHrNpw B4rcW8AxXhx+3vWPx8PSJsiVeWR1ndILXwxBsUHPuUW5SdsNBrty1L+4xrGIbxm7 qV7HVJ6BvJ4MXuTEOec3a9ACmvUTDNNMGRf2xonjXMcguojRZHjltRazsI34n8o= =sWTQ -END PGP SIGNATURE- -- 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: Cygwin + git log = no pager!
On Mon, Feb 24, 2014 at 3:06 AM, Jeff King p...@peff.net wrote: On Mon, Feb 24, 2014 at 08:55:41PM +1300, Chris Packham wrote: Thanks for the response. I did set this environment variable in my .bashrc like so: export GIT_PAGER=less However after I do a 'git log' it is just spitting it out all at once and not entering the pager. Um OK that was the obvious thing to try. There is also the config variable core.pager but GIT_PAGER should take precedence. Presumably we are actually running what's in GIT_PAGER, but we can double-check with: GIT_PAGER='echo custom pager' git log You can also try: GIT_TRACE=1 git log which should describe the pager being run. Could something be setting the environment variable LESS? Reading the git-config man page for core.pager git wants to set LESS to FRSX but if it is already set git takes that as an indication that you don't want to set LESS automatically. We can also double-check the LESS setting in the executed pager: GIT_PAGER='echo LESS=$LESS' git log If we are running less, and it is using FRSX, I'd suspect some kind of terminal weirdness with less itself. With -F, less will quit if the whole output can be displayed; it's possible it thinks the screen is bigger than it is. Trying: GIT_PAGER=less LESS=RSX git log might help. -Peff So I set GIT_PAGER to 'echo custom pager' as you instructed, and I noticed that wasn't being printed when I ran my git log alias. So what I did after that was set GIT_TRACE=1 and here is the output I see before my logs start printing: $ git lg trace: exec: 'git-lg' trace: run_command: 'git-lg' trace: run_command: 'git lg1' trace: exec: 'git-lg1' trace: run_command: 'git-lg1' trace: run_command: 'git short-log-base --branches --remotes --tags' trace: exec: 'git-short-log-base' '--branches' '--remotes' '--tags' trace: run_command: 'git-short-log-base' '--branches' '--remotes' '--tags' trace: alias expansion: short-log-base = 'log' '--graph' '--abbrev-commit' '--decorate' '--date=relative' '--format=format:%C(bold blue)%h%C(reset)%x09%C(bold green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim white)%an%C(reset) - %C(white)%s%C(reset)' trace: built-in: git 'log' '--graph' '--abbrev-commit' '--decorate' '--date=relative' '--format=format:%C(bold blue)%h%C(reset)%x09%C(bold green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim white)%an%C(reset) - %C(white)%s%C(reset)' '--branches' '--remotes' '--tags' Does using an alias have something to do with this? -- 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 0/3] Easier access to index-v4
Junio C Hamano gits...@pobox.com writes:
Thomas Gummerer t.gumme...@gmail.com writes:
previous round was at $gmane/242198.
Since then I've squashed the fixes suggested by Junio, added a test
showing what should happen if an index file is present and
GIT_INDEX_VERSION is set and fixed the typo found by Eric.
Looks good; thanks.
Tests, seem to leak these unnecessary diag (not limited to t0010).
sh t0010-racy-git.sh
warning: GIT_INDEX_VERSION set, but the value is invalid.
Using version 3
ok 1 - Racy GIT trial #0 part A
ok 2 - Racy GIT trial #0 part B
warning: GIT_INDEX_VERSION set, but the value is invalid.
Using version 3
...
# passed all 10 test(s)
1..10
The same thing under prove.
*** prove ***
t0010-racy-git.sh .. warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 2/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 4/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 6/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 8/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. ok
All tests successful.
I suspect the real culprit is the early part in test-lib.sh that
sets GIT_INDEX_VERSION explicitly from TEST_GIT_INDEX_VERSION when
the latter is not even specified.
Something along this line, perhaps?
t/test-lib.sh | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 492f81f..01a98cb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -108,8 +108,11 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
export EDITOR
-GIT_INDEX_VERSION=$TEST_GIT_INDEX_VERSION
-export GIT_INDEX_VERSION
+if test -n ${TEST_GIT_INDEX_VERSION+isset}
+then
+ GIT_INDEX_VERSION=$TEST_GIT_INDEX_VERSION
+ export GIT_INDEX_VERSION
+fi
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
--
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 4/5] log: handle integer overflow in timestamps
Jeff King p...@peff.net writes:
If an ident line has a ridiculous date value like (2^64)+1,
we currently just pass ULONG_MAX along to the date code,
which can produce nonsensical dates.
On systems with a signed long time_t (e.g., 64-bit glibc
systems), this actually doesn't end up too bad. The
ULONG_MAX is converted to -1, we apply the timezone field to
that, and the result ends up somewhere between Dec 31, 1969
and Jan 1, 1970.
...
We also recognize overflow in the timezone field, which
could produce nonsensical results. In this case we show the
parsed date, but in UTC.
Both are good measures to fallback to sanity, but why is that
if/else? In other words...
+ if (date_overflows(date))
+ date = 0;
+ else {
+ if (ident-tz_begin ident-tz_end)
+ tz = strtol(ident-tz_begin, NULL, 10);
+ if (tz == LONG_MAX || tz == LONG_MIN)
+ tz = 0;
+ }
... don't we want to fix an input having a bogus timestamp and also
a bogus tz recorded in it?
return show_date(date, tz, mode);
}
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index 83de981..ba25a2e 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -65,4 +65,20 @@ test_expect_success 'unparsable dates produce sentinel
value (%ad)' '
test_cmp expect actual
'
+# date is 2^64 + 1
+test_expect_success 'date parser recognizes integer overflow' '
+ commit=$(munge_author_date HEAD 18446744073709551617)
+ echo Thu Jan 1 00:00:00 1970 + expect
+ git log -1 --format=%ad $commit actual
+ test_cmp expect actual
+'
+
+# date is 2^64 - 2
+test_expect_success 'date parser recognizes time_t overflow' '
+ commit=$(munge_author_date HEAD 18446744073709551614)
+ echo Thu Jan 1 00:00:00 1970 + expect
+ git log -1 --format=%ad $commit actual
+ test_cmp expect actual
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Easier access to index-v4
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Thomas Gummerer t.gumme...@gmail.com writes:
previous round was at $gmane/242198.
Since then I've squashed the fixes suggested by Junio, added a test
showing what should happen if an index file is present and
GIT_INDEX_VERSION is set and fixed the typo found by Eric.
Looks good; thanks.
Tests, seem to leak these unnecessary diag (not limited to t0010).
sh t0010-racy-git.sh
warning: GIT_INDEX_VERSION set, but the value is invalid.
Using version 3
ok 1 - Racy GIT trial #0 part A
ok 2 - Racy GIT trial #0 part B
warning: GIT_INDEX_VERSION set, but the value is invalid.
Using version 3
...
# passed all 10 test(s)
1..10
The same thing under prove.
*** prove ***
t0010-racy-git.sh .. warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 2/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 4/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 6/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. 8/? warning: GIT_INDEX_VERSION set, but the value is
invalid.
Using version 3
t0010-racy-git.sh .. ok
All tests successful.
I suspect the real culprit is the early part in test-lib.sh that
sets GIT_INDEX_VERSION explicitly from TEST_GIT_INDEX_VERSION when
the latter is not even specified.
Something along this line, perhaps?
Sorry about this, I didn't run the test suite without
TEST_GIT_INDEX_VERSION in config.mak which I obviously should have.
Yes, this looks good to me, thanks!
t/test-lib.sh | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 492f81f..01a98cb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -108,8 +108,11 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
export EDITOR
-GIT_INDEX_VERSION=$TEST_GIT_INDEX_VERSION
-export GIT_INDEX_VERSION
+if test -n ${TEST_GIT_INDEX_VERSION+isset}
+then
+ GIT_INDEX_VERSION=$TEST_GIT_INDEX_VERSION
+ export GIT_INDEX_VERSION
+fi
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
--
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 4/5] log: handle integer overflow in timestamps
On Mon, Feb 24, 2014 at 11:50:00AM -0800, Junio C Hamano wrote:
We also recognize overflow in the timezone field, which
could produce nonsensical results. In this case we show the
parsed date, but in UTC.
Both are good measures to fallback to sanity, but why is that
if/else? In other words...
+ if (date_overflows(date))
+ date = 0;
+ else {
+ if (ident-tz_begin ident-tz_end)
+ tz = strtol(ident-tz_begin, NULL, 10);
+ if (tz == LONG_MAX || tz == LONG_MIN)
+ tz = 0;
+ }
... don't we want to fix an input having a bogus timestamp and also
a bogus tz recorded in it?
If there is a bogus timestamp, then we do not want to look at tz at all.
We leave it at 0, so that we get a true sentinel:
Thu Jan 1 00:00:00 1970 +
and not:
Wed Dec 31 19:00:00 1969 -0500
If the timestamp is valid, then we bother to parse the zone, and handle
overflow there.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Document a bunch of functions defined in sha1_file.c
On 02/24/2014 07:18 PM, Jakub Narębski wrote: Michael Haggerty wrote: -/* - * NOTE! This returns a statically allocated buffer, so you have to be - * careful about using it. Do an xstrdup() if you need to save the - * filename. - * - * Also note that this returns the location for creating. Reading - * SHA1 file can happen from any alternate directory listed in the - * DB_ENVIRONMENT environment variable if it is not found in - * the primary object database. - */ const char *sha1_file_name(const unsigned char *sha1) Has this changed? No, this hasn't changed. I've been documenting public functions in the header files above the declaration, and private ones where they are defined. So I moved the documentation for this function to cache.h: +/* + * Return the name of the file in the local object database that would + * be used to store a loose object with the specified sha1. The + * return value is a pointer to a statically allocated buffer that is + * overwritten each time the function is called. + */ extern const char *sha1_file_name(const unsigned char *sha1); I also rewrite the comment, as you can see. The NOTE! seemed a bit overboard to me, given that there are a lot of functions in our codebase that behave similarly. So I toned the warning down, and tightened up the comment overall. Let me know if you think I've made it less helpful. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git am and mangled subject lines
Hi Phillip, Phillip Susi wrote: git am already ignores the [PATCH X/Y] prefix that format-patch adds. Is it possible to get it to ignore any additional prefix that a bug tracker mangles into the subject line? i.e. bug #:? builtin/mailinfo.c is the place to start (see git-mailinfo(1)). This is a little tricky because some people *like* the bug #: in the subject line for a commit. Hope that helps, Jonathan -- 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 5/6] Document a bunch of functions defined in sha1_file.c
Hi, Michael Haggerty wrote: No, this hasn't changed. I've been documenting public functions in the header files above the declaration, and private ones where they are defined. So I moved the documentation for this function to cache.h: +/* + * Return the name of the file in the local object database that would + * be used to store a loose object with the specified sha1. The + * return value is a pointer to a statically allocated buffer that is + * overwritten each time the function is called. + */ extern const char *sha1_file_name(const unsigned char *sha1); I also rewrite the comment, as you can see. The NOTE! seemed a bit overboard to me, given that there are a lot of functions in our codebase that behave similarly. So I toned the warning down, and tightened up the comment overall. Let me know if you think I've made it less helpful. In the present state of the codebase, where many important functions have no documentation or out-of-date documentation, the first place I look to understand a function is its point of definition. So I do think that moving docs to the header file makes it less helpful. I'd prefer a mass move in the opposite direction (from header files to the point of definition). On the other hand I don't feel strongly about it. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git am and mangled subject lines
Phillip Susi ps...@ubuntu.com writes: git am already ignores the [PATCH X/Y] prefix that format-patch adds. Is it possible to get it to ignore any additional prefix that a bug tracker mangles into the subject line? i.e. bug #:? I think applypatch-msg hook is your friend in a case like this. -- 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 4/5] log: handle integer overflow in timestamps
Jeff King p...@peff.net writes:
On Mon, Feb 24, 2014 at 11:50:00AM -0800, Junio C Hamano wrote:
We also recognize overflow in the timezone field, which
could produce nonsensical results. In this case we show the
parsed date, but in UTC.
Both are good measures to fallback to sanity, but why is that
if/else? In other words...
+ if (date_overflows(date))
+ date = 0;
+ else {
+ if (ident-tz_begin ident-tz_end)
+ tz = strtol(ident-tz_begin, NULL, 10);
+ if (tz == LONG_MAX || tz == LONG_MIN)
+ tz = 0;
+ }
... don't we want to fix an input having a bogus timestamp and also
a bogus tz recorded in it?
If there is a bogus timestamp, then we do not want to look at tz at all.
We leave it at 0, so that we get a true sentinel:
Ah, OK, I missed the initialization to 0 at the beginning.
It might have been more clear if int tz declaration were left
uninitialized, and the variable were explicitly cleared to 0 in the
date-overflows error codepath, but it is not a big deal.
Thanks for clarification.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git am and mangled subject lines
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2/24/2014 3:19 PM, Junio C Hamano wrote: Phillip Susi ps...@ubuntu.com writes: git am already ignores the [PATCH X/Y] prefix that format-patch adds. Is it possible to get it to ignore any additional prefix that a bug tracker mangles into the subject line? i.e. bug #:? I think applypatch-msg hook is your friend in a case like this. Can you point me in the direction of some documentation on this? I don't see it mentioned in the man pages for git am or mailinfo ( I would think that would be the place to have it ). -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTC6pvAAoJEI5FoCIzSKrwQpYH/iJ8RPqEoIoO+GBU2HxmCfEZ Tlo1qwbvw26ALY71/WQFtVKW+3Bh1XYpAs0rsL5tpCJw/EMgAEm1cPFASf+BHcRI NO57NBdk9we+hvUju+o6/It5e6OrYj/im+nI/AFfenRsbwPj8/S0MoMiP4jOEVkW tTSCEeC5ldR4IbxBfphbV7me79Sk8nZssXTFmWJEv80H41LdMd8ZThR4ZFCcyzUR ifAflWHN7dj3uwY0Lr+OdCRrPw3qU1LXRjKK2SHBTG1Qk4uJW3sFJKHTupAipOEQ KKQ0QP/4WQWCSjq+8El4jnnlf++KAwbbOnAVYkbeKzy/4KOxJX7pimDypJHYGp8= =LZvc -END PGP SIGNATURE- -- 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] remote: handle pushremote config in any order order
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: Yes, with a few exceptions, we usually try to make the ordering in the config file irrelevant. This is a bug. The patch below should fix it. Looks good. Thanks. diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 926e7f6..1309c4d 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -536,6 +536,18 @@ test_expect_success 'push with config branch.*.pushremote' ' check_push_result down_repo $the_commit heads/master ' +test_expect_success 'branch.*.pushremote config order is irrelevant' ' +mk_test one_repo heads/master +mk_test two_repo heads/master +test_config remote.one.url one_repo +test_config remote.two.url two_repo +test_config branch.master.pushremote two_repo +test_config remote.pushdefault one_repo +git push +check_push_result one_repo $the_first_commit heads/master +check_push_result two_repo $the_commit heads/master +' + This test however does not pass in the Git 2.0 world, without having this line: test_config push.default matching immediately before git push. Am I missing something? -- 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 4/5] log: handle integer overflow in timestamps
On Mon, Feb 24, 2014 at 12:21:33PM -0800, Junio C Hamano wrote:
+if (date_overflows(date))
+date = 0;
+else {
+if (ident-tz_begin ident-tz_end)
+tz = strtol(ident-tz_begin, NULL, 10);
+if (tz == LONG_MAX || tz == LONG_MIN)
+tz = 0;
+}
... don't we want to fix an input having a bogus timestamp and also
a bogus tz recorded in it?
If there is a bogus timestamp, then we do not want to look at tz at all.
We leave it at 0, so that we get a true sentinel:
Ah, OK, I missed the initialization to 0 at the beginning.
It might have been more clear if int tz declaration were left
uninitialized, and the variable were explicitly cleared to 0 in the
date-overflows error codepath, but it is not a big deal.
It might be, but I think it would end up cumbersome. The initialization
was already there from the previous version, which was hitting the else
for ident-tz_begin. Without fallback initializations, you end up with:
if (ident-date_begin ident-date_end) {
date = strtoul(ident-date_begin, NULL, 10);
if (date_overflows(date)) {
date = 0;
tz = 0;
} else {
if (ident-tz_begin ident-tz_end) {
tz = strtol(ident-tz_begin, NULL, 10);
if (tz == LONG_MAX || tz == LONG_MIN)
tz = 0;
} else
tz = 0;
}
} else {
date = 0;
tz = 0;
}
which I think is much more confusing (and hard to verify that the
variables are always set). Checking !date as an error condition would
make it a little more readable:
if (ident-date_begin ident-date_end) {
date = strtoul(ident-date_begin, NULL, 10);
if (date_overflows(date))
date = 0;
} else
date = 0;
if (date) {
if (ident-tz_begin ident-tz_end) {
tz = strtol(ident-tz_begin, NULL, 10);
if (tz == LONG_MAX || tz == LONG_MIN)
tz = 0;
} else
tz = 0;
} else
tz = 0;
but then we treat date==0 as a sentinel, and can never correctly parse
dates on Jan 1, 1970.
So I'd be in favor of keeping it as-is, but feel free to mark it up if
you feel strongly.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git am and mangled subject lines
Hi, Phillip Susi wrote: On 2/24/2014 3:19 PM, Junio C Hamano wrote: Phillip Susi ps...@ubuntu.com writes: git am already ignores the [PATCH X/Y] prefix that format-patch adds. Is it possible to get it to ignore any additional prefix that a bug tracker mangles into the subject line? i.e. bug #:? I think applypatch-msg hook is your friend in a case like this. Can you point me in the direction of some documentation on this? I don't see it mentioned in the man pages for git am or mailinfo ( I would think that would be the place to have it ). Gladly. Thanks for noticing. -- 8 -- Subject: am doc: add a pointer to relevant hooks It is not obvious when looking at a new command what hooks will affect it. Add a HOOKS section to the git-am(1) page, imitating git-commit(1), to make it easier for people to discover e.g. the applypatch-msg hook that can implement a custom subject-mangling strategy (e.g., removing a bug #: prefix introduced by a bug tracker). Reported-by: Phillip Susi ps...@ubuntu.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Documentation/git-am.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 54d8461..abcffb6 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -189,6 +189,11 @@ commits, like running 'git am' on the wrong branch or an error in the commits that is more easily fixed by changing the mailbox (e.g. errors in the From: lines). +HOOKS +- +This command can run `applypatch-msg`, `pre-applypatch`, +and `post-applypatch` hooks. See linkgit:githooks[5] for more +information. SEE ALSO -- 1.9.0.rc1.175.g0b1dcb5 -- 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] remote: handle pushremote config in any order order
On Mon, Feb 24, 2014 at 12:32:32PM -0800, Junio C Hamano wrote: +test_expect_success 'branch.*.pushremote config order is irrelevant' ' + mk_test one_repo heads/master + mk_test two_repo heads/master + test_config remote.one.url one_repo + test_config remote.two.url two_repo + test_config branch.master.pushremote two_repo + test_config remote.pushdefault one_repo + git push + check_push_result one_repo $the_first_commit heads/master + check_push_result two_repo $the_commit heads/master +' + This test however does not pass in the Git 2.0 world, without having this line: test_config push.default matching immediately before git push. Am I missing something? No, you are not missing anything. I was copying and paring down the pushremote test above, and I accidentally pared out the push.default setting. It should definitely have a test_config push.default matching before the git push line, as the test above does. Can you mark it up as you apply? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote: handle pushremote config in any order order
Jeff King p...@peff.net writes: On Mon, Feb 24, 2014 at 12:32:32PM -0800, Junio C Hamano wrote: +test_expect_success 'branch.*.pushremote config order is irrelevant' ' + mk_test one_repo heads/master + mk_test two_repo heads/master + test_config remote.one.url one_repo + test_config remote.two.url two_repo + test_config branch.master.pushremote two_repo + test_config remote.pushdefault one_repo + git push + check_push_result one_repo $the_first_commit heads/master + check_push_result two_repo $the_commit heads/master +' + This test however does not pass in the Git 2.0 world, without having this line: test_config push.default matching immediately before git push. Am I missing something? No, you are not missing anything. I was copying and paring down the pushremote test above, and I accidentally pared out the push.default setting. It should definitely have a test_config push.default matching before the git push line, as the test above does. Can you mark it up as you apply? Gladly ;-) I wasn't thinking straight and thought push.default was somehow affecting the logic to read the configuration files you fixed, which was a complete nonsense. The selection of which remote to push to is affected by the branch.*.pushremote and remote.pushdefault, but this git push still expects that the way the branches are chosen to be pushed follow the matching semantics, not the simple semantics, so we need that configuration there. -- 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 4/5] log: handle integer overflow in timestamps
Jeff King p...@peff.net writes:
On Mon, Feb 24, 2014 at 12:21:33PM -0800, Junio C Hamano wrote:
+ if (date_overflows(date))
+ date = 0;
+ else {
+ if (ident-tz_begin ident-tz_end)
+ tz = strtol(ident-tz_begin, NULL, 10);
+ if (tz == LONG_MAX || tz == LONG_MIN)
+ tz = 0;
+ }
... don't we want to fix an input having a bogus timestamp and also
a bogus tz recorded in it?
If there is a bogus timestamp, then we do not want to look at tz at all.
We leave it at 0, so that we get a true sentinel:
Ah, OK, I missed the initialization to 0 at the beginning.
It might have been more clear if int tz declaration were left
uninitialized, and the variable were explicitly cleared to 0 in the
date-overflows error codepath, but it is not a big deal.
It might be, but I think it would end up cumbersome.
...
So I'd be in favor of keeping it as-is, but feel free to mark it up if
you feel strongly.
I'd be in favor of keeping it as-is.
--
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] difftool: support repositories with .git-files
Am 24.02.2014 17:55, schrieb Junio C Hamano: David Aguilar dav...@gmail.com writes: Modern versions of git submodule use .git-files to setup the submodule directory. When run in a git submodule-created repository git difftool --dir-diff dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták gabor.lip...@gmail.com Helped-by: Jens Lehmann jens.lehm...@web.de Helped-by: John Keeping j...@keeping.me.uk Signed-off-by: David Aguilar dav...@gmail.com --- Looks good; thanks. FWIW: Tested-by: Jens Lehmann jens.lehm...@web.de What about squashing this in to detect any future regressions? diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..d86ad68 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git difftool --tool=echo --dir-diff --cached + ) +' + 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 v2 0/3] Easier access to index-v4
Thomas Gummerer t.gumme...@gmail.com writes: Something along this line, perhaps? Sorry about this, I didn't run the test suite without TEST_GIT_INDEX_VERSION in config.mak which I obviously should have. Yes, this looks good to me, thanks! OK, will squash it (but using VAR:+isset instead of VAR+isset to allow people to set it to empty to disable) into the relevant patch. 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] clone: allow initial sparse checkouts
On Mon, Feb 24, 2014 at 09:47:16AM -0800, Junio C Hamano wrote: Robin H. Johnson robb...@gentoo.org writes: The only other clean alternative would be implementing ONLY --sparse-checkout-from, and letting uses use fds creatively: --sparse-checkout-from (echo X; echo Y) Not all POSIX shells have such an abomination that is process substitution. You can easily work it around by adopting the usual convention to use - to read from the standasrd input, though. (echo X; echo Y) | cmd --sparse-checkout-from - Is that a vote that you'd like to see a --sparse-checkout-from variant of my patch? -- Robin Hugh Johnson Gentoo Linux: Developer, Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 -- 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] commit: add --cleanup=scissors
On Tue, Feb 25, 2014 at 12:20 AM, Junio C Hamano gits...@pobox.com wrote:
Duy Nguyen pclo...@gmail.com writes:
On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote:
@@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_(Please enter the commit message for your
changes.
Lines starting\nwith '%c' will be ignored,
and an empty
message aborts the commit.\n),
comment_line_char);
+else if (cleanup_mode == CLEANUP_SCISSORS)
+wt_status_add_cut_line(s-fp);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
_(Please enter the commit message for your
changes.
This cut line does not cover the merge conflict message before it. The
following patch should be squashed in to move the cut line up in that
case.
I somehow thought that it was a policy decision we made in very
early days that these conflict messages are meant to be edited with
explanation of how they were resolved, not to be simply edited away?
The other stuff (commented out instructions and patch text) are
meant to aid humans while editing the log message, and stripping
away automatically after they are done editing like your patch does
is perfectly fine, but I find this change questionable.
I think I described it incorrectly as conflict message while it's
not. This is the part to be cut out by the patch
# It looks like you may be committing a merge.
# If this is not correct, please remove the file.
# MERGE_HEAD
# and try again.
(similar message for CHERRY_PICK_HEAD).
-- 8 --
diff --git a/builtin/commit.c b/builtin/commit.c
index ea2912f..1033c50 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor include_status) {
char *ai_tmp, *ci_tmp;
- if (whence != FROM_COMMIT)
+ if (whence != FROM_COMMIT) {
+ if (cleanup_mode == CLEANUP_SCISSORS)
+ wt_status_add_cut_line(s-fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
? _(\n
@@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
git_path(whence == FROM_MERGE
? MERGE_HEAD
: CHERRY_PICK_HEAD));
+ }
fprintf(s-fp, \n);
if (cleanup_mode == CLEANUP_ALL)
@@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_(Please enter the commit message for your
changes.
Lines starting\nwith '%c' will be ignored,
and an empty
message aborts the commit.\n),
comment_line_char);
- else if (cleanup_mode == CLEANUP_SCISSORS)
+ else if (cleanup_mode == CLEANUP_SCISSORS whence ==
FROM_COMMIT)
wt_status_add_cut_line(s-fp);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
-- 8 --
--
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 v2] tag: support --sort=spec
On Mon, Feb 24, 2014 at 11:39 PM, Junio C Hamano gits...@pobox.com wrote: diff --git a/t/test-lib.sh b/t/test-lib.sh index 1531c24..5e8c39a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -771,6 +771,8 @@ case $(uname -s) in ;; esac +[ $(uname -o) = GNU/Linux ] test_set_prereq GNULINUX + can pick up the value from GIT-BUILD-OPTIONS as a prerequisite (see the way we handle NO_PERL for an example). Though if we can just grab the glibc version as a fallback, we can do away with that completely. ;-) I like that. Jeff, I'm still waiting if you agree to go with this syntax or put version before refname before rerolling (either with build time option like this, or implement the compat function myself). -- 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 v2] tag: support --sort=spec
On Tue, Feb 25, 2014 at 06:30:35AM +0700, Duy Nguyen wrote: +[ $(uname -o) = GNU/Linux ] test_set_prereq GNULINUX + can pick up the value from GIT-BUILD-OPTIONS as a prerequisite (see the way we handle NO_PERL for an example). Though if we can just grab the glibc version as a fallback, we can do away with that completely. ;-) I like that. Jeff, I'm still waiting if you agree to go with this syntax or put version before refname before rerolling (either with build time option like this, or implement the compat function myself). Sorry. I didn't respond because I was trying to think of something to say besides no, I still like mine better. I admit that it's mostly a gut feeling, and I don't have any argument beyond what I've already made (your it's like a typecast does make some sense, but I think that is a convoluted way of thinking about it). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/19] Multiparent diff tree-walker + combine-diff speedup
On Mon, Feb 24, 2014 at 11:21 PM, Kirill Smelkov k...@mns.spb.ru wrote: Hello up there. Here go combine-diff speedup patches in form of first reworking diff tree-walker to work in general case - when a commit have several parents, not only one - we are traversing all 1+nparent trees in parallel. Then we are taking advantage of the new diff tree-walker for speeding up combine-diff, which for linux.git results in ~14 times speedup. I think there is another use case for this n-tree walker (but I'm not entirely sure yet as I haven't really read the series). In git-log (either with pathspec or --patch) we basically do this diff HEAD^ HEAD diff HEAD^^ HEAD^ diff HEAD^^^ HEAD^^ diff HEAD HEAD^^^ ... so except HEAD (and the last commit), all commits' tree will be read/diff'd twice. With n-tree walker I think we may be able to diff them in batch to reduce extra processing: commit lists are split into 16-commit blocks where 16 trees are fed to the new tree walker at the same time. I hope it would make git-log a bit faster (especially for -S). Maybe not much. -- 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 v2 3/3] commit: add --cleanup=scissors
Duy Nguyen pclo...@gmail.com writes:
I think I described it incorrectly as conflict message while it's
not. This is the part to be cut out by the patch
# It looks like you may be committing a merge.
# If this is not correct, please remove the file.
# MERGE_HEAD
# and try again.
(similar message for CHERRY_PICK_HEAD).
Ahh, OK. That should be removed, of course.
Will squash in (but not tonight). Thanks.
-- 8 --
diff --git a/builtin/commit.c b/builtin/commit.c
index ea2912f..1033c50 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor include_status) {
char *ai_tmp, *ci_tmp;
- if (whence != FROM_COMMIT)
+ if (whence != FROM_COMMIT) {
+ if (cleanup_mode == CLEANUP_SCISSORS)
+ wt_status_add_cut_line(s-fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
? _(\n
@@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
git_path(whence == FROM_MERGE
? MERGE_HEAD
: CHERRY_PICK_HEAD));
+ }
fprintf(s-fp, \n);
if (cleanup_mode == CLEANUP_ALL)
@@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_(Please enter the commit message for your
changes.
Lines starting\nwith '%c' will be
ignored, and an empty
message aborts the commit.\n),
comment_line_char);
- else if (cleanup_mode == CLEANUP_SCISSORS)
+ else if (cleanup_mode == CLEANUP_SCISSORS whence ==
FROM_COMMIT)
wt_status_add_cut_line(s-fp);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
-- 8 --
--
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] clone: allow initial sparse checkouts
Robin H. Johnson robb...@gentoo.org writes: On Mon, Feb 24, 2014 at 09:47:16AM -0800, Junio C Hamano wrote: Robin H. Johnson robb...@gentoo.org writes: The only other clean alternative would be implementing ONLY --sparse-checkout-from, and letting uses use fds creatively: --sparse-checkout-from (echo X; echo Y) Not all POSIX shells have such an abomination that is process substitution. You can easily work it around by adopting the usual convention to use - to read from the standasrd input, though. (echo X; echo Y) | cmd --sparse-checkout-from - Is that a vote that you'd like to see a --sparse-checkout-from variant of my patch? Honestly, I do not particularly care too much about this feature, regardless of the interface [*1*]. It is just a vote that says if --something-from form is going to be implemented, it should be able to read from the standard input with '-', unless there is a compelling reason not to do so. [Footnote] *1* In the longer term, I think sparse checkout is broken as a concept and I view this use sparse checkout from the get-go merely a stop-gap measure to make the band-aid a bit less painful to use. What you really want is a narrow clone, which is conceptually cleaner but a lot harder implentation-wise. Not that keeping a band-aid usable longer is necessarily bad in the real world, though---so even I said *I*'m not interested, that is different from saying I'm not taking a patch on this topic. -- 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
[FYI] 'next' will be rewound shortly
Just a quick heads-up. As the first wave for the post-1.9.0 development cycle, these topics will be merged to 'master' (unless there are serious last minute issues found) shortly: + kb/fast-hashmap 02-24/01-03 #18 + nv/commit-gpgsign-config 02-24/01-03 #3 + da/pull-ff-configuration 01-15/01-22 #2 + bk/refresh-missing-ok-in-merge-recursive 02-24/01-29 #4 + ds/rev-parse-required-args 01-28/01-31 #1 + jk/config-path-include-fix 01-28/01-31 #2 + bs/stdio-undef-before-redef 01-31/01-31 #1 + ep/varscope 01-31/01-31 #7 + nd/submodule-pathspec-ending-with-slash 02-24/01-31 #8 + nd/diff-quiet-stat-dirty 02-24/01-31 #2 + nd/test-rename-reset 02-04/02-06 #1 + mw/symlinks 02-04/02-06 #6 + ks/tree-diff-walk02-24/02-06 #5 + wk/submodule-on-branch 02-24/02-06 #4 + nd/reset-intent-to-add 02-05/02-07 #1 + jk/test-ports02-10/02-13 #2 + al/docs 02-11/02-13 #4 + bc/gpg-sign-everywhere 02-11/02-13 #9 + jk/pack-bitmap 02-12/02-13 #26 + nd/http-fetch-shallow-fix02-13/02-13 #7 + dk/blame-janitorial 02-24/02-13 #4 and the tip of 'next' will be rewound at the same time (like, say, tomorrow). We may want to merge the 2.0.0 transition topics as the second wave to 'master' (and starting Documentation/RelNotes/2.0.0.txt) before merging topics other than the above to it. There are about 30 more topics that are ready for 'next' (not counting the ones for 2.0.0 transition), all of which are likely to be in 'next' soonish. + nd/daemonize-gc 02-10/02-20 #2 + jk/run-network-tests-by-default 02-14/02-20 #1 + ks/combine-diff 02-24/02-20 #6 - jc/check-attr-honor-working-tree 02-06 #2 - nd/gitignore-trailing-whitespace 02-10 #2 - ss/completion-rec-sub-fetch-push 02-11 #1 - jk/http-no-curl-easy 02-18 #1 - jk/janitorial-fixes 02-18 #5 - ks/config-file-stdin 02-18 #4 - lb/contrib-contacts-looser-diff-parsing 02-18 #1 - nd/reset-setup-worktree 02-18 #1 - tr/diff-submodule-no-reuse-worktree 02-18 #1 - ak/gitweb-fit-image 02-20 #1 - mh/replace-refs-variable-rename 02-20 #1 - nd/no-more-fnmatch 02-20 #4 - rt/links-for-asciidoctor 02-20 #1 - jh/note-trees-record-blobs 02-20 #1 - da/difftool-git-files02-24 #1 - jk/commit-dates-parsing-fix 02-24 #5 - jk/remote-pushremote-config-reading 02-24 #1 - jm/stash-doc-k-for-keep 02-24 #1 - jn/am-doc-hooks 02-24 #1 - mh/object-code-cleanup 02-24 #4 - nd/i18n-progress 02-24 #1 - fc/transport-helper-fixes02-24 #7 - tg/index-v4-format 02-24 #3 - ks/tree-diff-more02-24 #15 I may be a bit too busy to be able to pick up new topics for a few days, as the topic-flipping is usually quite time consuming when some ordering of the topics change, i.e. when rerere stops to be helpful. 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
[PATCH] help: include list of aliases in git-help --all
Git help --all had listed all git commands, but no configured aliases.
This includes aliases as a separate listing, after commands in the main
git directory and other $PATH directories.
Signed-off-by: Joel Nothman joel.nothman at gmail.com
---
Documentation/git-help.txt | 4 +--
builtin/help.c | 7 ++---
help.c | 64 +++---
help.h | 7 -
4 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index b21e9d7..c9fe791 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -40,8 +40,8 @@ OPTIONS
---
-a::
--all::
- Prints all the available commands on the standard output. This
- option overrides any given command or guide name.
+ Prints all the available commands and aliases on the standard output.
+ This option overrides any given command or guide name.
-g::
--guides::
diff --git a/builtin/help.c b/builtin/help.c
index 1fdefeb..d1dfecd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static int show_guides = 0;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static struct option builtin_help_options[] = {
- OPT_BOOL('a', all, show_all, N_(print all available commands)),
+ OPT_BOOL('a', all, show_all, N_(print all available commands and
aliases)),
OPT_BOOL('g', guides, show_guides, N_(print list of useful
guides)),
OPT_SET_INT('m', man, help_format, N_(show man page),
HELP_FORMAT_MAN),
OPT_SET_INT('w', web, help_format, N_(show manual in web browser),
@@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
int nongit;
const char *alias;
enum help_format parsed_help_format;
+ struct cmdnames alias_cmds;
argc = parse_options(argc, argv, prefix, builtin_help_options,
builtin_help_usage, 0);
@@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
if (show_all) {
git_config(git_help_config, NULL);
printf(_(usage: %s%s), _(git_usage_string), \n\n);
- load_command_list(git-, main_cmds, other_cmds);
- list_commands(colopts, main_cmds, other_cmds);
+ load_commands_and_aliases(git-, main_cmds, other_cmds,
alias_cmds);
+ list_commands(colopts, main_cmds, other_cmds, alias_cmds);
}
if (show_guides)
diff --git a/help.c b/help.c
index df7d16d..3c14af4 100644
--- a/help.c
+++ b/help.c
@@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
int i;
for (i = 0; i cmds-cnt; i++)
- string_list_append(list, cmds-names[i]-name);
+ string_list_append(list, cmds-names[i]-name);
/*
* always enable column display, we only consult column.*
* about layout strategy and stuff
@@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
exclude_cmds(other_cmds, main_cmds);
}
+static struct cmdnames aliases;
+
+static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
+{
+ int i;
+ ALLOC_GROW(cmds-names, cmds-cnt + old-cnt, cmds-alloc);
+
+ for (i = 0; i old-cnt; i++)
+ cmds-names[cmds-cnt++] = old-names[i];
+ free(old-names);
+ old-cnt = 0;
+ old-names = NULL;
+}
+
+static int load_aliases_cb(const char *var, const char *value, void *cb)
+{
+ if (starts_with(var, alias.))
+ add_cmdname(aliases, var + 6, strlen(var + 6));
+ return 0;
+}
+
+void load_commands_and_aliases(const char *prefix,
+ struct cmdnames *main_cmds,
+ struct cmdnames *other_cmds,
+ struct cmdnames *alias_cmds)
+{
+ load_command_list(prefix, main_cmds, other_cmds);
+
+ /* reuses global aliases from unknown command functionality */
+ git_config(load_aliases_cb, NULL);
+
+ add_cmd_list(alias_cmds, aliases);
+ qsort(alias_cmds-names, alias_cmds-cnt,
+ sizeof(*alias_cmds-names), cmdname_compare);
+ uniq(alias_cmds);
+ exclude_cmds(alias_cmds, main_cmds);
+ exclude_cmds(alias_cmds, other_cmds);
+}
+
void list_commands(unsigned int colopts,
- struct cmdnames *main_cmds, struct cmdnames *other_cmds)
+ struct cmdnames *main_cmds, struct cmdnames *other_cmds,
+ struct cmdnames *alias_cmds)
{
if (main_cmds-cnt) {
const char *exec_path = git_exec_path();
@@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
pretty_print_string_list(other_cmds, colopts);
putchar('\n');
}
+
+ if (alias_cmds-cnt) {
+ printf_ln(_(aliases defined in git configuration));
+ putchar('\n');
+
