Re: [PATCH 03/13] Makefile: introduce make-var helper function
On Wed, Feb 5, 2014 at 12:50 PM, Jeff King p...@peff.net wrote: It's a common pattern in our Makefile to echo some make variables into a file, but only if they are different from a previous run. This sentinel file can then be used as a dependency to trigger rebuilds when the make variable changes. The code to do this is a bit ugly and repetetive; let's s/repetetive/repetitive/ factor it out into a reusable function. Note that this relies on the call and eval functions of GNU make. We previously avoided using call, as explained in 39c015c (Fixes for ancient versions of GNU make, 2006-02-18). However, it has been 8 years since then, so perhaps its use is acceptable now. The call function dates back to GNU make 3.77.90 (1997-07-21). The eval function dates back to 3.80 (2002-07-08). If it's still a problem to use these functions, we can do similar meta-programming with something like: include magic.mak magic.mak: ./generate-magic-rules $@+ mv $@+ $@ where the rules are generated by a shell (or other) script. Signed-off-by: Jeff King p...@peff.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] hackday and GSoC topic suggestions
On Wed, Feb 5, 2014 at 11:57 PM, Jeff King p...@peff.net wrote: On a similar note, the GSoC application deadline is Feb 14th. I am happy to be admin again and put together the application, but we will need an idea page. I'll set up a page to collect them, but in the meantime, please dump any ideas/discussion in this thread. Matthieu Moy created this nice page some time ago in the wiki: https://git.wiki.kernel.org/index.php/SmallProjectsIdeas and I think there is good stuff in it. I would be interested in mentoring a GSoC student working on the git bisect fix/unfixed feature. Some of Matthieu's students worked on it a few years ago but didn't finish. 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: [RFH] hackday and GSoC topic suggestions
Christian Couder christian.cou...@gmail.com writes: I would be interested in mentoring a GSoC student working on the git bisect fix/unfixed feature. Interestingly, I used the feature in real-life last week, and had to think upside-down to type the right good/bad keywords ;-). Some of Matthieu's students worked on it a few years ago but didn't finish. Right. There was still quite some work to do, but this is most likely too small for a GSoC project. But that could be a part of it. I'm not sure how google welcomes GSoC projects made of multiple small tasks, but my experience with students is that it's much better than a single (too) big task, and I think that was the general feeling on this list when we discussed it last year. -- 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
Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
Junio C Hamano gits...@pobox.com writes:
David Kastrup d...@gnu.org writes:
It's snake oil making debugging harder.
OK, that is a sensible argument.
This was fun ;-)
At the expense of seriously impacting my motivation to do any further
code cleanup on Git.
Well, I said it was fun because I was learning from a new person
who haven't made much technical or code aethesitics discussion here
so far. If teaching others frustrates you and stops contributing to
the project, that is a loss.
So let's post something noteworthy technical: the current code iteration
sported the following routine:
static struct blame_entry *blame_merge(struct blame_entry *list1,
struct blame_entry *list2)
{
struct blame_entry *p1 = list1, *p2 = list2,
**tail = list1;
if (!p1)
return p2;
if (!p2)
return p1;
if (p1-s_lno = p2-s_lno) {
do {
tail = p1-next;
if ((p1 = *tail) == NULL) {
*tail = p2;
return list1;
}
} while (p1-s_lno = p2-s_lno);
}
for (;;) {
*tail = p2;
do {
tail = p2-next;
if ((p2 = *tail) == NULL) {
*tail = p1;
return list1;
}
} while (p1-s_lno p2-s_lno);
*tail = p1;
do {
tail = p1-next;
if ((p1 = *tail) == NULL) {
*tail = p2;
return list1;
}
} while (p1-s_lno = p2-s_lno);
}
}
This is decidedly more complex than the equivalent
static struct blame_entry *blame_merge(struct blame_entry *list1,
struct blame_entry *list2)
{
struct blame_entry *p1 = list1, *p2 = list2,
**tail = list1;
while (p1 p2) {
if (p1-s_lno = p2-s_lno) {
*tail = p1;
p1 = *(tail = p1-next);
} else {
*tail = p2;
p2 = *(tail = p2-next);
}
}
*tail = p1 ? p1 : p2;
return list1;
}
Why not use the latter one? Apart from the somewhat
obsessive-compulsive avoidance of checking the same condition twice in a
row (which nowadays compilers are pretty capable of taking into account
by themselves) the actually decisive difference is to avoid rewriting
links that are already pointing to the right place.
Those are the only write accesses that take place, and since we are
talking about _sorting_, it is to be expected that in the long run,
every record ends up in a cacheline different from its ultimate
neighbors. Also, assuming a more or less random distribution and
equally sized chains, about half of the links will already be correct.
In practice, the situation tends to be more degenerate, leading to a
_higher_ ratio of already correct links. Cutting down the expensive
writeout of cachelines by a factor of more than 2 makes quite a
difference in performance.
Does it matter here? Unlikely. Profiling tools tend to be useless for
seeing the impact of dirty cachelines since their operation
significantly interferes with the read/write patterns so this kind of
detail often escapes notice. But the total contribution of this stage
to the runtime of git blame will be insignificant either way.
new blood bringing in new ideas is fine, but I'd want to see those new
ideas backed by solid thiniking, and that means they may have to
explain themselves to those who are new to their ideas.
Well, there _is_ solid thinking behind the above code, but there _also_
is solid thinking behind the notion that it won't matter in this case.
I still prefer not pushing out code in the wild that I would hesitate to
employ in _all_ comparable circumstances. Consider it as a pattern or a
style: a lot of thinking went into it once, and what this should buy you
is to never have to think about this issue again.
C++ is better at actual code reuse, but with C you basically get pattern
reuse. Your head is instantiating the templates. And it's a good idea
not to crowd one's head with unnecessary and/or suboptimal patterns.
--
David Kastrup
--
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
Sparse checkout leaves no entry on working directory all the time on Windows 7 on Git 1.8.5.2.msysgit.0
Greetings. I've tried to perform a single simple task to fetch data from the linux-next integration testing tree repo and sparse checkout the drivers/staging/usbip/ directory. I managed to perform it successfully under Linux with Git 1.7.1. But I always failed to perform checkout under Windows 7 with Git 1.8.5.2.msysgit.0 facing the following error: Sparse checkout leaves no entry on working directory. Unfortunately, I haven't tried that under Linux with Git 1.8.5.2. So, don't know if there is the same problem. However I typed the checkout directory in file .git/info/sparse-checkout by using different formats with and without the leading and the trailing slashes, with and without asterisk after trailing slash, having tried all the possible combinations, but, all the same, nevertheless, the error occured. On Linux I stored the checkout directory in sparse-checkout file in the following way: drivers/staging/usbip/. And everything was fine under Linux, but not under Windows. However, I don't, of course, if the format of this directory really matters. May be the Git install on Windows 7 specific configuration is the reason? Could you, please, tell me, what might be wrong? Am I doing something wrong or it's a bug? Best Regards, Constantine Gorbunov -- 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/3] Add a function skip_prefix_if_present()
On Wed, Feb 5, 2014 at 1:55 PM, Michael Haggerty mhag...@alum.mit.edu wrote: * Duy seemed to offer to rewrite his patch series, but I don't think that it has happened yet. It's really low in my todo list. So if you want to pick it up, please do. -- 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: [BUG] shallow clones over http
(Digging back an old topic after Jeff mentioned it) On Thu, May 9, 2013 at 2:12 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: I'm trying to track down a protocol bug that happens with shallow clones over smart-http. As far as I can tell, the bug has existed in all versions. You can reproduce it using the attached repository, which is a shallow clone of https://github.com/mileszs/ack.vim.git, like: $ tar xzf repo.tar.gz $ cd repo.git $ git fetch --depth=10 fatal: git fetch-pack: expected shallow list In that test my fetch actually hit github.com as the upstream full repo, but you can also clone it down locally and demonstrate it with purely local copies of git (but it's more of a pain, because you have to set up a smart http server). The last part of the conversation looks like this: packet: fetch-pack packet: fetch-pack ACK f183a345a0c10caed7684d07dabae33e007c7590 common packet: fetch-pack have f183a345a0c10caed7684d07dabae33e007c7590 packet: fetch-pack ACK 33312d4db4e91468957b1b41dd039c5d88e85fda common packet: fetch-pack ACK 34d0b2fbc182b31d926632d170bc07d6a6fc3f9b common packet: fetch-pack ACK 45c802e07c60686986474b6b05b2c7048330b6b5 common packet: fetch-pack ACK e93f693fd2a9940d6421bf9e4ddd1f535994eaa5 common packet: fetch-pack ACK 132ee41e8e2c8c545b3aed120171e1596c9211a4 common packet: fetch-pack ACK 973deb3145a2638b2301cfd654721cf35d68 common packet: fetch-pack ACK e53a88a4e72d84562493313e8911ada4def787da common packet: fetch-pack ACK 90be0bf3eee6f7a0cb9c2377a50610f4ce738da3 common packet: fetch-pack ACK aeab88ccf41bf216fde37983bd403d9b913391e7 common packet: fetch-pack ACK 5f480935d3ce431c393657c3000337bcbdbd5535 common packet: fetch-pack ACK db81e01b433501b159983ea38690aeb01eea1e6b common packet: fetch-pack ACK 06c44b8cab93e780a29ff7f7b5b1dd41dba4b2d5 common packet: fetch-pack ACK 65f3966becdb2d931d5afbdcc6a28008d154668a common packet: fetch-pack ACK 10e8caef9f2ed308231ce1abc326c512e86a5d4c common packet: fetch-pack ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 common packet: fetch-pack ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 ready packet: fetch-pack NAK packet: fetch-pack ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 fatal: git fetch-pack: expected shallow list So we see that upload-pack sends a bunch of detailed ACKs, followed by a NAK, and then it sends another ACK. Fetch-pack is inside find_common, reading these acks. At the beginning of each stateless-rpc response, it expects to consume any shallow/unshallow lines up to a flush packet (the call to consume_shallow_list). And then it reads the acks in a loop. After it sees the NAK, it assumes that the server is done sending the packet, and loops again, expecting another set of shallow/unshallow lines. But we get the next ACK instead, and die. So who is wrong? Is upload-pack wrong to send an ACK after the NAK? 3e63b21aced1 (upload-pack: Implement no-done capability, 2011-03-14) claims that the above sequence of acks and naks is what upload-pack wants to show. What happens when you disable no-done extension handling on the server end, I wonder? fetch succeeded when no-done was disabled. An immediate workaround would be disable no-done in fetch-pack.c in a shallow repo but maybe we can do better.. -- 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
attr.c doesn't honor --work-tree option
Hi, It seems that code in attr.c does not honor the current work tree path (set by e.g. --work-tree ...) and simply always assumes CWD. When the current dir is not in the work tree, git will attempt to find .gitattributes under ./ instead of under the correct work tree. Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file(): $ cd /tmp/ $ mkdir -p attr-test/repo $ cd attr-test/repo $ git init Initialized empty Git repository in /tmp/attr-test/repo/.git/ $ echo 'dir/* filter=foo' .gitattributes $ Inside the working tree, it works: $ ~/src/git.git/git check-attr -a dir/file read_attr_from_file: /home/lasse/etc/gitattributes read_attr_from_file: /home/lasse/.config/git/attributes read_attr_from_file: .gitattributes push: read_attr_from_file: .git/info/attributes read_attr_from_file: dir/.gitattributes push: dir fill: filter = foo (dir/*) dir/file: filter: foo $ Outside, it fails to find the .gitattributes file: $ cd .. $ ~/src/git.git/git --work-tree /tmp/attr-test/repo --git-dir /tmp/attr-test/repo/.git check-attr -a dir/file read_attr_from_file: /home/lasse/etc/gitattributes read_attr_from_file: /home/lasse/.config/git/attributes read_attr_from_file: .gitattributes push: read_attr_from_file: /tmp/attr-test/repo/.git/info/attributes read_attr_from_file: dir/.gitattributes push: dir $ This is with the latest rev on master: $ ~/src/git.git/git --version git version 1.8.5.2.192.g7794a68.dirty $ It (sort of) works with a committed .gitattributes file because git will find it in the index, but that will still yield incorrect results if the .gitattributes file happens to be dirty. Looking at the code, I'm not really sure if this can be fixed in read_attr_from_file() by resolving relative paths against get_git_work_tree(). I doubt it's that simple though... Thoughts? /Lasse -- 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: Sparse checkout leaves no entry on working directory all the time on Windows 7 on Git 1.8.5.2.msysgit.0
Am 2/6/2014 12:54, schrieb konst...@ngs.ru: However I typed the checkout directory in file ..git/info/sparse-checkout by using different formats with and without the leading and the trailing slashes, with and without asterisk after trailing slash, having tried all the possible combinations, but, all the same, nevertheless, the error occured. Make sure that you do not use CRLF line terminators in the sparse-checkout file. -- Hannes -- 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: Sparse checkout leaves no entry on working directory all the time on Windows 7 on Git 1.8.5.2.msysgit.0
On Thu, Feb 6, 2014 at 8:20 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 2/6/2014 12:54, schrieb konst...@ngs.ru: However I typed the checkout directory in file ..git/info/sparse-checkout by using different formats with and without the leading and the trailing slashes, with and without asterisk after trailing slash, having tried all the possible combinations, but, all the same, nevertheless, the error occured. Make sure that you do not use CRLF line terminators in the sparse-checkout file. I suspected the same thing. But it looks like we do trim CRLF. Another option is the file is encoded in utf-16 or some encoding which ascii is not a subset of. -- 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
Confusing git log --- First time bug submission please advise on best practices
My co-worker has an inconsistent git log output. Please see that attached files for output (I've made a best effort to remove confidential info from them). Comparing the two log commands we can see that master and originssh/master have a shared common commit at John Doe (4 hours ago) d85832d More pom fixes The top commit for originssh/master and the second to top for master. I would expect that both logs would share an _identical_ history from that commit onward. But the log for master contains the following Jeremy Doe (27 hours ago) 239ea21 (my-work) renamed class Jeremy Doe (28 hours ago) 27750b2 Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework and Jeremy Doe (2 days ago) a933acb white space changes Jeremy Doe (2 days ago) b5e51e7 Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework Jeremy Doe (2 days ago) 3a0f787 removed public methods Jeremy Doe (2 days ago) 4e91130 added the xml deserialisation None of which appear in the originssh/master log. Is there a scenario in which this is expected. It was my understanding that any two commits with the same hash have exactly the same history. Thanks for your time. Technical details: He is using the Windows git client version 1.8.5.2.msysgit.0 Running on Windows 7 Every command was run through git bash Background: He previously had tortoise-git installed, but I had him uninstall it to solve some ssh problems While uninstalling tortoise-git we also reinstalled git - I don't know what the previous version of git was. Output of git branch -avv * master 6833fd4 Completed merge my-work 239ea21 renamed class remotes/origin/HEAD - origin/master remotes/origin/masterf269789 Cleaning up GIT step 1 remotes/originssh/master d85832d More pom fixes The background on the originssh remote branch is that his origin uses an HTTP url which wasn't allowing him to push a large commit. I helped him to create originssh to allow the push to be made. master Description: Binary data originssh_master Description: Binary data
[PATCH 2/6] t5538: fix default http port
Originally I had t5537 use port 5536 and 5538 use port 5537(!). Then
Jeff found my fault so I changed port in t5537 from 5536 to 5537 in
3b32a7c (t5537: fix incorrect expectation in test case 10 -
2014-01-08). Which makes it worse because now both t5537 and t5538
both use the same port. Fix it.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
t/t5538-push-shallow.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index 0a6e40f..19fe425 100755
--- a/t/t5538-push-shallow.sh
+++ b/t/t5538-push-shallow.sh
@@ -126,7 +126,7 @@ if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'}
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5538'}
. $TEST_DIRECTORY/lib-httpd.sh
start_httpd
--
1.8.5.2.240.g8478abd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] Fix the shallow deepen bug with no-done
Reported by Jeff [1]. Junio spotted it right but nobody made any move
since then. The main fix is 6/6. The rest is just updates while I was
looking at it. 2/6 may need fast track though.
[1] http://thread.gmane.org/gmane.comp.version-control.git/219914
Nguyễn Thái Ngọc Duy (6):
test: rename http fetch and push test files
t5538: fix default http port
pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt
protocol-capabilities.txt: document no-done
fetch-pack: fix deepen shallow over smart http with no-done cap
Documentation/technical/pack-protocol.txt | 3 ++-
Documentation/technical/protocol-capabilities.txt | 18
fetch-pack.c | 3 ++-
t/t5537-fetch-shallow.sh | 24 ++
t/t5538-push-shallow.sh| 2 +-
...5540-http-push.sh = t5540-http-push-webdav.sh} | 0
t/{t5541-http-push.sh = t5541-http-push-smart.sh} | 0
...5550-http-fetch.sh = t5550-http-fetch-dumb.sh} | 0
...551-http-fetch.sh = t5551-http-fetch-smart.sh} | 0
9 files changed, 47 insertions(+), 3 deletions(-)
rename t/{t5540-http-push.sh = t5540-http-push-webdav.sh} (100%)
rename t/{t5541-http-push.sh = t5541-http-push-smart.sh} (100%)
rename t/{t5550-http-fetch.sh = t5550-http-fetch-dumb.sh} (100%)
rename t/{t5551-http-fetch.sh = t5551-http-fetch-smart.sh} (100%)
--
1.8.5.2.240.g8478abd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] test: rename http fetch and push test files
Make clear which one is for dumb protocol, which one is for smart from
their file name.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
t/{t5540-http-push.sh = t5540-http-push-webdav.sh} | 0
t/{t5541-http-push.sh = t5541-http-push-smart.sh} | 0
t/{t5550-http-fetch.sh = t5550-http-fetch-dumb.sh} | 0
t/{t5551-http-fetch.sh = t5551-http-fetch-smart.sh} | 0
4 files changed, 0 insertions(+), 0 deletions(-)
rename t/{t5540-http-push.sh = t5540-http-push-webdav.sh} (100%)
rename t/{t5541-http-push.sh = t5541-http-push-smart.sh} (100%)
rename t/{t5550-http-fetch.sh = t5550-http-fetch-dumb.sh} (100%)
rename t/{t5551-http-fetch.sh = t5551-http-fetch-smart.sh} (100%)
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push-webdav.sh
similarity index 100%
rename from t/t5540-http-push.sh
rename to t/t5540-http-push-webdav.sh
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push-smart.sh
similarity index 100%
rename from t/t5541-http-push.sh
rename to t/t5541-http-push-smart.sh
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch-dumb.sh
similarity index 100%
rename from t/t5550-http-fetch.sh
rename to t/t5550-http-fetch-dumb.sh
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch-smart.sh
similarity index 100%
rename from t/t5551-http-fetch.sh
rename to t/t5551-http-fetch-smart.sh
--
1.8.5.2.240.g8478abd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
It's introduced in 1bd8c8f (git-upload-pack: Support the multi_ack protocol - 2005-10-28) but probably better documented in the commit message of 78affc4 (Add multi_ack_detailed capability to fetch-pack/upload-pack - 2009-10-30) Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index c73b62f..eb0b8a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,7 +338,8 @@ during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. Once the 'done' line is read from the client, the server will either -send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends +send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the +last SHA-1 determined to be common. The server only sends ACK after 'done' if there is at least one common base and multi_ack or multi_ack_detailed is enabled. The server always sends NAK after 'done' if there is no common base found. -- 1.8.5.2.240.g8478abd -- 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 4/6] protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt
pack-protocol.txt explains in detail how multi_ack_detailed works and what's the difference between no multi_ack, multi_ack and multi_ack_detailed. No need to repeat here. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/protocol-capabilities.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index e3e7924..cb40ebb 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -69,6 +69,12 @@ ends. Without multi_ack the client would have sent that c-b-a chain anyway, interleaved with S-R-Q. +multi_ack_detailed +-- +This is an extension of multi_ack that permits client to better +understand the server's in-memory state. See pack-protocol.txt, +section Packfile Negotiation for more information. + thin-pack - -- 1.8.5.2.240.g8478abd -- 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 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
In smart http, upload-pack adds new shallow lines at the beginning of
each rpc response. Only shallow lines from the first rpc call are
useful. After that they are thrown away. It's designed this way
because upload-pack is stateless and has no idea when its shallow
lines are helpful or not.
So after refs are negotiated with multi_ack_detailed and both sides
happy. The server sends ACK obj-id ready, terminates the rpc call
and waits for the final rpc round. The client sends done. The server
sends another response, which also has shallow lines at the beginning,
and the last ACK obj-id line.
When no-done is active, the last round is cut out, the server sends
ACK obj-id ready and ACK obj-id in the same rpc
response. fetch-pack is updated to recognize this and not send
done. However it still tries to consume shallow lines, which are
never sent.
Update the code, make sure to skip consuming shallow lines when
no-done is enabled.
Reported-by: Jeff King p...@peff.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
fetch-pack.c | 3 ++-
t/t5537-fetch-shallow.sh | 24
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49..f061f1f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ done:
}
strbuf_release(req_buf);
- consume_shallow_list(args, fd[0]);
+ if (!got_ready || !no_done)
+ consume_shallow_list(args, fd[0]);
while (flushes || multi_ack) {
int ack = get_ack(fd[0], result_sha1);
if (ack) {
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..fb11073 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -200,5 +200,29 @@ EOF
)
'
+# This test is tricky. We need large enough haves that fetch-pack
+# will put pkt-flush in between. Then we need a have the the server
+# does not have, it'll send ACK %s ready
+test_expect_success 'add more commits' '
+ (
+ cd shallow
+ for i in $(seq 10); do
+ git checkout --orphan unrelated$i
+ test_commit unrelated$i /dev/null
+ git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git
refs/heads/unrelated$i:refs/heads/unrelated$i
+ git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
+ done
+ git checkout master
+ test_commit new
+ git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
+ )
+ (
+ cd clone
+ git checkout --orphan newnew
+ test_commit new-too
+ git fetch --depth=2
+ )
+'
+
stop_httpd
test_done
--
1.8.5.2.240.g8478abd
--
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 5/6] protocol-capabilities.txt: document no-done
See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/protocol-capabilities.txt | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb40ebb..cb2f5eb 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -75,6 +75,18 @@ This is an extension of multi_ack that permits client to better understand the server's in-memory state. See pack-protocol.txt, section Packfile Negotiation for more information. +no-done +--- +This capability should be only used with smart HTTP protocol. If +multi_ack_detailed and no-done are both present, then the sender is +free to immediately send a pack following its first ACK obj-id ready +message. + +Without no-done in smart HTTP protocol, the server session would end +and the client has to make another trip to send done and the server +can send the pack. no-done removes the last round and thus slightly +reduces latency. + thin-pack - -- 1.8.5.2.240.g8478abd -- 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: Confusing git log --- First time bug submission please advise on best practices
On Thu, Feb 6, 2014 at 3:02 PM, Francis Stephens francissteph...@gmail.com wrote: My co-worker has an inconsistent git log output. Please see that attached files for output (I've made a best effort to remove confidential info from them). Comparing the two log commands we can see that master and originssh/master have a shared common commit at John Doe (4 hours ago) d85832d More pom fixes The top commit for originssh/master and the second to top for master. I would expect that both logs would share an _identical_ history from that commit onward. But the log for master contains the following Jeremy Doe (27 hours ago) 239ea21 (my-work) renamed class Jeremy Doe (28 hours ago) 27750b2 Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework and Jeremy Doe (2 days ago) a933acb white space changes Jeremy Doe (2 days ago) b5e51e7 Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework Jeremy Doe (2 days ago) 3a0f787 removed public methods Jeremy Doe (2 days ago) 4e91130 added the xml deserialisation None of which appear in the originssh/master log. Is there a scenario in which this is expected. It was my understanding that any two commits with the same hash have exactly the same history. Thanks for your time. The commits that are in the log for master and which are not in the log for originssh/master are merged in at 6833fd4 (HEAD, master); Completed merge. As git log can only present the commits in a linear way, it shows the commits from the ancentry of both parents of HEAD in a reverse chronological order. This means that the commits from the two ancestries are mixed and commits that are shown after each other don't have to be parent and child. See the documentation of git log and the section Commit Ordering: By default, the commits are shown in reverse chronological order. Vincent -- 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: Confusing git log --- First time bug submission please advise on best practices
Vincent van Ravesteijn v...@lyx.org writes: The commits that are in the log for master and which are not in the log for originssh/master are merged in at 6833fd4 (HEAD, master); Completed merge. As git log can only present the commits in a linear way, it shows the commits from the ancentry of both parents of HEAD in a reverse chronological order. This means that the commits from the two ancestries are mixed and commits that are shown after each other don't have to be parent and child. See the documentation of git log and the section Commit Ordering: By default, the commits are shown in reverse chronological order. git log --graph can help with getting a better picture. -- David Kastrup -- 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 7/8] combine-diff: Fast changed-to-all-parents paths scanning
On Wed, Feb 05, 2014 at 02:58:36PM -0800, Junio C Hamano wrote:
Kirill Smelkov k...@navytux.spb.ru writes:
On Wed, Feb 05, 2014 at 11:42:41AM -0800, Junio C Hamano wrote:
Kirill Smelkov k...@navytux.spb.ru writes:
I agree object data should be immutable for good. The only thing I'm
talking
about here is mode, which is parsed from a tree buffer and is stored in
separate field:
Ah, I do not see any problem in that case, then.
Thanks.
Thanks, that simplifies things for me.
Below is a patch which does it. Please apply, if it is ok.
Surely.
Be careful when traversing N-trees in parallel---you may have to
watch out for the entry ordering rules that sorts the following
paths in the order shown:
a
a-b
a/b
a_b
Inside a single tree, you cannot have 'a' and 'a/b' at the same
time, but one tree may have 'a' (without 'a/b') while another one
may have 'a/b' (without 'a'), and walking them in parallel has
historically been one source of funny bugs.
Thanks for the warning. I'm relying here on base_name_compare() and
ordering it imposes on entries and how it differentiates directories and
files, so let's hope this should be ok.
Actual reworking is still in flux though...
Thanks,
Kirill
8
From: Kirill Smelkov k...@mns.spb.ru
Date: Thu, 6 Feb 2014 15:36:31 +0400
Subject: [PATCH] Finally switch over tree descriptors to contain a pre-parsed
entry
This continues 4651ece8 (Switch over tree descriptors to contain a
pre-parsed entry) and moves the only rest computational part
mode = canon_mode(mode)
from tree_entry_extract() to tree entry decode phase - to
decode_tree_entry().
The reason to do it, is that canon_mode() is at least 2 conditional
jumps for regular files, and that could be noticeable should canon_mode()
be invoked several times.
That does not matter for current Git codebase, where typical tree
traversal is
while (t-size) {
sha1 = tree_entry_extract(t, path, mode);
...
update_tree_entry(t);
}
i.e. we do t - sha1,path.mode extraction only once per entry. In such
cases, it does not matter performance-wise, where that mode
canonicalization is done - either once in tree_entry_extract(), or once
in decode_tree_entry() called by update_tree_entry() - it is
approximately the same.
But for future code, which could need to work with several tree_desc's
in parallel, it could be handy to operate on tree_desc descriptors, and
do extracts only when needed, or at all, access only relevant part of
it through structure fields directly.
And for such situations, having canon_mode() be done once in decode
phase is better - we won't need to pay the performance price of 2 extra
conditional jumps on every t-mode access.
So let's move mode canonicalization to decode_tree_entry(). That was the
final bit. Now after tree entry is decoded, it is fully ready and could
be accessed either directly via field, or through tree_entry_extract()
which this time got really totally trivial.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
tree-walk.c | 2 +-
tree-walk.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 79dba1d..4dc86c7 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -37,7 +37,7 @@ static void decode_tree_entry(struct tree_desc *desc, const
char *buf, unsigned
/* Initialize the descriptor entry */
desc-entry.path = path;
- desc-entry.mode = mode;
+ desc-entry.mode = canon_mode(mode);
desc-entry.sha1 = (const unsigned char *)(path + len);
}
diff --git a/tree-walk.h b/tree-walk.h
index ae04b64..ae7fb3a 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -16,7 +16,7 @@ struct tree_desc {
static inline const unsigned char *tree_entry_extract(struct tree_desc *desc,
const char **pathp, unsigned int *modep)
{
*pathp = desc-entry.path;
- *modep = canon_mode(desc-entry.mode);
+ *modep = desc-entry.mode;
return desc-entry.sha1;
}
--
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
bash autocompletion for sontrib/subtree?
I've built/installed git from src,
git --version
git version 1.9.rc2.15.g89ba81d
installed/sourced
/etc/bash_completion.d/git-completion.bash
For all git-core builtins, autocompletion works.
I've also installed contrib/subtree
`git subtree` works, but I get no autocompletion for the command.
I've tried adding/sourcing this script (@
https://github.com/camptocamp/puppet-git-subtree/blob/master/files/bash_completion.sh),
-
_git_subtree ()
{
local cur=${COMP_WORDS[COMP_CWORD]}
if [ $COMP_CWORD -eq 2 ]; then
__gitcomp add merge pull push split
return
elif [ $COMP_CWORD -eq 3 ]; then
__gitcomp --prefix=
return
fi
__gitcomp $(__git_remotes)
}
-
but still get not subtree autocomplete.
What's neede to enable subtree's autocomplete?
--
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: attr.c doesn't honor --work-tree option
Lasse Makholm lasse.makh...@gmail.com writes:
Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():
$ cd /tmp/
$ mkdir -p attr-test/repo
$ cd attr-test/repo
$ git init
Initialized empty Git repository in /tmp/attr-test/repo/.git/
$ echo 'dir/* filter=foo' .gitattributes
$
Inside the working tree, it works:
$ ~/src/git.git/git check-attr -a dir/file
Does check-ignore misbehave the same way?
I suspect that is this because check-attr is not a command that
requires a working tree. The command was written primarily as a
debugging aid that can be used anywhere as long as you have a
repository to read strings from either its standard input or its
arguments, and gives them directly to check_attr(), but it does so
without first going to the top of the real working tree like
check-ignore does.
Forcing it to go to the top of the working tree (see the attached
one-liner, but note that I didn't test it) may give you want you
want.
git.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git.c b/git.c
index 7cf2953..314ec9f 100644
--- a/git.c
+++ b/git.c
@@ -342,7 +342,7 @@ static struct cmd_struct commands[] = {
{ branch, cmd_branch, RUN_SETUP },
{ bundle, cmd_bundle, RUN_SETUP_GENTLY },
{ cat-file, cmd_cat_file, RUN_SETUP },
- { check-attr, cmd_check_attr, RUN_SETUP },
+ { check-attr, cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
{ check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
{ check-mailmap, cmd_check_mailmap, RUN_SETUP },
{ check-ref-format, cmd_check_ref_format },
--
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
For your Interest
-- 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-remote-bzr: fatal: mark :399654 not declared
Hello, I'm using git-remote-bzr to access the GNU Emacs Bazaar repo. I followed the guideline described here: https://lists.gnu.org/archive/html/emacs-devel/2013-05/msg8.html Pulling and pushing worked flawless for several month. But recently git bzr stopped working with the following error message: $ git pull fatal: mark :399654 not declared fast-import: dumping crash report to .git/fast_import_crash_16173 fatal: Error while running fast-import Traceback (most recent call last): File /usr/bin/git-remote-bzr, line 947, in module sys.exit(main(sys.argv)) File /usr/bin/git-remote-bzr, line 931, in main do_import(parser) File /usr/bin/git-remote-bzr, line 399, in do_import export_branch(repo, name) File /usr/bin/git-remote-bzr, line 327, in export_branch modified_final = export_files(cur_tree, modified) File /usr/bin/git-remote-bzr, line 272, in export_files print d IOError: [Errno 32] Broken pipe And the crash report $ cat .git/fast_import_crash_16173 fast-import crash report: fast-import process: 16173 parent process : 16172 at Thu Feb 6 18:38:00 2014 fatal: mark :399654 not declared Most Recent Commands Before Crash - feature done feature import-marks=.git/bzr/origin/marks-git feature export-marks=.git/bzr/origin/marks-git feature force blob mark :399656 data 145827 blob mark :399657 data 19785 blob mark :399658 data 228415 commit refs/bzr/origin/heads/trunk mark :399659 author Glenn Morris r...@gnu.org 1391150391 -0800 committer Glenn Morris r...@gnu.org 1391150391 -0800 data 101 * from :399654 Active Branch LRU - active_branches = 0 cur, 5 max pos clock name ~ Inactive Branches - refs/bzr/origin/heads/trunk: status : dirty tip commit : old tree: cur tree: commit clock: 0 last pack : Marks - exported to .git/bzr/origin/marks-git --- END OF CRASH REPORT $ tail .git/bzr/origin/marks-git :399643 b535a54e6bd1b98953aacda8e0c0bd4ffa4348e8 :399646 2c7d42a26b8984b4492b205f28b0de2ff8548a16 :399647 408cc787f4764481e8ca10861895970e579dec07 :399648 798ea0615d18b41dfe0dee321b286fd3570a4352 :399651 dfba6da980cfe657cb1be302131325011f5eb97d :399652 5964046e0475262b27157e39ceb7516b1222330c :399653 52dd3850ce599215898f5358997dd07f08c4d607 :399656 230b94e2fe9c6443b2ada399d8d985c7ae438727 :399657 8d0fc723f14c9b4a9de30754170b5ca5be21a05a :399658 102818c18b5a0724dc1c11dc5dcf5e2ba5b72b22 I'm using git version 1.8.5.3 on Kubuntu 13.10 installed from https://launchpad.net/~git-core/+archive/ppa. What's wrong and how can I fix this? Regards, Rüdiger -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t0003: do not chdir the whole test process
Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. Signed-off-by: Junio C Hamano gits...@pobox.com --- * This is purely a preparatory clean-up in the test script I'll be adding a new test to in the next patch. t/t0003-attributes.sh | 52 +-- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index febc45c..0554b13 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -197,39 +197,47 @@ test_expect_success 'root subdir attribute test' ' ' test_expect_success 'setup bare' ' - git clone --bare . bare.git - cd bare.git + git clone --bare . bare.git ' test_expect_success 'bare repository: check that .gitattribute is ignored' ' ( - echo f test=f - echo a/i test=a/i - ) .gitattributes - attr_check f unspecified - attr_check a/f unspecified - attr_check a/c/f unspecified - attr_check a/i unspecified - attr_check subdir/a/i unspecified + cd bare.git + ( + echo f test=f + echo a/i test=a/i + ) .gitattributes + attr_check f unspecified + attr_check a/f unspecified + attr_check a/c/f unspecified + attr_check a/i unspecified + attr_check subdir/a/i unspecified + ) ' test_expect_success 'bare repository: check that --cached honors index' ' - GIT_INDEX_FILE=../.git/index \ - git check-attr --cached --stdin --all ../stdin-all | - sort actual - test_cmp ../specified-all actual + ( + cd bare.git + GIT_INDEX_FILE=../.git/index \ + git check-attr --cached --stdin --all ../stdin-all | + sort actual + test_cmp ../specified-all actual + ) ' test_expect_success 'bare repository: test info/attributes' ' ( - echo f test=f - echo a/i test=a/i - ) info/attributes - attr_check f f - attr_check a/f f - attr_check a/c/f f - attr_check a/i a/i - attr_check subdir/a/i unspecified + cd bare.git + ( + echo f test=f + echo a/i test=a/i + ) info/attributes + attr_check f f + attr_check a/f f + attr_check a/c/f f + attr_check a/i a/i + attr_check subdir/a/i unspecified + ) ' test_done -- 1.9-rc2-233-ged4ee9f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository
Lasse Makholm noticed that running git check-attr from a place totally unrelated to $GIT_DIR and $GIT_WORK_TREE does not give expected results. I think it is because the command does not say it wants to call setup_work_tree(). We still need to support use cases where only a bare repository is involved, so unconditionally requiring a working tree would not work well. Instead, make a call only in a non-bare repository. We may want to see if we want to do a similar fix in the opposite direction to check-ignore. The command unconditionally requires a working tree, but it should be usable in a bare repository just like check-attr attempts to be. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/check-attr.c | 3 +++ t/t0003-attributes.sh | 10 ++ 2 files changed, 13 insertions(+) diff --git a/builtin/check-attr.c b/builtin/check-attr.c index 075d01d..f29d6c3 100644 --- a/builtin/check-attr.c +++ b/builtin/check-attr.c @@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) struct git_attr_check *check; int cnt, i, doubledash, filei; + if (!is_bare_repository()) + setup_work_tree(); + git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, check_attr_options, diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 0554b13..6e6aef5 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -196,6 +196,16 @@ test_expect_success 'root subdir attribute test' ' attr_check subdir/a/i unspecified ' +test_expect_success 'using --git-dir and --work-tree' ' + mkdir unreal real + git init real + echo file test=in-real real/.gitattributes + ( + cd unreal + attr_check file in-real --git-dir ../real/.git --work-tree ../real + ) +' + test_expect_success 'setup bare' ' git clone --bare . bare.git ' -- 1.9-rc2-233-ged4ee9f -- 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/6] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: It's introduced in 1bd8c8f (git-upload-pack: Support the multi_ack protocol - 2005-10-28) but probably better documented in the commit message of 78affc4 (Add multi_ack_detailed capability to fetch-pack/upload-pack - 2009-10-30) Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index c73b62f..eb0b8a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,7 +338,8 @@ during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. Once the 'done' line is read from the client, the server will either -send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends +send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the +last SHA-1 determined to be common. The server only sends I'd suggest rewording it to: 'obj-is' is the object name of the last commit determined to be common I do not mind too much if it used colloquial SHA-1 in place of object name, but what is common is not the name but the object the name refers to, so the last commit part is a moderately strong suggestion. 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] protocol-capabilities.txt: document no-done
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/protocol-capabilities.txt | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb40ebb..cb2f5eb 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -75,6 +75,18 @@ This is an extension of multi_ack that permits client to better understand the server's in-memory state. See pack-protocol.txt, section Packfile Negotiation for more information. +no-done +--- +This capability should be only used with smart HTTP protocol. If +multi_ack_detailed and no-done are both present, then the sender is +free to immediately send a pack following its first ACK obj-id ready +message. + +Without no-done in smart HTTP protocol, the server session would end +and the client has to make another trip to send done and the server +can send the pack. no-done removes the last round and thus slightly +reduces latency. + thin-pack - 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 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
In smart http, upload-pack adds new shallow lines at the beginning of
each rpc response. Only shallow lines from the first rpc call are
useful. After that they are thrown away. It's designed this way
because upload-pack is stateless and has no idea when its shallow
lines are helpful or not.
So after refs are negotiated with multi_ack_detailed and both sides
happy. The server sends ACK obj-id ready, terminates the rpc call
Is the above ... and both sides are happy, the server sends ...?
Lack of a verb makes it hard to guess.
and waits for the final rpc round. The client sends done. The server
sends another response, which also has shallow lines at the beginning,
and the last ACK obj-id line.
When no-done is active, the last round is cut out, the server sends
ACK obj-id ready and ACK obj-id in the same rpc
response. fetch-pack is updated to recognize this and not send
done. However it still tries to consume shallow lines, which are
never sent.
Update the code, make sure to skip consuming shallow lines when
no-done is enabled.
Reported-by: Jeff King p...@peff.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Thanks.
Do I understand the situation correctly if I said The call to
consume-shallow-list has been here from the very beginning, but it
should have been adjusted like this patch when no-done was
introduced.?
fetch-pack.c | 3 ++-
t/t5537-fetch-shallow.sh | 24
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49..f061f1f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ done:
}
strbuf_release(req_buf);
- consume_shallow_list(args, fd[0]);
+ if (!got_ready || !no_done)
+ consume_shallow_list(args, fd[0]);
while (flushes || multi_ack) {
int ack = get_ack(fd[0], result_sha1);
if (ack) {
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..fb11073 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -200,5 +200,29 @@ EOF
)
'
+# This test is tricky. We need large enough haves that fetch-pack
+# will put pkt-flush in between. Then we need a have the the server
+# does not have, it'll send ACK %s ready
+test_expect_success 'add more commits' '
+ (
+ cd shallow
+ for i in $(seq 10); do
+ git checkout --orphan unrelated$i
+ test_commit unrelated$i /dev/null
+ git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git
refs/heads/unrelated$i:refs/heads/unrelated$i
+ git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
+ done
+ git checkout master
+ test_commit new
+ git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
+ )
+ (
+ cd clone
+ git checkout --orphan newnew
+ test_commit new-too
+ git fetch --depth=2
+ )
+'
+
stop_httpd
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 0/6] Fix the shallow deepen bug with no-done
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Reported by Jeff [1]. Junio spotted it right but nobody made any move since then. Hrm. Was I supposed to make any move at that point? The discussion ended by me asking a question what happens if we did X? and there was no discussion. The main fix is 6/6. The rest is just updates while I was looking at it. 2/6 may need fast track though. -- 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] test: rename http fetch and push test files
On Thu, Feb 06, 2014 at 10:10:34PM +0700, Nguyễn Thái Ngọc Duy wrote: Make clear which one is for dumb protocol, which one is for smart from their file name. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Yay. This has often bugged me, and I can't believe we went this long without fixing 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 2/6] t5538: fix default http port
On Thu, Feb 06, 2014 at 10:10:35PM +0700, Nguyễn Thái Ngọc Duy wrote: Originally I had t5537 use port 5536 and 5538 use port 5537(!). Then Jeff found my fault so I changed port in t5537 from 5536 to 5537 in 3b32a7c (t5537: fix incorrect expectation in test case 10 - 2014-01-08). Which makes it worse because now both t5537 and t5538 both use the same port. Fix it. Yikes. I'm surprised we didn't notice this before, as it would probably barf intermittently when running the tests in parallel. Perhaps it was the cause of some intermittent failures that have gone undiagnosed. Patch looks obviously correct. -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] fetch-pack: fix deepen shallow over smart http with no-done cap
On Thu, Feb 6, 2014 at 10:10 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index b0fa738..fb11073 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -200,5 +200,29 @@ EOF ) ' +# This test is tricky. We need large enough haves that fetch-pack +# will put pkt-flush in between. Then we need a have the the server s/the the/that the/ +# does not have, it'll send ACK %s ready +test_expect_success 'add more commits' ' + ( + cd shallow + for i in $(seq 10); do Do you want to use test_seq here? + git checkout --orphan unrelated$i + test_commit unrelated$i /dev/null + git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git refs/heads/unrelated$i:refs/heads/unrelated$i + git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i + done + git checkout master + test_commit new + git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master + ) + ( + cd clone + git checkout --orphan newnew + test_commit new-too + git fetch --depth=2 + ) +' + stop_httpd test_done -- 1.8.5.2.240.g8478abd -- 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] protocol-capabilities.txt: document no-done
On Thu, Feb 06, 2014 at 10:10:38PM +0700, Nguyễn Thái Ngọc Duy wrote: See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Content looks good. A few minor grammar nits: +no-done +--- +This capability should be only used with smart HTTP protocol. If split infinitive: s/be only/only be/ I think it would be more customary to say the smart HTTP protocol, with the definite article. +Without no-done in smart HTTP protocol, the server session would end Ditto here. +and the client has to make another trip to send done and the server +can send the pack. no-done removes the last round and thus slightly s/and the server/before the server/ ? -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] fetch-pack: fix deepen shallow over smart http with no-done cap
On Thu, Feb 06, 2014 at 10:10:39PM +0700, Nguyễn Thái Ngọc Duy wrote: In smart http, upload-pack adds new shallow lines at the beginning of each rpc response. Only shallow lines from the first rpc call are useful. After that they are thrown away. It's designed this way because upload-pack is stateless and has no idea when its shallow lines are helpful or not. So after refs are negotiated with multi_ack_detailed and both sides happy. The server sends ACK obj-id ready, terminates the rpc call and waits for the final rpc round. The client sends done. The server sends another response, which also has shallow lines at the beginning, and the last ACK obj-id line. When no-done is active, the last round is cut out, the server sends ACK obj-id ready and ACK obj-id in the same rpc response. fetch-pack is updated to recognize this and not send done. However it still tries to consume shallow lines, which are never sent. Update the code, make sure to skip consuming shallow lines when no-done is enabled. Thanks for a nice explanation. +# This test is tricky. We need large enough haves that fetch-pack +# will put pkt-flush in between. Then we need a have the the server s/the the/the/ +# does not have, it'll send ACK %s ready +test_expect_success 'add more commits' ' + ( + cd shallow + for i in $(seq 10); do This probably needs to be test_seq for portability. -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 0/6] Fix the shallow deepen bug with no-done
On Thu, Feb 06, 2014 at 11:31:02AM -0800, Junio C Hamano wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Reported by Jeff [1]. Junio spotted it right but nobody made any move since then. Hrm. Was I supposed to make any move at that point? The discussion ended by me asking a question what happens if we did X? and there was no discussion. No, you were rightly waiting on me. I queued it on my todo list but haven't gotten around to it (and hadn't received any reports since the original, so didn't consider it high priority). I picked it off my todo list and stuck it on the bug list under insanely hard for fun (because I think the diagnosis probably would have been beyond a first-time git contributor). Thanks very much for looking at (and fixing!) this, Duy. -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/2] t0003: do not chdir the whole test process
Junio C Hamano wrote: Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. I agree with the above, and I like the patch... The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. ... but this logic seems wrong. I don't think we've ever supported setup tests failing or being skipped in the past. Thanks, 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 2/2] check-attr: move to the top of working tree when in non-bare repository
Hi,
Junio C Hamano wrote:
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char
*prefix)
struct git_attr_check *check;
int cnt, i, doubledash, filei;
+ if (!is_bare_repository())
+ setup_work_tree();
Hm. Shouldn't check-attr error out when run without a worktree and
without --cached?
That would mean something like
diff --git i/builtin/check-attr.c w/builtin/check-attr.c
index e9af7b2..c34b6ee 100644
--- i/builtin/check-attr.c
+++ w/builtin/check-attr.c
@@ -107,6 +107,9 @@ int cmd_check_attr(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, check_attr_options,
check_attr_usage, PARSE_OPT_KEEP_DASHDASH);
+ if (!cached_attrs)
+ setup_work_tree();
+
if (read_cache() 0) {
die(invalid cache);
}
--
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
Rebase fail with new file
GitDev - During a rebase of a feature branch to the master branch, I lost a new file in a new directory. I was able to recover by replaying the rebase and taking exceptional actions. However, I'm left with the impression the git does not handle new files properly under some rebase situations and gives the wrong advice to users. I was able to recover by reseting the branch to the pre-rebase commit, repeating the rebase, and manually applying the .git/rebase-apply/patch at the critical point. I do not understand why the patch did not work through the rebase command. Additionally, the git recommendation that after the failed patch, with no local changes, a [git rebase --skip] is a likely resolution is just wrong. Skipping after a complete patch failure lead me to very broken working directory. Details follow, and the contents of the problematic commit are attached (from [git show 2e9b460]). Thanks Lee This occurs with git version 1.8.3.2 [1024] $ git --version git version 1.8.3.2 *** Details *** Initial setup: After working in branch feature/build-projects for several days (with ~21 commits+pushes from branch point), I needed to integrate new changes from the master branch. Here are the steps from my bash history: 879 git checkout master 880 git pull 881 git checkout feature/build-projects 882 git rebase master There were a couple of conflicts, which I resolved and moved on with [git rebase --continue]. At approximately the 14th commit, the rebase stopped with this message: bash Applying: STRY0057770: Add -Z parent for late build modules. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... error: Your local changes to the following files would be overwritten by merge: glide-app-itbm/pom.xml Please, commit your changes or stash them before you can merge. Aborting Failed to merge in the changes. Patch failed at 0014 STRY0057770: Add -Z parent for late build modules. The copy of the patch that failed is found in: /Users/lee.carver/Service-now/dev/glide04/git/glide/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. /bash Git status indicated no change, and inspection of the glide-app-itbm/pom.xml showed no conflicts or unexpected content: bash [888] $ git status # HEAD detached from ee6d51c # You are currently rebasing branch 'feature/build-projects' on 'ee6d51c'. # (all conflicts fixed: run git rebase --continue) # When I tried to [git rebase --continue], the error message provided a little more information. Specifically, it mentions the added (new) file glide-module-parent-z/pom.xml. [904] $ git rebase --continue Applying: STRY0057770: Add -Z parent for late build modules. Applying: STRY0057770: Provide version properties for diagrammer and email-notification. Using index info to reconstruct a base tree... M glide-app-itbm/pom.xml A glide-module-parent-z/pom.xml Falling back to patching base and 3-way merge... Auto-merging glide-module-parent/pom.xml Auto-merging glide-app-itbm/pom.xml CONFLICT (content): Merge conflict in glide-app-itbm/pom.xml Failed to merge in the changes. Patch failed at 0015 STRY0057770: Provide version properties for diagrammer and email-notification. The copy of the patch that failed is found in: /Users/lee.carver/Service-now/dev/glide04/git/glide/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. /bash I made a minor edit to the glide-app-itbm/pom.xml (adding a -z to the parent name), [git add -u], and then [git rebase --continue]. There were a couple of addition conflicts, some resolved with --continue, some with --skip. In the end, I was left with a working direction that did not contain the new file glide-module-parent-z/pom.xml or other changes from the problematic commit/rebase step (STRY0057770: Add -Z parent for late build modules.). *** Recovery *** I was able to recover the branch, and complete the rebase with the expected results only by manually applying the patch file at the critical rebase step: Starting in the feature branch, these actions were performed: 928 git reset --hard 6793d9f 930 git checkout master 931 git pull 932 git checkout feature/build-projects 934 git rebase master When the rebase of (STRY0057770: Add -Z parent for late build modules.) stopped, I manually applied the patch 959 git apply .git/rebase-apply/patch With the appropriate adds, etc. I was able to [git rebase --continue] to a successful conclusion. 2e9b460.out Description: Binary data
Re: [PATCH v4 08/17] trailer: add interpret-trailers command
From: Eric Sunshine sunsh...@sunshineco.com On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder chrisc...@tuxfamily.org wrote: diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..9db4459 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *infile, int trim_empty, int argc, const char **argv); + +#endif /* TRAILER_H */ One might reasonably expect trailer.h and the process_trailers() declaration to be introduced by patch 7/17 (trailer: put all the processing together and print) in which process_trailers() is defined in trailer.c. On the other hand, I think that it is not so nice to add a header file like trailer.h unless it is included at least once. Maybe I can squash this patch with the previous one, but then the series might be a little bit more difficult to understand. 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: [PATCH v4 10/17] trailer: if no input file is passed, read from stdin
From: Eric Sunshine sunsh...@sunshineco.com
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
chrisc...@tuxfamily.org wrote:
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
diff --git a/trailer.c b/trailer.c
index 8681aed..73a65e0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char
*infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'),
infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
strbuf_fread(), perhaps?
I chose strbuf_read() because it can be passed 0 as a size hint, while
strbuf_fread() must be passed an exact size.
(As we might read from stdin, we might not be able to know the exact
size before we start reading.)
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: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository
Hi again, Jonathan Nieder wrote: Junio C Hamano wrote: +if (!is_bare_repository()) +setup_work_tree(); Hm. Shouldn't check-attr error out when run without a worktree and without --cached? That would mean something like diff --git i/builtin/check-attr.c w/builtin/check-attr.c index e9af7b2..c34b6ee 100644 --- i/builtin/check-attr.c +++ w/builtin/check-attr.c @@ -107,6 +107,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, check_attr_options, check_attr_usage, PARSE_OPT_KEEP_DASHDASH); + if (!cached_attrs) + setup_work_tree(); Someone asked in a private reply how this interacts with t0003. t0003 tries check-attr in a bare repository. The question is, is that a desirable feature, and are people relying on it? If people are relying on it, perhaps the intuitive behavior would be to make check-attr use an only-look-at-HEAD mode by default when running in a bare repo. How do I use the only-look-at-HEAD mode from a non-bare repo? If I want attributes with respect to some other commit instead of HEAD, is there a syntax for that? The command doesn't seem to have been well thought out. 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
[PATCH v5 06/14] trailer: put all the processing together and print
This patch adds the process_trailers() function that
calls all the previously added processing functions
and then prints the results on the standard output.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 40
1 file changed, 40 insertions(+)
diff --git a/trailer.c b/trailer.c
index f59efd1..186316f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -56,6 +56,26 @@ static inline int contains_only_spaces(const char *str)
return !*s;
}
+static void print_tok_val(const char *tok, const char *val)
+{
+ char c = tok[strlen(tok) - 1];
+ if (isalnum(c))
+ printf(%s: %s\n, tok, val);
+ else if (isspace(c) || c == '#')
+ printf(%s%s\n, tok, val);
+ else
+ printf(%s %s\n, tok, val);
+}
+
+static void print_all(struct trailer_item *first, int trim_empty)
+{
+ struct trailer_item *item;
+ for (item = first; item; item = item-next) {
+ if (!trim_empty || strlen(item-value) 0)
+ print_tok_val(item-token, item-value);
+ }
+}
+
static void add_arg_to_infile(struct trailer_item *infile_tok,
struct trailer_item *arg_tok)
{
@@ -522,3 +542,23 @@ static void process_input_file(const char *infile,
strbuf_list_free(lines);
}
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char
**argv)
+{
+ struct trailer_item *infile_tok_first = NULL;
+ struct trailer_item *infile_tok_last = NULL;
+ struct trailer_item *arg_tok_first;
+
+ git_config(git_trailer_config, NULL);
+
+ /* Print the non trailer part of infile */
+ if (infile) {
+ process_input_file(infile, infile_tok_first, infile_tok_last);
+ }
+
+ arg_tok_first = process_command_line_args(argc, argv);
+
+ process_trailers_lists(infile_tok_first, infile_tok_last,
arg_tok_first);
+
+ print_all(infile_tok_first, trim_empty);
+}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 08/14] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 208 ++ 1 file changed, 208 insertions(+) create mode 100755 t/t7513-interpret-trailers.sh diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh new file mode 100755 index 000..8be333c --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,208 @@ +#!/bin/sh +# +# Copyright (c) 2013 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +cat basic_message 'EOF' +subject + +body +EOF + +cat complex_message_body 'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# We want one trailing space at the end of each line. +# Let's use sed to make sure that these spaces are not removed +# by any automatic tool. +sed -e 's/ Z$/ /' complex_message_trailers -\EOF +Fixes: Z +Acked-by: Z +Reviewed-by: Z +Signed-off-by: Z +EOF + +test_expect_success 'without config' ' + printf ack: Peff\nReviewed-by: \nAcked-by: Johan\n expected + git interpret-trailers ack = Peff Reviewed-by Acked-by: Johan actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + printf ack: Peff\nAcked-by: Johan\n expected + git interpret-trailers --trim-empty ack = Peff Reviewed-by Acked-by: Johan sob: actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + printf Acked-by: Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by :Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and = sign' ' + git config trailer.ack.key Acked-by= + printf Acked-by= Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by= Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by : Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and # sign' ' + git config trailer.bug.key Bug # + printf Bug #42\n expected + git interpret-trailers --trim-empty bug = 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit basic message' ' + git interpret-trailers --infile basic_message actual + test_cmp basic_message actual +' + +test_expect_success 'with commit complex message' ' + cat complex_message_body complex_message_trailers complex_message + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message and args' ' + cat complex_message_body expected + printf Fixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \nBug #42\n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message, args and --trim-empty' ' + cat complex_message_body expected + printf Acked-by= Peff\nBug #42\n expected + git interpret-trailers --trim-empty --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before' ' + git config trailer.bug.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before for a token in the middle of infile' ' + git config trailer.review.key Reviewed-by: + git config trailer.review.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: Johan\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 review: Johan actual + test_cmp expected actual +' + +test_expect_success 'using where = before and --trim-empty' ' + cat complex_message_body expected + printf Bug #46\nBug #42\nAcked-by= Peff\nReviewed-by: Johan\n expected + git interpret-trailers --infile complex_message --trim-empty ack: Peff bug: 42 review: Johan Bug: 46 actual + test_cmp expected actual +' + +test_expect_success 'the default is ifExist = addIfDifferent' ' + cat complex_message_body expected +
[PATCH v5 00/14] Add interpret-trailers builtin
This patch series implements a new command:
git interpret-trailers
and an infrastructure to process trailers that can be reused,
for example in commit.c.
1) Rationale:
This command should help with RFC 822 style headers, called
trailers, that are found at the end of commit messages.
(Note that these headers do not follow and are not intended to
follow many rules that are in RFC 822. For example they do not
follow the line breaking rules, the encoding rules and probably
many other rules.)
For a long time, these trailers have become a de facto standard
way to add helpful information into commit messages.
Until now git commit has only supported the well known
Signed-off-by: trailer, that is used by many projects like
the Linux kernel and Git.
It is better to implement features for these trailers first in a
new command rather than in builtin/commit.c, because this way the
prepare-commit-msg and commit-msg hooks can reuse this command.
2) Current state:
Currently the usage string of this command is:
git interpret-trailers [--trim-empty] [--infile=file]
[(token[(=|:)value])...]
The following features are implemented:
- the result is printed on stdout
- the [token[=value]] arguments are interpreted
- a commit message passed using the --infile=file option is
interpreted
- if --infile is not used, a commit message is read from stdin
- the trailer.token.key options in the config are interpreted
- the trailer.token.where options are interpreted
- the trailer.token.ifExist options are interpreted
- the trailer.token.ifMissing options are interpreted
- the trailer.token.command config works
- $ARG can be used in commands
- ditto for GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} env variables
- there are some tests
- there is some documentation
The following features are planned but not yet implemented:
- add more tests related to commands
- add examples in documentation
- integration with git commit
Possible improvements:
- support GIT_COMMIT_PROTO env variable in commands
3) Changes since version 4, thanks to Eric:
* many small functions became 'static inline' instead of just 'static'
* alnum_len() has an int len parameter instead of size_t len
* some redundant comments were removed
* some const have been added
* some switch replaced some if ... else if ...
* some string related functions have been removed from strbuf.{c,h}
* some memory leaks have been fixed (but see below)
* the refactoring patch 11/17 was squashed into a previous patch
* the documentation patch was improved
4) TODO from previous review:
- check for memory leaks
- add test for empty token
Christian Couder (14):
Add data structures and basic functions for commit trailers
trailer: process trailers from file and arguments
trailer: read and process config information
trailer: process command line trailer arguments
trailer: parse trailers from input file
trailer: put all the processing together and print
trailer: add interpret-trailers command
trailer: add tests for git interpret-trailers
trailer: if no input file is passed, read from stdin
trailer: execute command from 'trailer.name.command'
trailer: add tests for trailer command
trailer: set author and committer env variables
trailer: add tests for commands using env variables
Documentation: add documentation for 'git interpret-trailers'
.gitignore | 1 +
Documentation/git-interpret-trailers.txt | 132 +++
Makefile | 2 +
builtin.h| 1 +
builtin/interpret-trailers.c | 36 ++
git.c| 1 +
t/t7513-interpret-trailers.sh| 262
trailer.c| 658 +++
trailer.h| 6 +
9 files changed, 1099 insertions(+)
create mode 100644 Documentation/git-interpret-trailers.txt
create mode 100644 builtin/interpret-trailers.c
create mode 100755 t/t7513-interpret-trailers.sh
create mode 100644 trailer.c
create mode 100644 trailer.h
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 03/14] trailer: read and process config information
This patch implements reading the configuration
to get trailer information, and then processing
it and storing it in a doubly linked list.
The config information is stored in the list
whose first item is pointed to by:
static struct trailer_item *first_conf_item;
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 134 ++
1 file changed, 134 insertions(+)
diff --git a/trailer.c b/trailer.c
index ba0cfe0..f844f46 100644
--- a/trailer.c
+++ b/trailer.c
@@ -25,6 +25,8 @@ struct trailer_item {
struct conf_info *conf;
};
+static struct trailer_item *first_conf_item;
+
static inline int same_token(struct trailer_item *a, struct trailer_item *b,
int alnum_len)
{
return !strncasecmp(a-token, b-token, alnum_len);
@@ -233,3 +235,135 @@ static void process_trailers_lists(struct trailer_item
**infile_tok_first,
apply_arg_if_missing(infile_tok_first, infile_tok_last,
arg_tok);
}
}
+
+static int set_where(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(after, value))
+ item-where = WHERE_AFTER;
+ else if (!strcasecmp(before, value))
+ item-where = WHERE_BEFORE;
+ else
+ return 1;
+ return 0;
+}
+
+static int set_if_exist(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(addIfDifferent, value))
+ item-if_exist = EXIST_ADD_IF_DIFFERENT;
+ else if (!strcasecmp(addIfDifferentNeighbor, value))
+ item-if_exist = EXIST_ADD_IF_DIFFERENT_NEIGHBOR;
+ else if (!strcasecmp(add, value))
+ item-if_exist = EXIST_ADD;
+ else if (!strcasecmp(overwrite, value))
+ item-if_exist = EXIST_OVERWRITE;
+ else if (!strcasecmp(doNothing, value))
+ item-if_exist = EXIST_DO_NOTHING;
+ else
+ return 1;
+ return 0;
+}
+
+static int set_if_missing(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(doNothing, value))
+ item-if_missing = MISSING_DO_NOTHING;
+ else if (!strcasecmp(add, value))
+ item-if_missing = MISSING_ADD;
+ else
+ return 1;
+ return 0;
+}
+
+enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
+TRAILER_IF_EXIST, TRAILER_IF_MISSING };
+
+static int set_name_and_type(const char *conf_key, const char *suffix,
+enum trailer_info_type type,
+char **pname, enum trailer_info_type *ptype)
+{
+ int ret = ends_with(conf_key, suffix);
+ if (ret) {
+ *pname = xstrndup(conf_key, strlen(conf_key) - strlen(suffix));
+ *ptype = type;
+ }
+ return ret;
+}
+
+static struct trailer_item *get_conf_item(const char *name)
+{
+ struct trailer_item *item;
+ struct trailer_item *previous;
+
+ /* Look up item with same name */
+ for (previous = NULL, item = first_conf_item;
+item;
+previous = item, item = item-next) {
+ if (!strcasecmp(item-conf-name, name))
+ return item;
+ }
+
+ /* Item does not already exists, create it */
+ item = xcalloc(sizeof(struct trailer_item), 1);
+ item-conf = xcalloc(sizeof(struct conf_info), 1);
+ item-conf-name = xstrdup(name);
+
+ if (!previous)
+ first_conf_item = item;
+ else {
+ previous-next = item;
+ item-previous = previous;
+ }
+
+ return item;
+}
+
+static int git_trailer_config(const char *conf_key, const char *value, void
*cb)
+{
+ if (starts_with(conf_key, trailer.)) {
+ const char *orig_conf_key = conf_key;
+ struct trailer_item *item;
+ struct conf_info *conf;
+ char *name;
+ enum trailer_info_type type;
+
+ conf_key += 8;
+ if (!set_name_and_type(conf_key, .key, TRAILER_KEY, name,
type)
+ !set_name_and_type(conf_key, .command, TRAILER_COMMAND,
name, type)
+ !set_name_and_type(conf_key, .where, TRAILER_WHERE,
name, type)
+ !set_name_and_type(conf_key, .ifexist, TRAILER_IF_EXIST,
name, type)
+ !set_name_and_type(conf_key, .ifmissing,
TRAILER_IF_MISSING, name, type))
+ return 0;
+
+ item = get_conf_item(name);
+ conf = item-conf;
+
+ switch (type) {
+ case TRAILER_KEY:
+ if (conf-key)
+ warning(_(more than one %s), orig_conf_key);
+ conf-key = xstrdup(value);
+ break;
+ case TRAILER_COMMAND:
+ if (conf-command)
+ warning(_(more than one %s),
[PATCH v5 11/14] trailer: add tests for trailer command
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index f5ef81f..2d50b7a 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -212,4 +212,31 @@ test_expect_success 'with input from stdin' ' test_cmp expected actual ' +test_expect_success 'with simple command' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExist addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'setup a commit' ' + echo Content of the first commit. a.txt + git add a.txt + git commit -m Add file a.txt +' + +test_expect_success 'with command using $ARG' ' + git config trailer.fix.ifExist overwrite + git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG + FIXED=$(git log -1 --oneline --format=%h (%s) --abbrev-commit --abbrev=14 HEAD) + cat complex_message_body expected + printf Fixes: $FIXED\nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=HEAD complex_message actual + test_cmp expected actual +' + test_done -- 1.8.5.2.206.g98f5689.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 14/14] Documentation: add documentation for 'git interpret-trailers'
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-interpret-trailers.txt | 132 +++ 1 file changed, 132 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..0617941 --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,132 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [--infile=file] [(token[(=|:)value])...] + +DESCRIPTION +--- +Help add RFC 822-like headers, called 'trailers', at the end of the +otherwise free-form part of a commit message. + +Unless `--infile=file` is used, this command is a filter. It reads +the standard input for a commit message and applies the `token` +arguments, if any, to this message. The resulting message is emited on +the standard output. + +Some configuration variables control the way the `token` arguments are +applied to the message and the way any existing trailer in the message +is changed. They also make it possible to automatically add some +trailers. + +By default, a 'token=value' or 'token:value' argument will be added +only if no trailer with the same (token, value) pair is already in the +message. The 'token' and 'value' parts will be trimmed to remove +starting and trailing whitespace, and the resulting trimmed 'token' +and 'value' will appear in the message like this: + + +token: value + + +By default, if there are already trailers with the same 'token' the +new trailer will appear just after the last trailer with the same +'token'. Otherwise it will appear at the end of the message. + +Note that 'trailers' do not follow and are not intended to follow many +rules that are in RFC 822. For example they do not follow the line +breaking rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the 'value' part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + +infile=file:: + Read the commit message from `file` instead of the standard + input. + +CONFIGURATION VARIABLES +--- + +trailer.token.key:: + This 'key' will be used instead of 'token' in the + trailer. After some alphanumeric characters, it can contain + some non alphanumeric characters like ':', '=' or '#' that will + be used instead of ':' to separate the token from the value in + the trailer, though the default ':' is more standard. + +trailer.token.where:: + This can be either `after`, which is the default, or + `before`. If it is `before`, then a trailer with the specified + token, will appear before, instead of after, other trailers + with the same token, or otherwise at the beginning, instead of + at the end, of all the trailers. + +trailer.token.ifexist:: + This option makes it possible to choose what action will be + performed when there is already at least one trailer with the + same token in the message. ++ +The valid values for this option are: `addIfDifferent` (this is the +default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`. ++ +With `addIfDifferent`, a new trailer will be added only if no trailer +with the same (token, value) pair is already in the message. ++ +With `addIfDifferentNeighbor`, a new trailer will be added only if no +trailer with the same (token, value) pair is above or below the line +where the new trailer will be added. ++ +With `add`, a new trailer will be added, even if some trailers with +the same (token, value) pair are already in the message. ++ +With `overwrite`, the new trailer will overwrite an existing trailer +with the same token. ++ +With `doNothing`, nothing will be done, that is no new trailer will be +added if there is already one with the same token in the message. + +trailer.token.ifmissing:: + This option makes it possible to choose what action will be + performed when there is not yet any trailer with the same + token in the message. ++ +The valid values for this option are: `add` (this is the default) and +`doNothing`. ++ +With `add`, a new trailer will be added. ++ +With `doNothing`, nothing will be done. + +trailer.token.command:: + This option can be used to specify a shell command that will + be used to automatically add or modify a trailer with the + specified 'token'. ++ +When this option is specified, it is like if a special 'token=value' +argument is added at the end of the command line, where 'value' will +be given by the standard output of the specified command. ++ +If the command
[PATCH v5 07/14] trailer: add interpret-trailers command
This patch adds the git interpret-trailers command.
This command uses the previously added process_trailers()
function in trailer.c.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
.gitignore | 1 +
Makefile | 1 +
builtin.h| 1 +
builtin/interpret-trailers.c | 36
git.c| 1 +
trailer.h| 6 ++
6 files changed, 46 insertions(+)
create mode 100644 builtin/interpret-trailers.c
create mode 100644 trailer.h
diff --git a/.gitignore b/.gitignore
index b5f9def..c870ada 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/git-index-pack
/git-init
/git-init-db
+/git-interpret-trailers
/git-instaweb
/git-log
/git-ls-files
diff --git a/Makefile b/Makefile
index ec90feb..a91465e 100644
--- a/Makefile
+++ b/Makefile
@@ -935,6 +935,7 @@ BUILTIN_OBJS += builtin/hash-object.o
BUILTIN_OBJS += builtin/help.o
BUILTIN_OBJS += builtin/index-pack.o
BUILTIN_OBJS += builtin/init-db.o
+BUILTIN_OBJS += builtin/interpret-trailers.o
BUILTIN_OBJS += builtin/log.o
BUILTIN_OBJS += builtin/ls-files.o
BUILTIN_OBJS += builtin/ls-remote.o
diff --git a/builtin.h b/builtin.h
index d4afbfe..30f4c30 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,7 @@ extern int cmd_hash_object(int argc, const char **argv, const
char *prefix);
extern int cmd_help(int argc, const char **argv, const char *prefix);
extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
extern int cmd_init_db(int argc, const char **argv, const char *prefix);
+extern int cmd_interpret_trailers(int argc, const char **argv, const char
*prefix);
extern int cmd_log(int argc, const char **argv, const char *prefix);
extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
new file mode 100644
index 000..04b0ae2
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,36 @@
+/*
+ * Builtin git interpret-trailers
+ *
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ *
+ */
+
+#include cache.h
+#include builtin.h
+#include parse-options.h
+#include strbuf.h
+#include trailer.h
+
+static const char * const git_interpret_trailers_usage[] = {
+ N_(git interpret-trailers [--trim-empty] [--infile=file]
[(token[(=|:)value])...]),
+ NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+ const char *infile = NULL;
+ int trim_empty = 0;
+
+ struct option options[] = {
+ OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_interpret_trailers_usage, 0);
+
+ process_trailers(infile, trim_empty, argc, argv);
+
+ return 0;
+}
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
diff --git a/trailer.h b/trailer.h
new file mode 100644
index 000..9db4459
--- /dev/null
+++ b/trailer.h
@@ -0,0 +1,6 @@
+#ifndef TRAILER_H
+#define TRAILER_H
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char
**argv);
+
+#endif /* TRAILER_H */
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 13/14] trailer: add tests for commands using env variables
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 2d50b7a..00894a8 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -223,6 +223,26 @@ test_expect_success 'with simple command' ' test_cmp expected actual ' +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExist addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: C O Mitter commit...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using author information' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExist addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + test_expect_success 'setup a commit' ' echo Content of the first commit. a.txt git add a.txt -- 1.8.5.2.206.g98f5689.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 12/14] trailer: set author and committer env variables
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 98187fc..b5de616 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
+
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -448,7 +475,7 @@ static const char *apply_command(const char *command, const
char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -459,6 +486,7 @@ static const char *apply_command(const char *command, const
char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 01/14] Add data structures and basic functions for commit trailers
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Makefile | 1 +
trailer.c | 48
2 files changed, 49 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..f129b5a
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,48 @@
+#include cache.h
+/*
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exist { EXIST_ADD_IF_DIFFERENT, EXIST_ADD_IF_DIFFERENT_NEIGHBOR,
+ EXIST_ADD, EXIST_OVERWRITE, EXIST_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
+
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
+};
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info *conf;
+};
+
+static inline int same_token(struct trailer_item *a, struct trailer_item *b,
int alnum_len)
+{
+ return !strncasecmp(a-token, b-token, alnum_len);
+}
+
+static inline int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a-value, b-value);
+}
+
+static inline int same_trailer(struct trailer_item *a, struct trailer_item *b,
int alnum_len)
+{
+ return same_token(a, b, alnum_len) same_value(a, b);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static inline size_t alnum_len(const char *buf, int len)
+{
+ while (--len = 0 !isalnum(buf[len]));
+ return (size_t) len + 1;
+}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 09/14] trailer: if no input file is passed, read from stdin
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const
char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index 186316f..108e104 100644
--- a/trailer.c
+++ b/trailer.c
@@ -483,8 +483,13 @@ static struct strbuf **read_input_file(const char *infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -551,10 +556,8 @@ void process_trailers(const char *infile, int trim_empty,
int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 02/14] trailer: process trailers from file and arguments
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
applied, it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 187 ++
1 file changed, 187 insertions(+)
diff --git a/trailer.c b/trailer.c
index f129b5a..ba0cfe0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,3 +46,190 @@ static inline size_t alnum_len(const char *buf, int len)
while (--len = 0 !isalnum(buf[len]));
return (size_t) len + 1;
}
+
+static void add_arg_to_infile(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf-where == WHERE_AFTER) {
+ arg_tok-next = infile_tok-next;
+ infile_tok-next = arg_tok;
+ arg_tok-previous = infile_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = infile_tok-previous;
+ infile_tok-previous = arg_tok;
+ arg_tok-next = infile_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf-where;
+ do {
+ if (!infile_tok)
+ return 1;
+ if (same_trailer(infile_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+* if we want to add a trailer after another one,
+* we have to check those before this one
+*/
+ infile_tok = (where == WHERE_AFTER) ? infile_tok-previous :
infile_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exist(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len)
+{
+ switch (arg_tok-conf-if_exist) {
+ case EXIST_DO_NOTHING:
+ free(arg_tok);
+ break;
+ case EXIST_OVERWRITE:
+ free((char *)infile_tok-value);
+ infile_tok-value = xstrdup(arg_tok-value);
+ free(arg_tok);
+ break;
+ case EXIST_ADD:
+ add_arg_to_infile(infile_tok, arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
+
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_infile_tok(struct trailer_item *infile_tok,
+ struct trailer_item **arg_tok_first,
+ enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int tok_alnum_len = alnum_len(infile_tok-token,
strlen(infile_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(infile_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf-where == where) {
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
+ /*
+* If arg has been added to infile,
+* then we need to process it too now.
+*/
+ if ((where == WHERE_AFTER ? infile_tok-next :
infile_tok-previous) == arg_tok)
+
[PATCH v5 05/14] trailer: parse trailers from input file
This patch reads trailers from an input file, parses
them and puts the result into a doubly linked list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 71 +++
1 file changed, 71 insertions(+)
diff --git a/trailer.c b/trailer.c
index 3fde21d..f59efd1 100644
--- a/trailer.c
+++ b/trailer.c
@@ -49,6 +49,13 @@ static inline size_t alnum_len(const char *buf, int len)
return (size_t) len + 1;
}
+static inline int contains_only_spaces(const char *str)
+{
+ const char *s;
+ for (s = str; *s isspace(*s); s++);
+ return !*s;
+}
+
static void add_arg_to_infile(struct trailer_item *infile_tok,
struct trailer_item *arg_tok)
{
@@ -451,3 +458,67 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
return arg_tok_first;
}
+
+static struct strbuf **read_input_file(const char *infile)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+
+ return strbuf_split(sb, '\n');
+}
+
+/*
+ * Return the the (0 based) index of the first trailer line
+ * or the line count if there are no trailers.
+ */
+static int find_trailer_start(struct strbuf **lines)
+{
+ int count, start, empty = 1;
+
+ /* Get the line count */
+ for (count = 0; lines[count]; count++);
+
+ /*
+* Get the start of the trailers by looking starting from the end
+* for a line with only spaces before lines with one ':'.
+*/
+ for (start = count - 1; start = 0; start--) {
+ if (contains_only_spaces(lines[start]-buf)) {
+ if (empty)
+ continue;
+ return start + 1;
+ }
+ if (strchr(lines[start]-buf, ':')) {
+ if (empty)
+ empty = 0;
+ continue;
+ }
+ return count;
+ }
+
+ return empty ? count : start + 1;
+}
+
+static void process_input_file(const char *infile,
+ struct trailer_item **infile_tok_first,
+ struct trailer_item **infile_tok_last)
+{
+ struct strbuf **lines = read_input_file(infile);
+ int start = find_trailer_start(lines);
+ int i;
+
+ /* Print non trailer lines as is */
+ for (i = 0; lines[i] i start; i++) {
+ printf(%s, lines[i]-buf);
+ }
+
+ /* Parse trailer lines */
+ for (i = start; lines[i]; i++) {
+ struct trailer_item *new = create_trailer_item(lines[i]-buf);
+ add_trailer_item(infile_tok_first, infile_tok_last, new);
+ }
+
+ strbuf_list_free(lines);
+}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 10/14] trailer: execute command from 'trailer.name.command'
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 63 +++
1 file changed, 63 insertions(+)
diff --git a/trailer.c b/trailer.c
index 108e104..98187fc 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,4 +1,5 @@
#include cache.h
+#include run-command.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -12,11 +13,14 @@ struct conf_info {
char *name;
char *key;
char *command;
+ unsigned command_uses_arg : 1;
enum action_where where;
enum action_if_exist if_exist;
enum action_if_missing if_missing;
};
+#define TRAILER_ARG_STRING $ARG
+
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -56,6 +60,13 @@ static inline int contains_only_spaces(const char *str)
return !*s;
}
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const char
*b)
+{
+ const char *ptr = strstr(sb-buf, a);
+ if (ptr)
+ strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b));
+}
+
static void print_tok_val(const char *tok, const char *val)
{
char c = tok[strlen(tok) - 1];
@@ -375,6 +386,7 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
if (conf-command)
warning(_(more than one %s), orig_conf_key);
conf-command = xstrdup(value);
+ conf-command_uses_arg = !!strstr(conf-command,
TRAILER_ARG_STRING);
break;
case TRAILER_WHERE:
if (set_where(conf, value))
@@ -411,6 +423,45 @@ static void parse_trailer(struct strbuf *tok, struct
strbuf *val, const char *tr
}
}
+static int read_from_command(struct child_process *cp, struct strbuf *buf)
+{
+ if (run_command(cp))
+ return error(running trailer command '%s' failed,
cp-argv[0]);
+ if (strbuf_read(buf, cp-out, 1024) 1)
+ return error(reading from trailer command '%s' failed,
cp-argv[0]);
+ strbuf_trim(buf);
+ return 0;
+}
+
+static const char *apply_command(const char *command, const char *arg)
+{
+ struct strbuf cmd = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *argv[] = {NULL, NULL};
+ const char *result = ;
+
+ strbuf_addstr(cmd, command);
+ if (arg)
+ strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
+
+ argv[0] = cmd.buf;
+ memset(cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.no_stdin = 1;
+ cp.out = -1;
+ cp.use_shell = 1;
+
+ if (read_from_command(cp, buf))
+ strbuf_release(buf);
+ else
+ result = strbuf_detach(buf, NULL);
+
+ strbuf_release(cmd);
+ return result;
+}
+
static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
const char* tok, const char* val)
{
@@ -420,6 +471,8 @@ static struct trailer_item *new_trailer_item(struct
trailer_item *conf_item,
if (conf_item) {
new-conf = conf_item-conf;
new-token = xstrdup(conf_item-conf-key);
+ if (conf_item-conf-command_uses_arg || !val)
+ new-value = apply_command(conf_item-conf-command,
val);
} else {
new-conf = xcalloc(sizeof(struct conf_info), 1);
new-token = tok;
@@ -470,12 +523,22 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
int i;
struct trailer_item *arg_tok_first = NULL;
struct trailer_item *arg_tok_last = NULL;
+ struct trailer_item *item;
for (i = 0; i argc; i++) {
struct trailer_item *new = create_trailer_item(argv[i]);
add_trailer_item(arg_tok_first, arg_tok_last, new);
}
+ /* Add conf commands that don't use $ARG */
+ for (item = first_conf_item; item; item = item-next) {
+ if (item-conf-command !item-conf-command_uses_arg)
+ {
+ struct trailer_item *new = new_trailer_item(item, NULL,
NULL);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+ }
+
return arg_tok_first;
}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 04/14] trailer: process command line trailer arguments
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 84 +++
1 file changed, 84 insertions(+)
diff --git a/trailer.c b/trailer.c
index f844f46..3fde21d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -367,3 +367,87 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
}
return 0;
}
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ const char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
+ if (end) {
+ strbuf_add(tok, trailer, end - trailer);
+ strbuf_trim(tok);
+ strbuf_addstr(val, end + 1);
+ strbuf_trim(val);
+ } else {
+ strbuf_addstr(tok, trailer);
+ strbuf_trim(tok);
+ }
+}
+
+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
+const char* tok, const char* val)
+{
+ struct trailer_item *new = xcalloc(sizeof(*new), 1);
+ new-value = val;
+
+ if (conf_item) {
+ new-conf = conf_item-conf;
+ new-token = xstrdup(conf_item-conf-key);
+ } else {
+ new-conf = xcalloc(sizeof(struct conf_info), 1);
+ new-token = tok;
+ }
+
+ return new;
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ struct trailer_item *item;
+ int tok_alnum_len;
+
+ parse_trailer(tok, val, string);
+
+ tok_alnum_len = alnum_len(tok.buf, tok.len);
+
+ /* Lookup if the token matches something in the config */
+ for (item = first_conf_item; item; item = item-next) {
+ if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
+ !strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
+ strbuf_release(tok);
+ return new_trailer_item(item, NULL, strbuf_detach(val,
NULL));
+ }
+ }
+
+ return new_trailer_item(NULL, strbuf_detach(tok, NULL),
strbuf_detach(val, NULL));;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+struct trailer_item **last,
+struct trailer_item *new)
+{
+ if (!*last) {
+ *first = new;
+ *last = new;
+ } else {
+ (*last)-next = new;
+ new-previous = *last;
+ *last = new;
+ }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char
**argv)
+{
+ int i;
+ struct trailer_item *arg_tok_first = NULL;
+ struct trailer_item *arg_tok_last = NULL;
+
+ for (i = 0; i argc; i++) {
+ struct trailer_item *new = create_trailer_item(argv[i]);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+
+ return arg_tok_first;
+}
--
1.8.5.2.206.g98f5689.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t0003: do not chdir the whole test process
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. ... but this logic seems wrong. I don't think we've ever supported setup tests failing or being skipped in the past. The first set-up test, yes, but something in the middle added as an afterthought? Even set-up in the middle added as an afterthought, yes. For a while I've been wanting to teach GIT_SKIP_TESTS not to skip tests with 'setup' or 'set up' in their name, but I never got around to it. If I try to skip the setup test this patch touches, then there is no bare.git and lots of later tests fail. Perhaps it would be better for each test to do rm -fr bare.git git clone --bare . bare.git ( cd bare.git ... ) for itself to make the state easier to think about. On the other hand I agree that the 'cd' here is a bad practice. I just don't think it's about skipping setup --- instead, it's about it being hard to remember the cwd in general. Thanks, 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 1/2] t0003: do not chdir the whole test process
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. I agree with the above, and I like the patch... The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. ... but this logic seems wrong. I don't think we've ever supported setup tests failing or being skipped in the past. The first set-up test, yes, but something in the middle added as an afterthought? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository
Jonathan Nieder jrnie...@gmail.com writes: Someone asked in a private reply how this interacts with t0003. It was me mistakenly using reply not reply all. t0003 tries check-attr in a bare repository. The question is, is that a desirable feature, and are people relying on it? Running tar-tree from a public distribution point comes to mind. bootstrap-attr-stack seems to have reference to is-bare-repository to validate the attribute direction to read from the index, but I tend to think what it really wants is to read from HEAD not the index. How do I use the only-look-at-HEAD mode from a non-bare repo? Is You don't a good answer? Use of --cached when your index matches HEAD is the equivalent, and if the index differs from HEAD, you must have had a reason to add that change to .gitattributes to the index, so I think it is reasonable to honour that change. -- 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/4] line-log: convert to using diff_tree_sha1()
Kirill Smelkov k...@mns.spb.ru writes: Since diff_tree_sha1() can now accept empty trees via NULL sha1, we could just call it without manually reading trees into tree_desc and duplicating code. Cc: Thomas Rast t...@thomasrast.ch Signed-off-by: Kirill Smelkov k...@mns.spb.ru --- line-log.c | 26 ++ 1 file changed, 2 insertions(+), 24 deletions(-) You have to love a diffstat like that :-) Thanks. -- Thomas Rast t...@thomasrast.ch -- 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/2] t0003: do not chdir the whole test process
Jonathan Nieder jrnie...@gmail.com writes: For a while I've been wanting to teach GIT_SKIP_TESTS not to skip tests with 'setup' or 'set up' in their name, but I never got around to it. Yeah, that would be a good thing. As part of doing so, we might want to come up with a way to test the tests, randomly skipping pieces that are not setup and find ones that break the later tests when skipped, and mark test scripts that fail such a test for fixing. If I try to skip the setup test this patch touches, then there is no bare.git and lots of later tests fail. Perhaps it would be better for each test to do rm -fr bare.git git clone --bare . bare.git ( cd bare.git ... ) for itself to make the state easier to think about. That is a better and worse way to do it at the same time ;-) It definitely is better from maintainability POV to keep each test as independent as possible. It however also is worse if it forces us to be repetitive X-. On the other hand I agree that the 'cd' here is a bad practice. I just don't think it's about skipping setup --- instead, it's about it being hard to remember the cwd in general. Exactly. -- 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/4] line-log: convert to using diff_tree_sha1()
Thomas Rast t...@thomasrast.ch writes: Kirill Smelkov k...@mns.spb.ru writes: Since diff_tree_sha1() can now accept empty trees via NULL sha1, we could just call it without manually reading trees into tree_desc and duplicating code. Cc: Thomas Rast t...@thomasrast.ch Signed-off-by: Kirill Smelkov k...@mns.spb.ru --- line-log.c | 26 ++ 1 file changed, 2 insertions(+), 24 deletions(-) You have to love a diffstat like that :-) Thanks. Yes, indeed. 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 v4 10/17] trailer: if no input file is passed, read from stdin
Christian Couder chrisc...@tuxfamily.org writes:
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
sdtout???
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv,
const char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index 8681aed..73a65e0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char
*infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -530,10 +535,8 @@ void process_trailers(const char *infile, int
trim_empty, int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/14] Add data structures and basic functions for commit trailers
Christian Couder chrisc...@tuxfamily.org writes:
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Makefile | 1 +
trailer.c | 48
2 files changed, 49 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..f129b5a
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,48 @@
+#include cache.h
+/*
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exist { EXIST_ADD_IF_DIFFERENT,
EXIST_ADD_IF_DIFFERENT_NEIGHBOR,
+EXIST_ADD, EXIST_OVERWRITE, EXIST_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
All these names and conf_info below are not named to be specific
to this little tool. Can I assume that these will never be exposed
to the rest of the system? If so, they are fine.
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
It still feels somewhat strange. It is true that an item can be
either exist or missing and it is understandable that it tempts
you to split that into two, but EXIST_OVERWRITE will not trigger
either WHERE_AFTER or WHERE_BEFORE action.
+};
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info *conf;
+};
+
+static inline int same_token(struct trailer_item *a, struct trailer_item *b,
int alnum_len)
+{
+ return !strncasecmp(a-token, b-token, alnum_len);
+}
+
+static inline int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a-value, b-value);
+}
+
+static inline int same_trailer(struct trailer_item *a, struct trailer_item
*b, int alnum_len)
+{
+ return same_token(a, b, alnum_len) same_value(a, b);
+}
All these inlines look premature optimization that can be
delegated to any decent compiler, don't they?
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static inline size_t alnum_len(const char *buf, int len)
+{
+ while (--len = 0 !isalnum(buf[len]));
Style:
while (--len = 0 !isalnum(buf[len]))
;
You may add a comment on the empty statement to make it stand out
even more, i.e.
; /* nothing */
+ return (size_t) len + 1;
This is somewhat unfortunate. if the caller wants to receive
size_t, perhaps it should be passing in size_t (or ssize_t) to the
function? Hard to guess without an actual caller, though.
+}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/14] Add data structures and basic functions for commit trailers
Christian Couder chrisc...@tuxfamily.org writes:
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
Probably if_exists is more gramatically correct.
if (x-if_exists) {
... do this ...
}
would read well, but not x-if_exist.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
applied, it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 187
++
1 file changed, 187 insertions(+)
diff --git a/trailer.c b/trailer.c
index f129b5a..ba0cfe0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,3 +46,190 @@ static inline size_t alnum_len(const char *buf, int len)
while (--len = 0 !isalnum(buf[len]));
return (size_t) len + 1;
}
+
+static void add_arg_to_infile(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf-where == WHERE_AFTER) {
+ arg_tok-next = infile_tok-next;
+ infile_tok-next = arg_tok;
+ arg_tok-previous = infile_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = infile_tok-previous;
+ infile_tok-previous = arg_tok;
+ arg_tok-next = infile_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf-where;
+ do {
+ if (!infile_tok)
+ return 1;
+ if (same_trailer(infile_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+ * if we want to add a trailer after another one,
+ * we have to check those before this one
+ */
+ infile_tok = (where == WHERE_AFTER) ? infile_tok-previous :
infile_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exist(struct trailer_item *infile_tok,
+struct trailer_item *arg_tok,
+int alnum_len)
+{
+ switch (arg_tok-conf-if_exist) {
+ case EXIST_DO_NOTHING:
+ free(arg_tok);
+ break;
+ case EXIST_OVERWRITE:
+ free((char *)infile_tok-value);
+ infile_tok-value = xstrdup(arg_tok-value);
+ free(arg_tok);
+ break;
+ case EXIST_ADD:
+ add_arg_to_infile(infile_tok, arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
Makes me wonder if people want a rule to say if the same key
already exists, regardless of the value.
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+ struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
Will callers free the item that now is not on the list?
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_infile_tok(struct trailer_item *infile_tok,
+struct trailer_item **arg_tok_first,
+enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int tok_alnum_len = alnum_len(infile_tok-token,
strlen(infile_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(infile_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf-where == where) {
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
+ /*
+ * If arg has been added to infile,
+ * then we need to process it
Re: [PATCH v5 04/14] trailer: process command line trailer arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
No the patch doesn't parse anything ;-).
Parse the command line arguments and
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ const char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
How would you explain the behaviour of the above code for this
input?
frotz: nitfol=xyzzy
Perhaps strcspn()?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 07/14] trailer: add interpret-trailers command
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
Does this even need to have a git repository? What is the RUN_SETUP
for?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 08/14] trailer: add tests for git interpret-trailers
Christian Couder chrisc...@tuxfamily.org writes: + +cat basic_message 'EOF' +subject + +body +EOF + +cat complex_message_body 'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# We want one trailing space at the end of each line. +# Let's use sed to make sure that these spaces are not removed +# by any automatic tool. +sed -e 's/ Z$/ /' complex_message_trailers -\EOF +Fixes: Z +Acked-by: Z +Reviewed-by: Z +Signed-off-by: Z +EOF Please put things like these inside the first setup test. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 09/14] trailer: if no input file is passed, read from stdin
Christian Couder chrisc...@tuxfamily.org writes:
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
sdtout?
Why isn't this a pure filter without any infile parameter in the
first place?
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv,
const char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index 186316f..108e104 100644
--- a/trailer.c
+++ b/trailer.c
@@ -483,8 +483,13 @@ static struct strbuf **read_input_file(const char
*infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -551,10 +556,8 @@ void process_trailers(const char *infile, int
trim_empty, int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 10/14] trailer: execute command from 'trailer.name.command'
Christian Couder chrisc...@tuxfamily.org writes:
+#define TRAILER_ARG_STRING $ARG
No need to support users who may want to use a string that happens
to match this substring literally as part of the command line?
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -56,6 +60,13 @@ static inline int contains_only_spaces(const char *str)
return !*s;
}
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const
char *b)
Same comment about inline for an earlier patch applies.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 12/14] trailer: set author and committer env variables
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 98187fc..b5de616 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
This is far too confusing. Name it read_fn() or something.
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -448,7 +475,7 @@ static const char *apply_command(const char *command,
const char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -459,6 +486,7 @@ static const char *apply_command(const char *command,
const char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 12/14] trailer: set author and committer env variables
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 98187fc..b5de616 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
Whoever is using this tool while replaying an existing commit likely
wants to export these two variables _into_ this command from the
outside, and this function will not interfere with it.
But for other uses, do we need to export these variables? After
all, this matters _only_ when the command we spawn wants to know
the idents to be used, but they can ask git-var themselves to cover
both cases, whether the environment that called this tool already
had the ident environment variables or it didn't.
So I am not sure what value this step is adding to the series.
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
+
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -448,7 +475,7 @@ static const char *apply_command(const char *command,
const char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -459,6 +486,7 @@ static const char *apply_command(const char *command,
const char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
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] Fix the shallow deepen bug with no-done
On Fri, Feb 7, 2014 at 2:31 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Reported by Jeff [1]. Junio spotted it right but nobody made any move since then. Hrm. Was I supposed to make any move at that point? The discussion ended by me asking a question what happens if we did X? and there was no discussion. Don't take it the wrong way. I was just summarizing the last round. It surprised me though that this went under my radar. Perhaps a bug tracker is not a bad idea after all (if Jeff went missing, this bug could fall under the crack) -- 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 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
On Fri, Feb 7, 2014 at 2:16 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: In smart http, upload-pack adds new shallow lines at the beginning of each rpc response. Only shallow lines from the first rpc call are useful. After that they are thrown away. It's designed this way because upload-pack is stateless and has no idea when its shallow lines are helpful or not. So after refs are negotiated with multi_ack_detailed and both sides happy. The server sends ACK obj-id ready, terminates the rpc call Is the above ... and both sides are happy, the server sends ...? Yes. Although think again, both sides is incorrect. If the server side is happy, then it'll send ACK.. ready to stop the client. The client can hardly protest. Do I understand the situation correctly if I said The call to consume-shallow-list has been here from the very beginning, but it should have been adjusted like this patch when no-done was introduced.? It's been there since the introduction of smart http (there are so many beginnings in git). The rest is right. -- 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
[PATCH] gitweb: Avoid overflowing page body frame with large images
When displaying a blob in gitweb, if it's an image, specify constraints for
maximum display width and height to prevent the image from overflowing the
frame of the enclosing page_body div.
This change assumes that it is more desirable to see the whole image without
scrolling (new behavior) than it is to see every pixel without zooming
(previous behavior).
Signed-off-by: Andrew B Keller and...@kellerfarm.com
---
I recently used Git to archive a set of scanned photos, and I used gitweb to
provide access to them. Overall, everything worked well, but I found it
undesirable that I had to zoom out in my browser on every photo to see the
whole photo. In the spirit of making the default behavior the most likely
correct behavior, this patch seems to be a good idea.
However, I'm not an expert on the use cases of gitweb. In order for the
maximum size constraints to take effect, the image would have to be at least
the size of the web browser window (minus a handful of pixels), so the affected
images are usually going to be pretty big. Are there any common use cases for
displaying a large image without scaling (and hence, with scrolling)?
Thanks,
Andrew
gitweb/gitweb.perl |2 +-
gitweb/static/gitweb.css |5 +
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3bc0f0b..2c6a77f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7094,7 +7094,7 @@ sub git_blob {
git_print_page_path($file_name, blob, $hash_base);
print div class=\page_body\\n;
if ($mimetype =~ m!^image/!) {
- print qq!img type=!.esc_attr($mimetype).qq!!;
+ print qq!img class=image_blob
type=!.esc_attr($mimetype).qq!!;
if ($file_name) {
print qq! alt=!.esc_attr($file_name).qq!
title=!.esc_attr($file_name).qq!!;
}
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 3b4d833..cd57c2f 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -32,6 +32,11 @@ img.avatar {
vertical-align: middle;
}
+img.image_blob {
+ max-height: 100%;
+ max-width: 100%;
+}
+
a.list img.avatar {
border-style: none;
}
--
1.7.7.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
Johannes Sixt j.sixt at viscovery.net writes: Am 2/6/2014 12:54, schrieb konstunn at ngs.ru: However I typed the checkout directory in file ..git/info/sparse-checkout by using different formats with and without the leading and the trailing slashes, with and without asterisk after trailing slash, having tried all the possible combinations, but, all the same, nevertheless, the error occured. Make sure that you do not use CRLF line terminators in the sparse-checkout file. This is it. Right you are. I've just tried to edit manually with notepad .git\info\sparse-checkout and found out that there really was a CRLF line terminator. After I removed it I managed to succeed in my sparse checkout. -- 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
