Re: Reset by checkout?
Atsushi Nakagawa at...@chejz.com writes: Ok, the typical use case is: I'm on 'master' and I make a few test commits. Afterwards, I want to discard the commits and move back to 'origin/master'. I could type 'reset --hard origin/master' and risk blowing away dirty files if I'm not careful. Or, I could use reset by checkout and be carefree. I think that is what 'reset --keep' is doing. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line 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: [ANNOUNCE] Git v2.0.0
Felipe Contreras felipe.contre...@gmail.com writes: Jeff King wrote: On Wed, May 28, 2014 at 06:17:25PM -0500, Felipe Contreras wrote: This is the last mail I sent to you, because you ignore them anyway, and remove them from the mailing list. [...] [2], a mail you conveniently removed from the tracked record. [...] You also conveniently removed this mail from the archives. I see you already noticed the changes in v2.0, but I wanted to address these points, because I consider silent censorship to be a serious accusation. Yes, I also think silent censorship is a very seriours matter, and I was very dissapointed that this mailing list would engage in that. I've reported the bug to gmane.discuss (no link yet, as I'm waiting for the message to go through, but it is not a high traffic group, so it should be easy to find the thread once it is there). Thanks. At first I thought that was the reason, but then I noticed it was always my mails that seemed to get this bug, so I decided it was too much of a coincidence. Some mailing list filters and/or spam filters flag mails with too many recipients so that they need to pass through moderation first. The typical threads on this list are short and have few recipients while longer threads, due to the list policy of adding every participants to the Cc, will tend to have more recipients. So there may a bias against long-running threads with multiple participants with regard to timely delivery. And frankly, if I were a list moderator and software asked me through this sort of coincidence whether a mail should be delivered or not and a glance at it shows nothing but insults, wild accusations, threats and so on for the umpteenth time, I'd consider twice clicking Accept. Whether or not I ultimately did so, this would likely contribute to the delay. -- 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 v2 2/2] connect.c: replace signal() with sigaction()
On 31/05/14 08:58, Jeremiah Mahler wrote:
From signal(2) man page:
The behavior of signal() varies across UNIX versions, and has also var‐
ied historically across different versions of Linux. Avoid its use:
use sigaction(2) instead.
Replaced signal() with sigaction() in connect.c
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
connect.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/connect.c b/connect.c
index a983d06..b2a33c9 100644
--- a/connect.c
+++ b/connect.c
@@ -665,11 +665,14 @@ struct child_process *git_connect(int fd[2], const char
*url,
enum protocol protocol;
const char **arg;
struct strbuf cmd = STRBUF_INIT;
+ struct sigaction sa;
/* Without this we cannot rely on waitpid() to tell
* what happened to our children.
*/
- signal(SIGCHLD, SIG_DFL);
+ memset(sa, 0, sizeof(sa));
+ sa.sa_handler = SIG_DFL;
+ sigaction(SIGCHLD, sa, 0);
I think this got lost in the wash with v1 but
Documentation/CodingGuidelines says to use NULL here instead of 0.
protocol = parse_connect_url(url, hostandport, path);
if (flags CONNECT_DIAG_URL) {
--
To unsubscribe from this list: send the line 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] check_refname_component: Optimize
On 05/30/2014 07:29 PM, Jeff King wrote: On Fri, May 30, 2014 at 11:47:33AM +0200, Michael Haggerty wrote: [...] If we want to be robust to future changes to refname rules, we could add a header flag like # pack-refs with: peeled fully-peeled check-level=1.0 [...] Yeah, I thought about mentioning something like that. But really, this just seems like a lot of complexity to solve the problem in a wrong way. Yes, you are right. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Reset by checkout?
Atsushi Nakagawa wrote: Ok, the typical use case is: I'm on 'master' and I make a few test commits. Afterwards, I want to discard the commits and move back to 'origin/master'. I could type 'reset --hard origin/master' and risk blowing away dirty files if I'm not careful. Or, I could use reset by checkout and be carefree. Doesn't 'git reset orign/master' do that? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] check_refname_component: Optimize
On Fri, May 30, 2014 at 7:07 AM, Jeff King p...@peff.net wrote: But then we're just trusting that the trust me flag on disk is correct. Why not just trust that packed-refs is correct in the first place? I missed one thing in the first reply: because packed-refs is a plain text file, the user could be tempted to edit it manually (I know I did a few times for fast rename) and so it could not be trusted. If we ignore this (and I think we can, it's not like we encourage people to edit files in $GIT_DIR), then #3 and #4 are as good as #2. IOW, consider this progression of changes: 1. Check refname format when we read packed-refs (the current behavior). 2. Keep a separate file packed-refs.stat with stat information. If the packed-refs file matches that stat information, do not bother checking refname formats. 3. Put a flag in packed-refs that says trust me, I'm valid. Check the refnames when it is generated. 4. Realize that we already check the refnames when we write it out. Don't bother writing trust me, I'm valid; readers can assume that it is. What is the scenario that option (2) protects against that options (3) and (4) do not? -- 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: Reset by checkout?
On 31/05/2014 08:46, Atsushi Nakagawa wrote: `git checkout -B current-branch-name tree-ish` This is such an useful notion that I can fathom why there isn't a better, first-tier, alternative.q I'm 100% in agreement. Reset current branch to X is an extremely common operation, and I use this all the time. But having to actually name the current branch is silly, and like you, I'm prone to swapping the parameters. I guess in theory using checkout allows fancier extra options like --merge and --patch, but I don't think I've ever used those with checkout, let alone this mode, where I really do just want a reset, with safety checks. The original git reset --hard used to be a pretty top-level command. It was used for aborting merges in particular. But I think it now stands out as being one of the only really dangerous porcelain commands, and I can't think of any real workflow it's still useful for. Maybe it could now be modified to warn and require -f to overwrite anything in the working tree? While digging into this, it seems git reset --keep is actually pretty close to git checkout -B current branch. It certainly won't lose your workspace file, but unlike checkout it /does /forget what you've staged, which could be annoying. Maybe that could be modified to keep the index too? (I like your alias.become - might try that). Kevin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] connect.c: replace signal() with sigaction()
Chris, On Sat, May 31, 2014 at 10:39:39PM +1200, Chris Packham wrote: On 31/05/14 08:58, Jeremiah Mahler wrote: From signal(2) man page: ... - signal(SIGCHLD, SIG_DFL); + memset(sa, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigaction(SIGCHLD, sa, 0); I think this got lost in the wash with v1 but Documentation/CodingGuidelines says to use NULL here instead of 0. Got it. Will be in to the next revision. sigaction(SIGCHLD, sa, NULL); Thanks, -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-multimail: migration: Config is not iterable
On 05/29/2014 04:22 PM, Azat Khuzhin wrote: Using the latest version of git-multimail there is an issue with migration: $ ~azat/git-multimail/git-multimail/migrate-mailhook-config --overwrite Traceback (most recent call last): File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 271, in module main(sys.argv[1:]) File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 268, in main migrate_config(strict=options.strict, retain=options.retain, overwrite=options.overwrite) File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 159, in migrate_config if not _check_old_config_exists(old): File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 66, in _check_old_config_exists if name in old: TypeError: argument of type 'Config' is not iterable Tested on 2.6 and 2.7 python versions. If you revert 09d0d5b92203f019763e43cef1e57f76f117d2b4 (Get Python files to pass pep8's tests.) there issue goes away. I understand that this is not the right solution and I'm not the guru of python, so just let you know. Thanks for the bug report and for narrowing it down to the broken commit. I just pushed a fix to GitHub. Let me know if it works for you now. Elijah: your fix is also correct, but I didn't see it before I pushed my own solution. Sorry about that. Junio: you don't have to worry about any of this, because the commit that caused the breakage is not in your tree yet. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line 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] Improve function dir.c:trim_trailing_spaces()
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
---
Correct 'if()' statements to conform to the general style
which implies using 'if(ptr)' as an equivalent of 'if(ptr != NULL)'
dir.c | 29 ++---
t/t0008-ignores.sh | 21 +
2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index eb6f581..98c81a8 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,20 @@ void clear_exclude_list(struct exclude_list *el)
static void trim_trailing_spaces(char *buf)
{
- int i, last_space = -1, nr_spaces, len = strlen(buf);
- for (i = 0; i len; i++)
- if (buf[i] == '\\')
- i++;
- else if (buf[i] == ' ') {
- if (last_space == -1) {
- last_space = i;
- nr_spaces = 1;
- } else
- nr_spaces++;
- } else
- last_space = -1;
-
- if (last_space != -1 last_space + nr_spaces == len)
- buf[last_space] = '\0';
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ if (*p == ' ') {
+ if (!last_space)
+ last_space = p;
+ } else {
+ if (*p == '\\')
+ p++;
+ last_space = NULL;
+ }
+
+ if (last_space)
+ *last_space = '\0';
}
int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..7becf96 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,25 @@ test_expect_success !MINGW 'quoting allows trailing
whitespace' '
test_cmp err.expect err
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+ rm -rf whitespace
+ mkdir whitespace
+ whitespace/trailing 1
+ whitespace/trailing 2
+ whitespace/trailing 3
+ whitespace/trailing 4 \\
+ whitespace/trailing 5 \\ \\
+ whitespace/untracked
+ echo whitespace/trailing 1 \\ ignore
+ echo whitespace/trailing 2 ignore
+ echo whitespace/trailing 3 ignore
+ echo whitespace/trailing 4 ignore
+ echo whitespace/trailing 5 ignore
+ echo whitespace/untracked expect
+ : err.expect
+ git ls-files -o -X ignore whitespace actual 2err
+ test_cmp expect actual
+ test_cmp err.expect err
+'
+
test_done
--
1.9.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: git-multimail: migration: Config is not iterable
On Sat, May 31, 2014 at 04:59:45PM +0200, Michael Haggerty wrote: On 05/29/2014 04:22 PM, Azat Khuzhin wrote: Using the latest version of git-multimail there is an issue with migration: $ ~azat/git-multimail/git-multimail/migrate-mailhook-config --overwrite Traceback (most recent call last): File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 271, in module main(sys.argv[1:]) File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 268, in main migrate_config(strict=options.strict, retain=options.retain, overwrite=options.overwrite) File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 159, in migrate_config if not _check_old_config_exists(old): File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 66, in _check_old_config_exists if name in old: TypeError: argument of type 'Config' is not iterable Tested on 2.6 and 2.7 python versions. If you revert 09d0d5b92203f019763e43cef1e57f76f117d2b4 (Get Python files to pass pep8's tests.) there issue goes away. I understand that this is not the right solution and I'm not the guru of python, so just let you know. Thanks for the bug report and for narrowing it down to the broken commit. I just pushed a fix to GitHub. Let me know if it works for you now. Yeah, it works, thanks! -- Respectfully Azat Khuzhin -- To unsubscribe from this list: send the line 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] Makefile: don't hardcode HEAD in dist target
Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}.
This makes sure the archive name and contents are consistent, if HEAD
has moved, but GIT-VERSION-FILE hasn't been regenerated yet.
Signed-off-by: Dennis Kaarsemaker den...@kaarsemaker.net
---
I have a somewhat odd setup in which I share a .git between multiple
checkouts for automated builds. To minimize locking time for parallel
builds with different options, there's only a lock around checkout and
running git describe $commit version, the builds themselves run in
parallel.
This works just fine except during 'make dist', which is hardcoded to
package up HEAD, but that's not always the commit that is actually
checked out, another process may have checked out something else.
I realize this setup is somewhat strange, but the only change necessary
to make this work is this one-line patch, so I hope that's acceptable.
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a53f3a8..63d9bac 100644
--- a/Makefile
+++ b/Makefile
@@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE
GIT_TARNAME = git-$(GIT_VERSION)
dist: git.spec git-archive$(X) configure
./git-archive --format=tar \
- --prefix=$(GIT_TARNAME)/ HEAD^{tree} $(GIT_TARNAME).tar
+ --prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree}
$(GIT_TARNAME).tar
@mkdir -p $(GIT_TARNAME)
@cp git.spec configure $(GIT_TARNAME)
@echo $(GIT_VERSION) $(GIT_TARNAME)/version
--
2.0.0-538-ga6b2d95
--
To unsubscribe from this list: send the line 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: Reset by checkout?
Felipe Contreras wrote: Atsushi Nakagawa wrote: Ok, the typical use case is: I'm on 'master' and I make a few test commits. Afterwards, I want to discard the commits and move back to 'origin/master'. I could type 'reset --hard origin/master' and risk blowing away dirty files if I'm not careful. Or, I could use reset by checkout and be carefree. Doesn't 'git reset orign/master' do that? Unless you want to keep the staged files, in which case adding the --stage and --work options I originally suggested[1] would help. So you could do `git reset --no-stage --no-work origin/master` Which is essentially the same as `git update-ref refs/heads/master origin/master`. [1] http://article.gmane.org/gmane.comp.version-control.git/247086 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nd/split-index] fixup! read-cache: new API write_locked_index instead of write_index/write_cache
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
I intended it resend the series after the comments I received, but it
looks like Junio has picked up all comments except this one, so
here's the fix.
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 377c877..4b709db 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -672,7 +672,7 @@ static void prepare_revs(struct replay_opts *opts)
static void read_and_refresh_cache(struct replay_opts *opts)
{
static struct lock_file index_lock;
- hold_locked_index(index_lock, 0);
+ int index_fd = hold_locked_index(index_lock, 0);
if (read_index_preload(the_index, NULL) 0)
die(_(git %s: failed to read the index), action_name(opts));
refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL,
NULL);
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line 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: Reset by checkout?
Andreas Schwab sch...@linux-m68k.org wrote: Atsushi Nakagawa at...@chejz.com writes: Ok, the typical use case is: I'm on 'master' and I make a few test commits. Afterwards, I want to discard the commits and move back to 'origin/master'. I could type 'reset --hard origin/master' and risk blowing away dirty files if I'm not careful. Or, I could use reset by checkout and be carefree. I think that is what 'reset --keep' is doing. I must admit, I didn't know about 'reset --keep'. As you've pointed out, it does look like the command I was after all along! And to think that it's been around since 1.7.1. Thanks! -- Atsushi Nakagawa at...@chejz.com Changes are made when there is inconvenience. -- To unsubscribe from this list: send the line 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: Reset by checkout?
Kevin Bracey ke...@bracey.fi wrote: On 31/05/2014 08:46, Atsushi Nakagawa wrote: `git checkout -B current-branch-name tree-ish` This is such an useful notion that I can fathom why there isn't a better, first-tier, alternative.q ... I guess in theory using checkout allows fancier extra options like --merge and --patch, but I don't think I've ever used those with checkout, let alone this mode, where I really do just want a reset, with safety checks. It does indeed have those fancier options. However, I just noticed there's even a 'reset --merge'! And like you say, I can't remember ever using 'checkout --merge' together with 'checkout -B'. The original git reset --hard used to be a pretty top-level command. It was used for aborting merges in particular. But I think it now stands out as being one of the only really dangerous porcelain commands, and I can't think of any real workflow it's still useful for. My thoughts exactly. I think the 'reset --soft/--mixed/--hard' pattern is so ingrained, that many people just don't realize there's a safer alternative. (I've heard work mates on more than one occasion recommending 'reset --hard' as the go-to command for discarding commits.) I believe this is likely because many third party GUI tools just don't support 'reset --keep', and these tools present a Reset... dialog with the de facto Soft/Mixed/Hard options. (Even 'gitk' does this.) Maybe it could now be modified to warn and require -f to overwrite anything in the working tree? If people just forgot about '--hard' and used '--mixed/--keep' for regular cases, '--hard' would effectively be -f. ;) While digging into this, it seems git reset --keep is actually pretty close to git checkout -B current branch. It certainly won't lose your workspace file, but unlike checkout it /does /forget what you've staged, which could be annoying. Maybe that could be modified to keep the index too? Yes, I didn't realize that 'reset --keep' existed and now I'm feeling a bit silly for asking. The index preservation artefact of 'checkout -B' could be useful, though I can't remember at this point if I've relied on it in the past. The documetation for 'reset --keep' is ambiguous about what happens to index entries of differing files, so modifying it may be an option if there's demand.. I'm going to try out 'reset --keep' for a while and see if it does get annoying. Cheers, -- Atsushi Nakagawa at...@chejz.com Changes are made when there is inconvenience. -- To unsubscribe from this list: send the line 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: Reset by checkout?
Felipe Contreras felipe.contre...@gmail.com wrote: Felipe Contreras wrote: Atsushi Nakagawa wrote: Ok, the typical use case is: I'm on 'master' and I make a few test commits. Afterwards, I want to discard the commits and move back to 'origin/master'. I could type 'reset --hard origin/master' and risk blowing away dirty files if I'm not careful. Or, I could use reset by checkout and be carefree. Doesn't 'git reset orign/master' do that? Unless you want to keep the staged files, in which case adding the --stage and --work options I originally suggested[1] would help. ... [1] http://article.gmane.org/gmane.comp.version-control.git/247086 What I was looking for is basically what 'git checkout' does to the working tree when it moves from one commit to another, as well as the semantic checks it offers such that I'm incapable of making an unrecoverable change (i.e. It aborts if I'm about to blow away changes that aren't committed.). I was introduced to 'git reset --keep' in another reply and that for most intent and purpose is what I think I was after. Cheers, -- Atsushi Nakagawa at...@chejz.com Changes are made when there is inconvenience. -- To unsubscribe from this list: send the line 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 v4 1/2] refs.c: optimize check_refname_component()
In a repository with many refs, check_refname_component can be a major
contributor to the runtime of some git commands. One such command is
git rev-parse HEAD
Timings for one particular repo, with about 60k refs, almost all
packed, are:
Old: 35 ms
New: 29 ms
Many other commands which read refs are also sped up.
Signed-off-by: David Turner dtur...@twitter.com
---
refs.c | 68 --
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/refs.c b/refs.c
index 28d5eca..62e2301 100644
--- a/refs.c
+++ b/refs.c
@@ -5,9 +5,32 @@
#include dir.h
#include string-list.h
+/* How to handle various characters in refnames:
+ * 0: An acceptable character for refs
+ * 1: End-of-component
+ * 2: ., look for a following . to reject .. in refs
+ * 3: @, look for a following { to reject @{ in refs
+ * 9: A bad character, reject ref
+ *
+ * See below for the list of illegal characters, from which
+ * this table is derived.
+ */
+static unsigned char refname_disposition[] = {
+ 1, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
+ 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
+ 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 2, 1,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 9,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 9, 0, 9, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 9, 9
+};
+
/*
- * Make sure ref is something reasonable to have under .git/refs/;
- * We do not like it if:
+ * Try to read one refname component from the front of refname.
+ * Return the length of the component found, or -1 if the component is
+ * not legal. It is legal if it is something reasonable to have under
+ * .git/refs/; We do not like it if:
*
* - any path component of it begins with ., or
* - it has double dots .., or
@@ -15,24 +38,7 @@
* - it ends with a /.
* - it ends with .lock
* - it contains a \ (backslash)
- */
-/* Return true iff ch is not allowed in reference names. */
-static inline int bad_ref_char(int ch)
-{
- if (((unsigned) ch) = ' ' || ch == 0x7f ||
- ch == '~' || ch == '^' || ch == ':' || ch == '\\')
- return 1;
- /* 2.13 Pattern Matching Notation */
- if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
- return 1;
- return 0;
-}
-
-/*
- * Try to read one refname component from the front of refname. Return
- * the length of the component found, or -1 if the component is not
- * legal.
*/
static int check_refname_component(const char *refname, int flags)
{
@@ -40,17 +46,25 @@ static int check_refname_component(const char *refname, int
flags)
char last = '\0';
for (cp = refname; ; cp++) {
- char ch = *cp;
- if (ch == '\0' || ch == '/')
+ unsigned char ch = (unsigned char) *cp;
+ char disp = refname_disposition[ch];
+ switch(disp) {
+ case 1:
+ goto out;
+ case 2:
+ if (last == '.')
+ return -1; /* Refname contains ... */
break;
- if (bad_ref_char(ch))
- return -1; /* Illegal character in refname. */
- if (last == '.' ch == '.')
- return -1; /* Refname contains ... */
- if (last == '@' ch == '{')
- return -1; /* Refname contains @{. */
+ case 3:
+ if (last == '@')
+ return -1; /* Refname contains @{. */
+ break;
+ case 9:
+ return -1;
+ }
last = ch;
}
+out:
if (cp == refname)
return 0; /* Component has zero length. */
if (refname[0] == '.') {
--
2.0.0.rc1.18.gf763c0f
--
To unsubscribe from this list: send the line 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 v4 2/2] refs.c: SSE4.2 optimizations for check_refname_component
Optimize check_refname_component using SSE4.2, where available.
git rev-parse HEAD is a good test-case for this, since it does almost
nothing except parse refs. For one particular repo with about 60k
refs, almost all packed, the timings are:
Look up table: 29 ms
SSE4.2:25 ms
This is about a 15% improvement.
The configure.ac changes include code from the GNU C Library written
by Joseph S. Myers joseph at codesourcery dot com.
Signed-off-by: David Turner dtur...@twitter.com
---
Makefile | 6 +++
aclocal.m4 | 6 +++
configure.ac | 17
git-compat-util.h | 20 +
refs.c | 116 +
t/t5511-refspec.sh | 13 ++
6 files changed, 161 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index a53f3a8..dd2127a 100644
--- a/Makefile
+++ b/Makefile
@@ -1326,6 +1326,11 @@ else
COMPAT_OBJS += compat/win32mmap.o
endif
endif
+ifdef NO_SSE42
+ BASIC_CFLAGS += -DNO_SSE42
+else
+ BASIC_CFLAGS += -msse4.2
+endif
ifdef OBJECT_CREATION_USES_RENAMES
COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1
endif
@@ -2199,6 +2204,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst
','\'',$(NO_UNIX_SOCKETS)))'\' $@
+ @echo NO_SSE42=\''$(subst ','\'',$(subst ','\'',$(NO_SSE42)))'\' $@
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' $@
endif
diff --git a/aclocal.m4 b/aclocal.m4
index f11bc7e..d9f3f19 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -38,3 +38,9 @@ AC_DEFUN([TYPE_SOCKLEN_T],
[#include sys/types.h
#include sys/socket.h])
])
+
+dnl Test a compiler option or options with an empty input file.
+dnl LIBC_TRY_CC_OPTION([options], [action-if-true], [action-if-false])
+AC_DEFUN([LIBC_TRY_CC_OPTION],
+[AS_IF([AC_TRY_COMMAND([${CC-cc} $1 -xc /dev/null -S -o /dev/null])],
+ [$2], [$3])])
diff --git a/configure.ac b/configure.ac
index b711254..3a5bda9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -382,6 +382,23 @@ AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a
system.]),
GIT_PARSE_WITH(tcltk))
#
+# Declare the with-sse42/without-sse42 options.
+AC_ARG_WITH(sse42,
+AS_HELP_STRING([--with-sse42],[use SSE4.2 instructions])
+AS_HELP_STRING([],[(default is YES if your compiler supports -msse4.2)]),
+GIT_PARSE_WITH(sse42))
+
+if test $NO_SSE42 != YesPlease; then
+ dnl Check if -msse4.2 works.
+ AC_CACHE_CHECK(for SSE4.2 support, cc_cv_sse42, [dnl
+ LIBC_TRY_CC_OPTION([-msse4.2], [cc_cv_sse42=yes], [cc_cv_sse42=no])
+ ])
+ if test $cc_cv_sse42 = no; then
+ NO_SSE42=1
+ fi
+fi
+
+GIT_CONF_SUBST([NO_SSE42])
## Checks for programs.
AC_MSG_NOTICE([CHECKS for programs])
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..254487a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -668,6 +668,26 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#endif
#endif
+#ifndef NO_SSE42
+#include nmmintrin.h
+/* Clang ships with a version of nmmintrin.h that's incomplete; if
+ * necessary, we define the constants that we're going to use. */
+#ifndef _SIDD_UBYTE_OPS
+#define _SIDD_UBYTE_OPS 0x00
+#define _SIDD_CMP_EQUAL_ANY 0x00
+#define _SIDD_CMP_RANGES0x04
+#define _SIDD_CMP_EQUAL_ORDERED 0x0c
+#define _SIDD_NEGATIVE_POLARITY 0x10
+#endif
+
+/* This is the system memory page size; it's used so that we can read
+ * outside the bounds of an allocation without segfaulting. It is
+ * assumed to be a power of 2. */
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+#endif
+
#ifdef UNRELIABLE_FSTAT
#define fstat_is_reliable() 0
#else
diff --git a/refs.c b/refs.c
index 62e2301..ed436ea 100644
--- a/refs.c
+++ b/refs.c
@@ -26,6 +26,25 @@ static unsigned char refname_disposition[] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 9, 9
};
+static int check_refname_component_trailer(const char *cp, const char
*refname, int flags)
+{
+ if (cp == refname)
+ return 0; /* Component has zero length. */
+ if (refname[0] == '.') {
+ if (!(flags REFNAME_DOT_COMPONENT))
+ return -1; /* Component starts with '.'. */
+ /*
+* Even if leading dots are allowed, don't allow .
+* as a component (.. is prevented by a rule above).
+*/
+ if (refname[1] == '\0')
+ return -1; /* Component equals .. */
+ }
+ if (cp - refname = 5 !memcmp(cp - 5, .lock, 5))
+ return -1; /* Refname ends with .lock. */
+ return cp - refname;
+}
+
/*
* Try to read one refname component from the front of refname.
* Return the
