Re: Re*: mergetool: support --tool-help option like difftool does
On Thu, Aug 23, 2012 at 10:39 AM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: Would the ability to resolve the various merge situations using the command-line be a wanted addition? This would let a submodule or deleted/modified encountering user do something like: $ git mergetool --theirs -- submodule ...and not have to remember the various git commands that it runs. Does it have to run various git commands? For a normal path, all it needs to do is git checkout --theirs $path, no? What does it mean to resolve a conflicted merge of a gitlink to take theirs? We obviously would want to point the resolved gitlink at the submodule commit their side wants in the resulting index but what, if any, should we do to the submodule itself? Stepping back a bit, if there is no conflict, and as a result of a clean merge (this applies to the case where you check out another branch that has different commit at the submodule path), if gitlink changed to point at a different commit in the submodule, what should happen? If you start from a clean working tree, with your gitlink pointing at the commit that matches HEAD in the submodule, and if the working tree of the submodule does not have any local modification, it may be ideal to check out the new commit in the submodule (are there cases where git checkout other_branch in the superproject does not want to touch the submodule working tree?). There are cases where it is not possible; checking out the new commit in the submodule working tree may not succeed due to local modifications. But is that kind of complication limited to the merge resolution case? Isn't it shared with normal switching branches case as well? If so, it could be that your git mergetool --theirs -- submodule is working around a wrong problem, and the right solution may be to make git checkout --theirs -- $path to always do an appropriate thing regardless of what kind of object $path is, no? True. Admittedly, I'm not a heavy submodule user myself so I tried crafting the directory vs submodule conflict to see what the usability is like. checkout --theirs and --ours could learn a few tricks. When trying to choose the directory in that situation I had to had to git rm --cached the submodule path so that git would recognize that there was no longer a conflict. That makes sense to me because I was following along by reading the mergetool code, but I don't think most users would know to git rm a path which they intend to keep. Afterwards, the .git file is left behind, which could cause problems elsewhere since we really don't want a .git file in that situation. I'm not even sure what to do about the .gitmodules file either. Here's the script I was using to setup the merge conflict in case anyone wants to get a feel for the usability around this edge case. Pass --submodule if you want the remote side to have a submodule. By default, the local side has a submodule and the remote side of the merge brings along a directory. That said, this really isn't an issue, per say. I first poked at it because I noticed that mergetool still needed stdin for some things. It's certainly an edge case, and perhaps this just shows that mergetool really is the right porcelain for the job when a user runs into these types of conflicts (the stdin thing really isn't an issue). #!/bin/sh if test $1 = --submodule then first=with-directory second=with-submodule echo local will be a directory, remote will be a submodule else first=with-submodule second=with-directory echo local will be a submodule, remote will be a directory fi repo=submodule-conflict-$$ echo creating $repo mkdir $repo cd $repo git init git commit --allow-empty -m'initial' git checkout -b with-directory master mkdir the-conflict touch the-conflict/path git add the-conflict/path git commit -m'added path into the-conflict' git checkout -b with-submodule master git submodule add ./ the-conflict git commit -m'added submodule as the-conflict' git checkout -b tmp-merge master git merge $first git merge $second -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Gettext poison rework
On Fri, Aug 24, 2012 at 7:43 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Still WIP but I'm getting closer. I dropped test-poisongen and started to use podebug [2] instead. Less code in git. podebug does not preserve shell variables yet. I'll follow that up at upstream [1]. With this series, if you have translation toolkit installed, you could do make pseudo-locale L=your language code make GETTEXT_POISON=$LANG test podebug supports a few way of rewriting translations. Currently unicode is used but you can change it via PODEBUG_OPTS t9001 is not happy with $LANG != C though. May need to add some prereq there. [1] http://bugs.locamotion.org/show_bug.cgi?id=2450 [2] http://translate.sourceforge.net/wiki/toolkit/podebug The reason I didn't do something like this to begin with is that gettext/glibc doesn't have support for fake locales, so you'd have to appropriate a real one for tests. It's good to see you poking the gettext mailing list about adding support far thot. But something like podebug gets around that quite nicely, so we can still have the testing the poison stuff was intended for, without the complexity of supporting it throughout all our i18n code. -- To unsubscribe from this list: send the line 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: [Q] Comparing differences introduced by two commits?
On Wednesday 22-August-2012 10:55:29 Jonathan del Strother wrote: On 22 August 2012 17:58, Junio C Hamano gits...@pobox.com wrote: Jonathan del Strother maill...@steelskies.com writes: On 22 August 2012 13:10, Brian Foster brian.fos...@maxim-ic.com wrote: ... In the past I've done: diff (git show A) (git show B) which produces rather messy output [...] Isn't this what interdiff is for? I'd never(?) heard of interdiff(1) — THANKS! With my current problem it produces (1) Some false results, and (2) Gets enough patch-rejects so as to be useful only in getting a 10km-high overview. Nonetheless, it's a help. Some searching hasn't found any suggestions I'm too happy with, albeit I've very possibly overlooked something. What about cherry picking B onto A, then showing the cherry-picked commit? [...] I often do git checkout A^ git cherry-pick B git diff A when queuing an updated patch. This works fairly well. I get conflicts (not surprising), which _probably_ corrolate rather well to the interdiff patch-rejects (not checked), but the advantage here is I can easily see what's going on (what the conflict _is_). Neither compares commit-comments, but that is a obviously a scriptable problem. As it so happens, it turns out my number of A/B pairs is rather less than expected (c.50 not the estimated c.90), of which c.10 get cherry-pick conflicts. So the problem is now looking quite tractable. Thanks for the help! cheers, -blf- -- Brian Foster Principal MTS, Software| La Ciotat, France Maxim Integrated Products | Web: http://www.maxim-ic.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: Reg:Git-ssh bug
On Thu, Aug 23, 2012 at 06:22:01PM -0400, Gokulramkumar Subramaniam wrote: I am new to Git and I am trying to add my machine with Git but it is failing through ssh method. Error received: $ ssh-add -l 2048 5f:6f:39:ed:b0:76:2e:d0:xx:xx:xx:xx:xx:xx:xx:xx id_rsa (RSA) Gokul$ ssh -vT g...@github.com [...] debug1: Authentications that can continue: publickey debug1: Next authentication method: publickey debug1: Offering RSA public key: id_rsa debug1: Authentications that can continue: publickey debug1: Offering RSA public key: /Users/Gokul/.ssh/id_rsa debug1: Authentications that can continue: publickey debug1: Trying private key: /Users/Gokul/.ssh/id_dsa debug1: No more authentication methods to try. Permission denied (publickey). That means that the server does not like your public key, typically because it does not match a keypair that has been uploaded. This is a GitHub issue, not a Git issue. Double-check that your key is listed at: https://github.com/settings/ssh and add it there if necessary. If it is still not working, then contact GitHub support (supp...@github.com). -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: in_merge_bases() is too expensive for recent pu update
Junio C Hamano gits...@pobox.com writes:
As a corollary, the is pu@{0} a fast-forward of pu@{1}? check does
not need merge base computation at all. The only thing it needs to
do is to prove pu@{1} is reachable from pu@{0}, and that can be done
the same way in which '1' can be proved unreachable from '2' in the
post processing phase as described above, i.e. it needs only the
first phase of running merge_bases_many() without postprocessing.
Well, yeah, you snipped this part from my original post :-)
} Even if this turns out to be flawed, we should also identify uses of
} in_merge_bases() where the real question was is_descendant_of() [I
} somewhat suspect that's all of them], and then replace is_descendant_of
} with a much cheaper lookup. This can be as simple as propagating a mark
} from the candidate until it either goes beyond all possible ancestors,
} or hits one of them.
As far as the real question goes, I'm purely guessing here -- it is
entirely possible that a bunch of the in_merge_bases() checks really do
need the pruned set of merge bases. But this particular check, and I
suspect a bunch of others, does not.
Then there's the next bit:
} By the way, the internal slowness of git-merge-base also affects the
} A...B syntax. For example,
}
} git rev-list --left-right --count @{upstream}...
}
} is used by the __git_ps1 code to determine the prompt display, which
} just got very slow for me today. Again, it should be easy to figure out
} the boundary of the symmetric difference simply by propagating two
} marks. I do not think that the result of A...B actually depends on
} figuring out exactly what the merge bases are, simply excluding *any*
} candidate without pruning is enough.
Apart from __git_ps1, this is also used by git-status, git-checkout and
'git branch -v' to show your branch is N behind and M ahead. Again
it's a bit of a hunch, but I think figuring out the symmetric difference
is a simple matter of propagating two marks and including only commits
that ended up having exactly one of them. At least the counting case
should be easy, rev-list is slightly harder to fix.
--
Thomas Rast
trast@{inf,student}.ethz.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: in_merge_bases() is too expensive for recent pu update
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@student.ethz.ch writes:
At the very least it should be possible to change in_merge_bases() to
not do any of the post-filtering; perhaps like the patch below.
I do not recall the details but the post-filtering was added after
the initial naive version without it that was posted to the list did
not work correctly in corner cases. I wouldn't doubt that N*(N-1)
loop can be optimized to something with log(N), but I offhand find
it hard to believe to not do any could be correct without seeing
more detailed analysis.
If one on one side, many on the other side in merge_bases_many()
confuses any of you in the readers, you can just ignore many-ness.
Getting merge bases for one against many twos using
merge_bases_many() is exactly the same as getting merge bases for
one against a (fictitious) single commit you can make by merging
all twos.
So we can think about the degenerate case of merge base between two
commits A and B in the following discussion.
A merge-base between A and B is defined to be a commit that is a
common ancestor of both A and B, and that is not an ancestor of any
other merge-base between A and B.
Ok, under that definition I really do think that it's easy.
You have all the pieces here but one:
Start from A and B. Follow from B to find 'x' and paint it in blue,
follow from A to find 'y' and paint it in amber. Follow from 'x' to
'1', paint it in blue. Follow from 'y' to '1', paint it in amber
but notice that it already is painted in blue.
[...]
o---o
/ \
/ y---A
/ /
---2---z---1---x---B
\ /
o---o
[...]
So we need to notice that '1' and '2' have ancestry relation in
order to reject '2' as common but not merge-base. One way of
doing so is not to stop at '1' and keep digging (and eventually we
find that '2' is what we could reach from '1' that already is a
merge base), but then we will be susceptible to the same kind of
clock skew issue as the revision traverser.
I think that is *the* way to do it.
And the way to fix the clock skew issue (also in the revision traversal)
is Peff's generation number cache. Just keep propagating marks, digging
in generation order, until the generations prove that nothing new can
happen.
[Side note, in reply to an earlier comment in the rev-list thread: I
agree with Peff's reasoning that a cache is better than a commit
header.]
The precise termination condition should be:
There are no more one-colored commits to walk, and
The maximum generation of the remaining commits in the walk is less
than the minimum generation of the merge base candidates
Then the generation numbers ensure that no commit in the walk (which by
then only propagates STALE marks) can possibly be a descendant of any of
the candidates. At that point, the candidates are the true set of merge
bases.
I conjecture that every history walking problem can be solved in time
linear in the number of commits once we properly use the generation
numbers ;-)
--
Thomas Rast
trast@{inf,student}.ethz.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
[PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- This time I hopefully didn't screw up whitespace and line breaks. Makefile| 18 ++ compat/win32/poll.c | 8 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 66e8216..e150816 100644 --- a/Makefile +++ b/Makefile @@ -152,6 +152,11 @@ all:: # # Define NO_MMAP if you want to avoid mmap. # +# Define NO_SYS_POLL_H if you don't have sys/poll.h. +# +# Define NO_POLL if you do not have or do not want to use poll. +# This also implies NO_SYS_POLL_H. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1216,7 +1221,7 @@ ifeq ($(uname_S),Windows) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease NO_UNIX_SOCKETS = YesPlease @@ -1257,7 +1262,7 @@ ifeq ($(uname_S),Windows) BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib @@ -1312,7 +1317,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_UNIX_SOCKETS = YesPlease NO_SETENV = YesPlease @@ -1347,7 +1352,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o EXTLIBS += -lws2_32 PTHREAD_LIBS = X = .exe @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT BASIC_CFLAGS += -DNO_GETTEXT USE_GETTEXT_SCHEME ?= fallthrough endif +ifdef NO_POLL + NO_SYS_POLL_H = YesPlease + COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h + COMPAT_OBJS += compat/win32/poll.o +endif ifdef NO_STRCASESTR COMPAT_CFLAGS += -DNO_STRCASESTR COMPAT_OBJS += compat/strcasestr.o diff --git a/compat/win32/poll.c b/compat/win32/poll.c index 403eaa7..49541f1 100644 --- a/compat/win32/poll.c +++ b/compat/win32/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Prefer sysconf(_SC_OPEN_MAX) over getrlimit(RLIMIT_NOFILE,...)
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
As discussed now as a small helper function rather than #ifdef/#endif in the
primary flow of the code.
And hopefully without having screwed up whitespace and line breaks
sha1_file.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index af5cfbd..427f9e6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -731,6 +731,20 @@ void free_pack_by_name(const char *pack_name)
}
}
+static unsigned int get_max_fd_limit(void)
+{
+#ifdef _SC_OPEN_MAX
+ return sysconf(_SC_OPEN_MAX);
+#else
+ struct rlimit lim;
+
+ if (getrlimit(RLIMIT_NOFILE, lim))
+ die_errno(cannot get RLIMIT_NOFILE);
+
+ return lim.rlim_cur;
+#endif
+}
+
/*
* Do not call this directly as this leaks p-pack_fd on error return;
* call open_packed_git() instead.
@@ -747,13 +761,7 @@ static int open_packed_git_1(struct packed_git *p)
return error(packfile %s index unavailable, p-pack_name);
if (!pack_max_fds) {
- struct rlimit lim;
- unsigned int max_fds;
-
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
-
- max_fds = lim.rlim_cur;
+ unsigned int max_fds = get_max_fd_limit();
/* Save 3 for stdin/stdout/stderr, 22 for work */
if (25 max_fds)
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Don't use curl_easy_strerror prior to curl-7.12.0
This reverts be22d92 (http: avoid empty error messages for some curl errors,
2011-09-05) on platforms with older versions of libcURL.
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
Resend, regardless that Junio said this not to be needed, as I don't see it
applied yet.
Also tried to fix the formatting issues too, to practice submissions ;-)
http.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/http.c b/http.c
index b61ac85..18bc6bf 100644
--- a/http.c
+++ b/http.c
@@ -806,10 +806,12 @@ static int http_request(const char *url, void *result,
int target, int options)
ret = HTTP_REAUTH;
}
} else {
+#if LIBCURL_VERSION_NUM = 0x070c00
if (!curl_errorstr[0])
strlcpy(curl_errorstr,
curl_easy_strerror(results.curl_result),
sizeof(curl_errorstr));
+#endif
ret = HTTP_ERROR;
}
} else {
--
1.7.12
--
To unsubscribe from this list: send the line 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] Ignore trailing slash in mkdir() on platforms that can't deal with this
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
compat/mkdir.c | 24
1 file changed, 24 insertions(+)
create mode 100644 compat/mkdir.c
diff --git a/compat/mkdir.c b/compat/mkdir.c
new file mode 100644
index 000..9e253fb
--- /dev/null
+++ b/compat/mkdir.c
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */
+int compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode)
+{
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0';
+ }
+ else
+ tmp_dir = (char *)dir;
+
+ retval = mkdir(tmp_dir, mode);
+ if (tmp_dir != dir)
+ free(tmp_dir);
+
+ return retval;
+}
--
1.7.12
--
To unsubscribe from this list: send the line 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] Ignore trailing slash in mkdir() on platforms that can't deal with this
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- git-compat-util.h | 5 + 1 file changed, 5 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 35b095e..34f040f 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -162,6 +162,11 @@ #define probe_utf8_pathname_composition(a,b) #endif +#ifdef MKDIR_WO_TRAILING_SLASH +#define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b)) +extern int compat_mkdir_wo_trailing_slash(const char*, mode_t); +#endif + #ifndef NO_LIBGEN_H #include libgen.h #else -- 1.7.12 -- To unsubscribe from this list: send the line 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] Support for setitimer() on platforms lacking it
Implementation includes getitimer(), but for now it is static.
Supports ITIMER_REAL only.
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
May need a header file for ITIMER_*, struct itimerval and the prototypes,
But for now, and the HP NonStop platform this isn't needed, here
sys/time has ITIMER_* and struct timeval, and the prototypes can
vo into git-compat-util.h for now (Patch 2/2)
compat/itimer.c | 50 ++
1 file changed, 50 insertions(+)
create mode 100644 compat/itimer.c
diff --git a/compat/itimer.c b/compat/itimer.c
new file mode 100644
index 000..713f1ff
--- /dev/null
+++ b/compat/itimer.c
@@ -0,0 +1,50 @@
+#include ../git-compat-util.h
+
+static int git_getitimer(int which, struct itimerval *value)
+{
+ int ret = 0;
+
+ switch (which) {
+ case ITIMER_REAL:
+ value-it_value.tv_usec = 0;
+ value-it_value.tv_sec = alarm(0);
+ ret = 0; /* if alarm() fails, we get a SIGLIMIT */
+ break;
+ case ITIMER_VIRTUAL: /* FALLTHRU */
+ case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
+ default: errno = EINVAL; ret = -1;
+ }
+ return ret;
+}
+
+int git_setitimer(int which, const struct itimerval *value,
+ struct itimerval *ovalue)
+{
+ int ret = 0;
+
+ if (!value
+ || value-it_value.tv_usec 0
+ || value-it_value.tv_usec 100
+ || value-it_value.tv_sec 0) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ else if (ovalue)
+ if (!git_getitimer(which, ovalue))
+ return -1; /* errno set in git_getitimer() */
+
+ else
+ switch (which) {
+ case ITIMER_REAL:
+ alarm(value-it_value.tv_sec +
+ (value-it_value.tv_usec 0) ? 1 : 0);
+ ret = 0; /* if alarm() fails, we get a SIGLIMIT */
+ break;
+ case ITIMER_VIRTUAL: /* FALLTHRU */
+ case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
+ default: errno = EINVAL; ret = -1;
+ }
+
+ return ret;
+}
--
1.7.12
--
To unsubscribe from this list: send the line 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] Support for setitimer() on platforms lacking it
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- Seems it needs my mkdir() ignoretraile slash patch first to be applied cleanly... It is independent of it otherwise. git-compat-util.h | 5 + 1 file changed, 5 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 34f040f..a047221 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -162,6 +162,11 @@ #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NO_SETITIMER +#define setitimer(a,b,c) git_setitimer((a),(b),(c)) +extern int git_setitimer(int, const struct itimerval *, struct itimerval *); +#endif + #ifdef MKDIR_WO_TRAILING_SLASH #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b)) extern int compat_mkdir_wo_trailing_slash(const char*, mode_t); -- 1.7.12 -- To unsubscribe from this list: send the line 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] Gettext poison rework
On Fri, Aug 24, 2012 at 3:51 PM, Ævar Arnfjörð Bjarmason ava...@gmail.com wrote: [1] http://bugs.locamotion.org/show_bug.cgi?id=2450 [2] http://translate.sourceforge.net/wiki/toolkit/podebug The reason I didn't do something like this to begin with is that gettext/glibc doesn't have support for fake locales, so you'd have to appropriate a real one for tests. It's good to see you poking the gettext mailing list about adding support far thot. I don't see glibc getting fake locale support any time soon even if it's more open than before. I can try to make gettext support pseudo translations, though not sure if I can make it. But something like podebug gets around that quite nicely, so we can still have the testing the poison stuff was intended for, without the complexity of supporting it throughout all our i18n code. But yes, even if gettext does not have native support, relying on podebug should be ok. Whatever changes we need, other projects might too. It's better to do it outside of git. -- Duy1 -- To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update
On Thu, Aug 23, 2012 at 9:20 PM, Thomas Rast tr...@student.ethz.ch wrote:
At the very least it should be possible to change in_merge_bases() to
not do any of the post-filtering; perhaps like the patch below. It
passes the test suite. The whole merge bases of A and a list of Bs
thing is blowing my overheated mind, though, so I'm not able to convince
myself that it is correct in all cases.
diff --git i/commit.c w/commit.c
index 65a8485..70427ab 100644
--- i/commit.c
+++ w/commit.c
@@ -837,10 +837,13 @@ int in_merge_bases(struct commit *commit, struct commit
**reference, int num)
struct commit_list *bases, *b;
int ret = 0;
- if (num == 1)
- bases = get_merge_bases(commit, *reference, 1);
- else
+ if (num != 1)
die(not yet);
+
+ bases = merge_bases_many(commit, 1, reference);
+ clear_commit_marks(commit, all_flags);
+ clear_commit_marks(*reference, all_flags);
+
for (b = bases; b; b = b-next) {
if (!hashcmp(commit-object.sha1, b-item-object.sha1)) {
ret = 1;
Without looking into detail (as I'm not familiar with this code), this
patch does not help much. Without the patch:
$ time ./git merge-base a4f2db3 b95a282
ed36e5bd41f7192e42e9b4c573875a343a9daf48
real0m19.988s
user0m19.797s
sys 0m0.082s
With the patch:
$ time ./git merge-base a4f2db3 b95a282
ed36e5bd41f7192e42e9b4c573875a343a9daf48
real0m19.560s
user0m19.448s
sys 0m0.037s
--
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 00/17] Clean up how fetch_pack() handles the heads list
From: Junio C Hamano gits...@pobox.com Sent: Friday, August 24, 2012 5:23 AM Jeff King p...@peff.net writes: It may be (?) that it is a good time to think about a 'datedepth' capability to bypass the current counted-depth shallow fetch that can cause so much trouble. With a date limited depth the relevant tags could also be fetched. I don't have anything against such an idea, but I think it is orthogonal to the issue being discussed here. Correct. The biggest problem with the shallow hack is that the deepening fetch counts from the tip of the refs at the time of deepening when deepening the history (i.e. clone --depth, followed by number of fetch, followed by fetch --depth). If you start from a shallow clone of depth 1, repeated fetch to keep up while the history grew by 100, you would still have a connected history down to the initial cauterization point, and fetch --depth=200 would give you a history that is deeper than your original clone by 100 commits. But if you start from the same shallow clone of depth 1, did not do anything while the history grew by 100, and then decide to fetch again with fetch --depth=20, it does not grow. It just makes 20-commit deep history from the updated tip, and leave the commit from your original clone dangling. The problem with depth does not have anything to do with how it is specified at the UI level. That is, correct me if I'm wrong, the server currently lacks a capability to provide anything other than the counted depth shallow response (if available). Hence my comment about needing an additional server side capability, though that would need a well thought out set of use cases to make it useful. The end-user input is used to find out at what set of commits the history is cauterized, and once they are computed, the shallow logic solely works on is the history before these cauterization points, or after, in topological terms. (and it has to be that way to make sure we get reproducible results). Even if a new way to specify these cauterization points in terms of date is introduced, it does not change anything and does not solve the fundamental problem the code has when deepening. fundamental problem = no server capability other than counted depth. -- To unsubscribe from this list: send the line 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: a small git proposal
Thanks for the tip. It should give me a good starting point for what I'm about to do, since notes seem to be able to add comments for objects without changing the commit tree (which was one of the things I was aiming for and quite frankly, one of the parts that worried me on the implementation side). However, what I want to implement has a few differences: a) the flags I'm proposing form a limited set of strings, as I'm interested in finding which files have a particular flag (I did mention the idea to add comment as well when adding a certain flag, but that was something extra, since most of the searching/listing was around the set of flags of a certain file, not around the comment itself... I can cook up some more usage and output examples if anyone thinks it can clear things up). I realize this can be achieved by using a sound naming convention (and a simple grep for a particular prefix when listing all notes would handle the search by flag I mentioned) - unfortunately, some other differences don't seem to be covered as you'll see below b) I would like to have all subsequent versions of a file to inherit the flags/tags of the original file, until specified otherwise; in the original idea that a 'feature tag' (or 'flag' as I keep calling them lately - seems a better name that 'feature tag') remains on the file until someone decides that feature is no longer implemented in the file (for example, a file implements a certain technique since version 3 until version 15, when the implementation of a particular method changed entirely). Unfortunately, what seems to be a good choice to preserve a state of a file until not needed are branches, but then I would need to have the same version of the file on different branches (a file can have multiple flags after all, since multiple features are usually implemented in a file) Anyway, I just wanted to point out that the notes you suggested are not quite what I was looking for, but it should be a good implementation starting point, so again, lots of thanks. Catalin Pol On Thu, Aug 23, 2012 at 6:16 PM, Hilco Wijbenga hilco.wijbe...@gmail.com wrote: On 23 August 2012 08:10, Catalin Pol catalin@gmail.com wrote: Hi everyone, This is my first email to this mailing list, so this may be somehow too straight forward... the idea is that I was thinking to develop a new feature in Git (although I'm kind of new to git myself). I wrote a small description of what I intend to do and I figured I could use some pointers (how I can improve it, any possible usage scenarios you can think for it and so on). Details are available at the gist link below or as attachment to this email (I zipped the text file since it was more it is larger than 10k and I didn't want it to get rejected by the email server... although it still might) gist link:https://gist.github.com/3437530 I made the gist public, so feel free to edit it directly... or, if you prefer, just email me with any comments. I'm opened to any suggestion, so feel free to send me any kind of comment (maybe I'm trying to implement something that is already in git for example, and since I'm a bit of a newbie in the git world, I didn't notice any way to obtain my desired workflow). Thanks in advance for any feedback, Have you looked at git notes? -- To unsubscribe from this list: send the line 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] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Friday, August 24, 2012 11:45 AM To: Junio C Hamano (gits...@pobox.com) Cc: git@vger.kernel.org; Erik Faye-Lund (kusmab...@gmail.com) Subject: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- This time I hopefully didn't screw up whitespace and line breaks. Makefile| 18 ++ compat/win32/poll.c | 8 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 66e8216..e150816 100644 --- a/Makefile +++ b/Makefile @@ -152,6 +152,11 @@ all:: # # Define NO_MMAP if you want to avoid mmap. # +# Define NO_SYS_POLL_H if you don't have sys/poll.h. +# +# Define NO_POLL if you do not have or do not want to use poll. +# This also implies NO_SYS_POLL_H. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1216,7 +1221,7 @@ ifeq ($(uname_S),Windows) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease NO_UNIX_SOCKETS = YesPlease @@ -1257,7 +1262,7 @@ ifeq ($(uname_S),Windows) BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H - D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex - Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib @@ -1312,7 +1317,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_UNIX_SOCKETS = YesPlease NO_SETENV = YesPlease @@ -1347,7 +1352,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o EXTLIBS += -lws2_32 PTHREAD_LIBS = X = .exe @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT BASIC_CFLAGS += -DNO_GETTEXT USE_GETTEXT_SCHEME ?= fallthrough endif +ifdef NO_POLL + NO_SYS_POLL_H = YesPlease + COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h + COMPAT_OBJS += compat/win32/poll.o +endif ifdef NO_STRCASESTR COMPAT_CFLAGS += -DNO_STRCASESTR COMPAT_OBJS += compat/strcasestr.o diff --git a/compat/win32/poll.c b/compat/win32/poll.c index 403eaa7..49541f1 100644 --- a/compat/win32/poll.c +++ b/compat/win32/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. Another is to just ignore the warning, at least here it seems to work just fine? Or to avoid using an unnamed union. But the later 2 cases would still mean that we include the wrong dirent.h, so the 1st solution seems the cleanest. Any other idea? Let me know your thoughts... Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
libgit2 status
What is the status of libgit2 WRT the overall git project? I recall that there was some discussion of basing bits of git on libgit2 once it matures. I ask because I'm starting a project to improve the abysmal speed of git-subtree split. It's unbearably slow at the moment and as far as I can puzzle out it's due almost entirely to repeated subshell invocations to run git commands. I was planning on doing some experiments rewriting bits of git-subtree using libgit2 but I want to make sure that that isn't wasted work. It appears to be exactly what I need to code bits of git-subtree natively. Thoughts? -Dave -- To unsubscribe from this list: send the line 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] branch -v: align even when the first column is in UTF-8
Branch names are usually in ASCII so they are not the problem. The
problem most likely comes from (no branch) translation, which is in
UTF-8 and makes length calculation just wrong.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
So far all git translations are utf-8 compatible. Branch names may
use filesystem encoding, but then packed-refs specifies no encoding.
Anyway branch names should be in utf-8.. at least internally, imo.
builtin/branch.c | 8 +---
1 tập tin đã bị thay đổi, 5 được thêm vào(+), 3 bị xóa(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 0e060f2..7c1ffa8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -17,6 +17,7 @@
#include revision.h
#include string-list.h
#include column.h
+#include utf8.h
static const char * const builtin_branch_usage[] = {
git branch [options] [-r | -a] [--merged | --no-merged],
@@ -490,11 +491,12 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
}
strbuf_addf(name, %s%s, prefix, item-name);
- if (verbose)
+ if (verbose) {
+ int utf8_compensation = strlen(name.buf) -
utf8_strwidth(name.buf);
strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color),
- maxwidth, name.buf,
+ maxwidth + utf8_compensation, name.buf,
branch_get_color(BRANCH_COLOR_RESET));
- else
+ } else
strbuf_addf(out, %c %s%s%s, c, branch_get_color(color),
name.buf, branch_get_color(BRANCH_COLOR_RESET));
--
1.7.12.rc2.18.g61b472e
--
To unsubscribe from this list: send the line 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: misleading diff-hunk header
On Tue, Aug 21, 2012 at 10:52:03AM -0700, Junio C Hamano wrote:
diff.{type}.xfuncname seems to start searching backwards in
from the beginning of the hunk, not the first differing line.
[...]
@@ -4,4 +4,5 @@ int call_me(int maybe)
int main()
{
+ return 0;
}
misleadingly suggesting that the change occurred in the call_me()
function, rather than in main()
I think that's intentional, and matches what 'diff -p' does. It gives
you the context before the hunk. After all, if a new function starts in
the leading context lines, you can see that in the usual diff data.
Correct. It is about give the user _more_ hint/clue on the context
of the hunk, in addition to what the user can see in the
pre-context of the hunk, so it is pointless to hoist int main()
there.
I don't think it is pointless. If you are skimming a diff, then the hunk
headers stand out to easily show which functions were touched. Of
course, as you mentioned later in your email, it is not an exhaustive
list, and I think for Tim's use case, he needs to actually read and
parse the whole patch.
But mentioning call_me here _is_ pointless, because it is not relevant
context at all (it was not modified; it just happens to be located near
the code in question). So I would argue that showing main() is more
useful to a reader.
It gets even more obvious as you increase the context. Imagine I have
code like this:
int foo(void)
{
return 1;
}
int bar(void)
{
return 2;
}
int baz(void)
{
return 3;
}
and I modify baz to return 4 instead. With the regular diff
settings, the hunk header would claim that bar() is the context in the
hunk header. But if I ask for -U7, then foo() is mentioned in the hunk
header. To me, that doesn't make sense; the modification is exactly the
same, so why would the hunk header differ?
I suppose one could argue that the hunk header is not showing the
context of the change, but rather the context of the surrounding context
lines. But that doesn't seem useful to me.
We discussed this a while ago and you did a how about this patch:
http://article.gmane.org/gmane.comp.version-control.git/181385
Gmane seems to be acting up this morning, so here is the patch (and your
comment) for reference:
Would this be sufficient? Instead of looking for the first line that
matches the beginning pattern going backwards starting from one line
before the displayed context, we start our examination at the first line
shown in the context.
xdiff/xemit.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 277e2ee..5f9c0e0 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
if (xecfg-flags XDL_EMIT_FUNCNAMES) {
long l;
- for (l = s1 - 1; l = 0 l funclineprev; l--) {
+ for (l = s1; l = 0 l funclineprev; l--) {
const char *rec;
long reclen = xdl_get_rec(xe-xdf1, l, rec);
long newfunclen = ff(rec, reclen, funcbuf,
In the case we were discussing then, the modified function started on
the first line of context. But as Tim's example shows, it doesn't
necessarily have to. I think it would make more sense to start counting
from the first modified line.
-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: misleading diff-hunk header
On 08/24/12 09:29, Jeff King wrote:
On Tue, Aug 21, 2012 at 10:52:03AM -0700, Junio C Hamano wrote:
diff.{type}.xfuncname seems to start searching backwards in
from the beginning of the hunk, not the first differing line.
[...]
@@ -4,4 +4,5 @@ int call_me(int maybe)
int main()
{
+ return 0;
}
misleadingly suggesting that the change occurred in the call_me()
function, rather than in main()
I think that's intentional, and matches what 'diff -p' does.
...
xdiff/xemit.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 277e2ee..5f9c0e0 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
if (xecfg-flags XDL_EMIT_FUNCNAMES) {
long l;
-for (l = s1 - 1; l = 0 l funclineprev; l--) {
+for (l = s1; l = 0 l funclineprev; l--) {
const char *rec;
long reclen = xdl_get_rec(xe-xdf1, l, rec);
long newfunclen = ff(rec, reclen, funcbuf,
In the case we were discussing then, the modified function started on
the first line of context. But as Tim's example shows, it doesn't
necessarily have to. I think it would make more sense to start counting
from the first modified line.
Junio mentions that it matches the diff -p output, though I'd
consider that a bug in diff as well, since the diff(1) man/info
pages state -p Show which C function each change is in. In the
above (both with diff -p and with git), the change was clearly in
main() but it's not showing main(). Documented behavior and
implemented behavior conflict.
Starting at the first differing line rather than the first line of
context in the hunk would ameliorate this. It doesn't address what
happens if multiple functions were changed in the same hunk, but at
least it becomes correct for the first one. More complex code might
be doable to split hunks if an xfuncname match occurs between two
disjoint changes in the same hunk. But for my purposes here, the
above should suffice.
-tkc
--
To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update
On Fri, Aug 24, 2012 at 11:43:40AM +0200, Thomas Rast wrote: Start from A and B. Follow from B to find 'x' and paint it in blue, follow from A to find 'y' and paint it in amber. Follow from 'x' to '1', paint it in blue. Follow from 'y' to '1', paint it in amber but notice that it already is painted in blue. [...] o---o / \ / y---A / / ---2---z---1---x---B \ / o---o [...] So we need to notice that '1' and '2' have ancestry relation in order to reject '2' as common but not merge-base. One way of doing so is not to stop at '1' and keep digging (and eventually we find that '2' is what we could reach from '1' that already is a merge base), but then we will be susceptible to the same kind of clock skew issue as the revision traverser. I think that is *the* way to do it. And the way to fix the clock skew issue (also in the revision traversal) is Peff's generation number cache. Just keep propagating marks, digging in generation order, until the generations prove that nothing new can happen. I thought you were just interested in speeding up is_descendent_of. You should be able to do that without a generation number. Just start from A and B as above, do the two-color painting, and do not add the parents of any two-color commits (because you know they are ancestors of both, and therefore you cannot find either by looking backwards). If you paint B amber, then it is a descendent of A (and vice versa if you paint A blue). Clock skew may make you take longer (because you may go depth-first down an uninteresting chain of commits), but it should never give you the wrong answer. It's not as fast as using a generation-based cutoff (because you have to keep walking to the actual shared ancestor), but in practice it's usually fine. The reason I did not do that for tag --contains is that I wanted to do a simultaneous walk for all tags. That would require N color bits for N tags, and we have a limited space in each commit object. I didn't time using a separate hash table to store those bits outside of the commit objects. That would have a higher constant, but should still yield good big-O complexity. [Side note, in reply to an earlier comment in the rev-list thread: I agree with Peff's reasoning that a cache is better than a commit header.] I still think it's a good idea to keep it out of the commit header. But I think I might lean towards tying it to the pack index (i.e., generating it when we generate the index, and depending on the sha1 table in the index). That would be smaller, and gets rid of any complexity or performance issues with making incremental updates to the cache. Loose objects wouldn't have a cached version number, but that's OK; in practice you can quickly walk backwards to a commit that _is_ cached. -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: in_merge_bases() is too expensive for recent pu update
Thomas Rast tr...@student.ethz.ch writes: Well, yeah, you snipped this part from my original post :-) } Even if this turns out to be flawed, we should also identify uses of } in_merge_bases() where the real question was is_descendant_of() [I } somewhat suspect that's all of them], and then replace is_descendant_of } with a much cheaper lookup. This can be as simple as propagating a mark } from the candidate until it either goes beyond all possible ancestors, } or hits one of them. Yeah, I agree with the above, and the cheaper lookup would probably be merge-bases-many() without postprocess. -- To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update
Thomas Rast tr...@student.ethz.ch writes: Junio C Hamano gits...@pobox.com writes: ... Start from A and B. Follow from B to find 'x' and paint it in blue, follow from A to find 'y' and paint it in amber. Follow from 'x' to '1', paint it in blue. Follow from 'y' to '1', paint it in amber but notice that it already is painted in blue. [...] o---o / \ / y---A / / ---2---z---1---x---B \ / o---o [...] So we need to notice that '1' and '2' have ancestry relation in order to reject '2' as common but not merge-base. One way of doing so is not to stop at '1' and keep digging (and eventually we find that '2' is what we could reach from '1' that already is a merge base), but then we will be susceptible to the same kind of clock skew issue as the revision traverser. I think that is *the* way to do it. But we do not live in *that* world. At least not yet. I conjecture that every history walking problem can be solved in time linear in the number of commits once we properly use the generation numbers ;-) I would conjecture that too, but we do not live in that world yet. -- To unsubscribe from this list: send the line 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] Gettext poison rework
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Still WIP but I'm getting closer. I dropped test-poisongen and started to use podebug [2] instead. Less code in git. podebug does not preserve shell variables yet. I'll follow that up at upstream [1]. Thanks; this looks promising. -- To unsubscribe from this list: send the line 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] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Joachim Schmitz j...@schmitz-digital.de writes: There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. I think that is the most sensible way to go, because poll.[ch] (1) has proven itself to be useful outside the context of win32, and (2) the code is coming from gnulib anyway. I thought I already suggested going that route in my previous review, but perhaps I forgot to do so. -- To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update
Jeff King p...@peff.net writes: I thought you were just interested in speeding up is_descendent_of. You should be able to do that without a generation number. Just start from A and B as above, do the two-color painting, and do not add the parents of any two-color commits (because you know they are ancestors of both, and therefore you cannot find either by looking backwards). If you paint B amber, then it is a descendent of A (and vice versa if you paint A blue). Yes, that is what merge_bases_many() (the one without the post processing to cull candidates) is primarily doing. The function also does the STALE bit processing that aims to reduce the number of candidates in no clock skew cases with minimum effort, to mimimize the cost of get_merge_bases_many() in the post-processing phase, but removing the STALE bit processing shouldn't affect the correctness of get_merge_bases_many() and would help performance of merge_bases_many() proper, which is useful for is_descendant_of(). The reason I did not do that for tag --contains is that I wanted to do a simultaneous walk for all tags. That would require N color bits for N tags, and we have a limited space in each commit object. I didn't time using a separate hash table to store those bits outside of the commit objects. That would have a higher constant, but should still yield good big-O complexity. It may not be a bad idea to at least try and see the performance implications of moving many users of object-flags to a new implementation of per-purpose flag bits based on the decoration infrastracture. -- To unsubscribe from this list: send the line 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] Prefer sysconf(_SC_OPEN_MAX) over getrlimit(RLIMIT_NOFILE,...)
Joachim Schmitz j...@schmitz-digital.de writes:
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
As discussed now as a small helper function rather than #ifdef/#endif in the
primary flow of the code.
And hopefully without having screwed up whitespace and line breaks
The formatting looks fine.
Perhaps I am being overly paranoid, but I would prefer not to change
things for people who have been using getrlimit(). For them, if
they also have sysconf(_SC_OPEN_MAX), your code _ought to_ work, but
if it does not work for whatever reason (perhaps some platforms
claim to have both, but getrlimit() works and sysconf(_SC_OPEN_MAX)
is broken), it will given them an unnecessary regression.
So how about doing it this way instead?
-- 8 --
Subject: sha1_file.c: introduce get_max_fd_limit() helper
Not all platforms have getrlimit(), but there are other ways to see
the maximum number of files that a process can have open. If
getrlimit() is unavailable, fall back to sysconf(_SC_OPEN_MAX) if
available, and use OPEN_MAX from limits.h.
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
sha1_file.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git c/sha1_file.c w/sha1_file.c
index af5cfbd..9152974 100644
--- c/sha1_file.c
+++ w/sha1_file.c
@@ -731,6 +731,24 @@ void free_pack_by_name(const char *pack_name)
}
}
+static unsigned int get_max_fd_limit(void)
+{
+#ifdef RLIMIT_NOFILE
+ struct rlimit lim;
+
+ if (getrlimit(RLIMIT_NOFILE, lim))
+ die_errno(cannot get RLIMIT_NOFILE);
+
+ return lim.rlim_cur;
+#elif defined(_SC_OPEN_MAX)
+ return sysconf(_SC_OPEN_MAX);
+#elif defined(OPEN_MAX)
+ return OPEN_MAX;
+#else
+ return 1; /* see the caller ;-) */
+#endif
+}
+
/*
* Do not call this directly as this leaks p-pack_fd on error return;
* call open_packed_git() instead.
@@ -747,13 +765,7 @@ static int open_packed_git_1(struct packed_git *p)
return error(packfile %s index unavailable, p-pack_name);
if (!pack_max_fds) {
- struct rlimit lim;
- unsigned int max_fds;
-
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
-
- max_fds = lim.rlim_cur;
+ unsigned int max_fds = get_max_fd_limit();
/* Save 3 for stdin/stdout/stderr, 22 for work */
if (25 max_fds)
--
To unsubscribe from this list: send the line 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: misleading diff-hunk header
On Fri, Aug 24, 2012 at 10:29:09AM -0400, Jeff King wrote:
Would this be sufficient? Instead of looking for the first line that
matches the beginning pattern going backwards starting from one line
before the displayed context, we start our examination at the first line
shown in the context.
xdiff/xemit.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 277e2ee..5f9c0e0 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
if (xecfg-flags XDL_EMIT_FUNCNAMES) {
long l;
- for (l = s1 - 1; l = 0 l funclineprev; l--) {
+ for (l = s1; l = 0 l funclineprev; l--) {
const char *rec;
long reclen = xdl_get_rec(xe-xdf1, l, rec);
long newfunclen = ff(rec, reclen, funcbuf,
In the case we were discussing then, the modified function started on
the first line of context. But as Tim's example shows, it doesn't
necessarily have to. I think it would make more sense to start counting
from the first modified line.
That patch would look something like this:
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index d11dbf9..441ecf5 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -194,7 +194,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
if (xecfg-flags XDL_EMIT_FUNCNAMES) {
get_func_line(xe, xecfg, func_line,
- s1 - 1, funclineprev);
+ xche-i1 - 1, funclineprev);
funclineprev = s1 - 1;
}
if (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,
Note that this breaks a ton of tests. Some of them are just noise (e.g.,
t4042 changes line 2, so line 1 is the top of the context; before, we
would show no hunk header, since we were at the top of the file, but now
we will show line 1). Some of them are improved in the way that this
patch intends (e.g., t4051).
But some I'm not sure of. For instance, the failure in t4018.38 is odd.
I think it's because the pattern it is looking for is a somewhat odd toy
example (it's looking for a line with s in it, so naturally when we
shift the start-point of our search, we are likely to find some other
false positive). But it raises an interesting point: what if the pattern
is just looking for lines in a list, and not an enclosing function?
For example, imagine you have a file a list of items, one per line.
With the old code, you'd get:
diff --git a/old b/new
index f384549..1066a25 100644
--- a/old
+++ b/new
@@ -2,3 +2,3 @@ one
two
-three
+three -- modified
four
So the hunk header is showing you something useful; the element just
above your context. But with my patch, you'd see:
diff --git a/old b/new
index f384549..1066a25 100644
--- a/old
+++ b/new
@@ -2,3 +2,3 @@ two
two
-three
+three -- modified
four
I.e., it shows the element just before the change, which is already in
the context anyway. So it's actually less useful. Although note that the
current behavior is not all that useful, either; it is not really giving
you any information about the change, but rather just showing one extra
line of context.
So I would say that which you would prefer might depend on exactly what
you are diffing. But I would also argue that in any case where the new
code produces a worse result, the hunk header was not all that useful to
begin with.
-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] branch -v: align even when the first column is in UTF-8
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Branch names are usually in ASCII so they are not the problem. The
problem most likely comes from (no branch) translation, which is in
UTF-8 and makes length calculation just wrong.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
So far all git translations are utf-8 compatible. Branch names may
use filesystem encoding, but then packed-refs specifies no encoding.
Anyway branch names should be in utf-8.. at least internally, imo.
I agree with all of the above, but shouldn't you be computing the
maxwidth based on the strwidth in the first place? The use of
maxwidth in strbuf_addf() here clearly wants we know N columns is
sufficient to show all output items, so pad the string to N columns
here. Looking for assignment item.len = xxx in the same file
shows these are computed as byte length, so you are offsetting off
of an incorrectly computed value.
Giving fewer padding bytes when showing a string that will occupy
fewer columns than it has bytes is independently necessary, once we
have the correct maxwidth that is computed in terms of the strwidth,
so this patch is not wrong per-se, but it is incomplete without a
correct maxwidth, no?
builtin/branch.c | 8 +---
1 tập tin đã bị thay đổi, 5 được thêm vào(+), 3 bị xóa(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 0e060f2..7c1ffa8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -17,6 +17,7 @@
#include revision.h
#include string-list.h
#include column.h
+#include utf8.h
static const char * const builtin_branch_usage[] = {
git branch [options] [-r | -a] [--merged | --no-merged],
@@ -490,11 +491,12 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
}
strbuf_addf(name, %s%s, prefix, item-name);
- if (verbose)
+ if (verbose) {
+ int utf8_compensation = strlen(name.buf) -
utf8_strwidth(name.buf);
strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color),
- maxwidth, name.buf,
+ maxwidth + utf8_compensation, name.buf,
branch_get_color(BRANCH_COLOR_RESET));
- else
+ } else
strbuf_addf(out, %c %s%s%s, c, branch_get_color(color),
name.buf, branch_get_color(BRANCH_COLOR_RESET));
--
To unsubscribe from this list: send the line 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] Ignore trailing slash in mkdir() on platforms that can't deal with this
As the compat/mkdir.c file includes git-compat-util.h and expects the declaration of the new function to be found in it, it does not make any sense to have this as two patches. I'll squash them into one for now, but it would have been even more complete to have an update to the Makefile to actually compile these files in the same change. These things go together. The other itimer set shares the same issue. I've queued mkdir and itimer series as one patch each; please check the result in 'pu' after I push it out. 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] contrib: GnomeKeyring support + generic helper implementation
Philipp A. Hartmann p...@qo.cx writes: All, the following patch series proposes enhancements to the credential helper implementations in the contrib section. The detailed development history can be found at GitHub [1]. The first patch adds a GnomeKeyring credential backend. The GnomeKeyring specific parts are based on the work by John Szakmeister, who wrote the helper originally for the initial, unpublished version of the credential helper protocol. The second patch adds a generic implementation of a credential helper based on a simplified credential API and common helper functions. Helpers based on this implementation do not need to worry about the credential protocol and only need to implement callback functions for the different credential operations. The third and fourth patches port the existing helpers to this generic implementation. Looking forward to your thoughts. It struck me somewhat odd to see a new one added as the first step in the series, and then the generic, the third patch only to lose code from the first one, and then use the same code reduction of existing one with the last one in the series. A natural progression would have been the introduction of generic infrastructure (including the tiny bit this series had to add as part of 4/4) as the first patch, update existing OSX one to it as the second, and then build a new Gnome one on the infrastructure as the third and final patch; that way we have to review less code, too ;-). I gave it a cursory look; other than getting distracted by inconsistent coding styles here and there, the patches seemed reasonably clean and well thought out. 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 1/2] Ignore trailing slash in mkdir() on platforms that can't deal with this
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, August 24, 2012 7:44 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH 1/2] Ignore trailing slash in mkdir() on platforms that can't deal with this As the compat/mkdir.c file includes git-compat-util.h and expects the declaration of the new function to be found in it, it does not make any sense to have this as two patches. I'll squash them into one for now, but it would have been even more complete to have an update to the Makefile to actually compile these files in the same change. These things go together. A changed Makefile is in the pipeline, but right now it is pretty NonStop specific, while I tried to keep compat/itimer.cm compat/mkdir.c and git-compat-util.h as NonStop independent as I possible could. That Makefile also depends on the outcome of the discussion about my NonStop specific changed in git-compat.util.h The other itimer set shares the same issue. I've queued mkdir and itimer series as one patch each; please check the result in 'pu' after I push it out. Thanks. What does 'pu' stand for? Bye, Jojo -- To unsubscribe from this list: send the line 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] Prefer sysconf(_SC_OPEN_MAX) over getrlimit(RLIMIT_NOFILE,...)
From: Junio C Hamano [mailto:gits...@pobox.com]
Sent: Friday, August 24, 2012 6:44 PM
To: Joachim Schmitz
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2] Prefer sysconf(_SC_OPEN_MAX) over
getrlimit(RLIMIT_NOFILE,...)
Joachim Schmitz j...@schmitz-digital.de writes:
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
As discussed now as a small helper function rather than #ifdef/#endif in
the primary flow of the code.
And hopefully without having screwed up whitespace and line breaks
The formatting looks fine.
Perhaps I am being overly paranoid, but I would prefer not to change
things for people who have been using getrlimit(). For them, if
they also have sysconf(_SC_OPEN_MAX), your code _ought to_ work, but
if it does not work for whatever reason (perhaps some platforms
claim to have both, but getrlimit() works and sysconf(_SC_OPEN_MAX)
is broken), it will given them an unnecessary regression.
Sounds reasonable, so reasonable that I wonder why I didn't have that idea ;-)
So how about doing it this way instead?
-- 8 --
Subject: sha1_file.c: introduce get_max_fd_limit() helper
Not all platforms have getrlimit(), but there are other ways to see
the maximum number of files that a process can have open. If
getrlimit() is unavailable, fall back to sysconf(_SC_OPEN_MAX) if
available, and use OPEN_MAX from limits.h.
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
sha1_file.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git c/sha1_file.c w/sha1_file.c
index af5cfbd..9152974 100644
--- c/sha1_file.c
+++ w/sha1_file.c
@@ -731,6 +731,24 @@ void free_pack_by_name(const char *pack_name)
}
}
+static unsigned int get_max_fd_limit(void)
+{
+#ifdef RLIMIT_NOFILE
+ struct rlimit lim;
+
+ if (getrlimit(RLIMIT_NOFILE, lim))
+ die_errno(cannot get RLIMIT_NOFILE);
+
+ return lim.rlim_cur;
+#elif defined(_SC_OPEN_MAX)
+ return sysconf(_SC_OPEN_MAX);
+#elif defined(OPEN_MAX)
+ return OPEN_MAX;
+#else
+ return 1; /* see the caller ;-) */
+#endif
+}
+
/*
* Do not call this directly as this leaks p-pack_fd on error return;
* call open_packed_git() instead.
@@ -747,13 +765,7 @@ static int open_packed_git_1(struct packed_git *p)
return error(packfile %s index unavailable, p-pack_name);
if (!pack_max_fds) {
- struct rlimit lim;
- unsigned int max_fds;
-
- if (getrlimit(RLIMIT_NOFILE, lim))
- die_errno(cannot get RLIMIT_NOFILE);
-
- max_fds = lim.rlim_cur;
+ unsigned int max_fds = get_max_fd_limit();
/* Save 3 for stdin/stdout/stderr, 22 for work */
if (25 max_fds)
Looks good to me.
Stupid newbie question: how would I revert my commit to my clone, to then add
(and test) this one?
Bye, Jojo
--
To unsubscribe from this list: send the line 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] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, August 24, 2012 6:07 PM To: Joachim Schmitz Cc: git@vger.kernel.org; Erik Faye-Lund Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz j...@schmitz-digital.de writes: There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. I think that is the most sensible way to go, because poll.[ch] (1) has proven itself to be useful outside the context of win32, and (2) the code is coming from gnulib anyway. I thought I already suggested going that route in my previous review, but perhaps I forgot to do so. No, I believe you did, but I had forgotten about it. Could/should that be a 2nd patch? Or a v3 of this one? Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] Support for HP NonStop
Hi folks On top of the patches Ive submitted so far, which were needed for HP NonStop, but possibly useful for other platforms too, here is one that is at least in parts NonStop specific diff --git a/git-compat-util.h b/git-compat-util.h index a047221..d6a142a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -74,7 +74,8 @@ # define _XOPEN_SOURCE 500 # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ - !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) + !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ + !defined(__TANDEM) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 fo #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif +#ifdef __TANDEM /* or HAVE_STRINGS_H ? */ +#include strings.h /* for strcasecmp() */ +#endif #include errno.h #include limits.h #include sys/param.h @@ -141,6 +145,10 @@ #else #include stdint.h #endif +#ifdef __TANDEM /* or NO_INTPTR_T resp. NO_UINTPTR_T? */ +typedef int intptr_t; +typedef unsigned int uintptr_t; +#endif #if defined(__CYGWIN__) #undef _XOPEN_SOURCE #include grp.h The 1st hunk avoids a is already defined with a different value warning, and I believe this is the only and right way to fix this, but on the 2nd and 3rd hunk Id need advice on how to properly add those. The #ifdef __TANDEM #endif works fine for me, but doesnt seem 100% clean to me. In the comment I mention alternatives. strcasecamp() is declared in strings.h as per C99/POSIX, and in C99 mode a prototype has to be seen by the compiler. intptr_t and uintprt_t seem to be optional in C99 and are not provided for NonStop What do you think? Bye, Jojo -- To unsubscribe from this list: send the line 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] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Joachim Schmitz j...@schmitz-digital.de writes: Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Seeing other existing generic wrappers directly under compat/, e.g. fopen.c, mkdtemp.c, doing so, I would say why not. Windows folks (I see Erik is already CC'ed, which is good ;-), please work with Joachim to make sure such a move won't break your builds. I believe that it should just be the matter of updating a couple of paths in the top-level Makefile. 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: [RFC] Support for HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: Hi folks On top of the patches I’ve submitted so far, which were needed for HP NonStop, but possibly useful for other platforms too, here is one that is at least in parts NonStop specific diff --git a/git-compat-util.h b/git-compat-util.h index a047221..d6a142a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -74,7 +74,8 @@ # define _XOPEN_SOURCE 500 # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ - !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) + !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ + !defined(__TANDEM) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 fo #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif +#ifdef __TANDEM /* or HAVE_STRINGS_H ? */ +#include strings.h /* for strcasecmp() */ +#endif #include errno.h #include limits.h #include sys/param.h Yeah, it appears that glibc headers have strcasecmp() and friends in the string.h and that was why majority of us were fine without including strings.h. A cursory look of /usr/include/strings.h on a GNU system suggests that it is safe to include strings.h after we incude string.h on that platform. I think it is OK to leave it __TANDEM /* or HAVE_STRINGS_H? */ for now and let the next person who wants to port us to a platform that needs this inclusion turn it to HAVE_STRINGS_H. Alternatively, we bite the bullet now and include strings.h on any platform that has the header file and see if anybody complains (that reminds me; I at least should get one flavor of BSD build environment for this kind of thing myself). @@ -141,6 +145,10 @@ #else #include stdint.h #endif +#ifdef __TANDEM /* or NO_INTPTR_T resp. NO_UINTPTR_T? */ +typedef int intptr_t; +typedef unsigned int uintptr_t; +#endif A bit wider context for this hunk is #ifndef NO_INTTYPES_H #include inttypes.h #else #include stdint.h #endif So we have been assuming that stdint.h has intptr_t but __TANDEM apparently doesn't. POSIX requires intptr_t and uintptr_t to be declared for systems conforming to XSI, but otherwise these are optional (in other words, some XSI non-conforming platforms may have them in stdint.h), so it would not help to check _XOPEN_UNIX to see if the system is XSI X-. We would need NO_INTPTR_T as you hinted above, perhaps like this. #ifndef NO_INTTYPES_H #include inttypes.h #else #include stdint.h #endif #ifdef NO_INTPTR_T typedef int intptr_t; typedef unsigned int uintptr_t; #endif By the way, is int wide enough, or should they be long? -- To unsubscribe from this list: send the line 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 no longer prompting for password
Hi List, A recent update to git 1.7.12 from 1.7.3.5 seems to have changed something - trying to push to a smart http backend no longer prompts for a password and hence fails the server auth. The server is currently running git 1.7.9 behind apache 2.4.3 with an almost verbatim copy of the apache config from the git-http-backend manpage. Backtracking through the versions I've skipped and this doesn't seem to be a new problem, client side up to 1.7.7.7 works, 1.7.8 onwards don't. Server side version doesn't seem to make a difference. user@fubar01:~/test# git --version git version 1.7.7.7 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master Password: type the password in and the push is successful user@fubar01:~/test# git --version git version 1.7.8 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master --verbose Pushing to http://ipaton@10.0.0.1/git/test.git Counting objects: 6, done. Delta compression using up to 8 threads. Compressing objects: 100% (3/3), done. Writing objects: 100% (5/5), 491 bytes, done. Total 5 (delta 0), reused 0 (delta 0) error: RPC failed; result=22, HTTP code = 401 fatal: The remote end hung up unexpectedly fatal: The remote end hung up unexpectedly Watching the connection with wireshark shows that it does appear to try to authenticate with the correct username, but without a password. Not surprising since it doesn't ask for one.. googling for git and password just seems to give results where people want it to stop asking for a password, which is the oppsite of what I want! Looking at changelogs for 1.7.8 and I'm not really seeing anything that says I need to do something different. Any help or pointers appreciated. Thanks, Iain -- To unsubscribe from this list: send the line 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 no longer prompting for password
On Fri, Aug 24, 2012 at 09:19:28PM +0100, Iain Paton wrote: A recent update to git 1.7.12 from 1.7.3.5 seems to have changed something - trying to push to a smart http backend no longer prompts for a password and hence fails the server auth. [...] Backtracking through the versions I've skipped and this doesn't seem to be a new problem, client side up to 1.7.7.7 works, 1.7.8 onwards don't. Server side version doesn't seem to make a difference. There was some work in v1.7.8 to avoid prompting for a password when it is not necessary; I suspect this is a fallout of that. You could try bisecting the bug. My guess is that you will end up at commit 986bbc0 (http: don't always prompt for password, 2011-11-04). user@fubar01:~/test# git --version git version 1.7.7.7 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master Password: As per the discussion in 986bbc0, this is actually prompting you before git makes any request. Whereas here: user@fubar01:~/test# git --version git version 1.7.8 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master --verbose We should get an HTTP 401 from the server, then prompt, then retry. What's weird is that it sort of works: Pushing to http://ipaton@10.0.0.1/git/test.git Counting objects: 6, done. Delta compression using up to 8 threads. Compressing objects: 100% (3/3), done. Writing objects: 100% (5/5), 491 bytes, done. Total 5 (delta 0), reused 0 (delta 0) error: RPC failed; result=22, HTTP code = 401 fatal: The remote end hung up unexpectedly fatal: The remote end hung up unexpectedly It's like the initial http requests do not get a 401, and the push proceeds, and then some later request causes a 401 when we do not expect it. Which is doubly odd, since we should also be able to handle that case (the first 401 we get should cause us to ask for a password). Can you show us the result of running with GIT_CURL_VERBOSE=1? I'd really like to see which requests are being made with and without authentication. Looking at changelogs for 1.7.8 and I'm not really seeing anything that says I need to do something different. No, you shouldn't need to do anything different. I'd suspect the weirdness you are seeing is from a credential helper trying to supply a blank password, except that you would have to have configured one manually for it to run (I assume you are not on a shared machine where somebody might have tweaked /etc/gitconfig or anything like that). -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] contrib: GnomeKeyring support + generic helper implementation
On Fri, Aug 24, 2012 at 11:15:36AM -0700, Junio C Hamano wrote: The third and fourth patches port the existing helpers to this generic implementation. It struck me somewhat odd to see a new one added as the first step in the series, and then the generic, the third patch only to lose code from the first one, and then use the same code reduction of existing one with the last one in the series. A natural progression would have been the introduction of generic infrastructure (including the tiny bit this series had to add as part of 4/4) as the first patch, update existing OSX one to it as the second, and then build a new Gnome one on the infrastructure as the third and final patch; that way we have to review less code, too ;-). I think this is partially because I talked with Phillipp off-list and was somewhat unsure how much we wanted to factor out of the helpers. My initial thought was that the protocol would be sufficiently simple that helpers would just be stand-alone and not need to share code with each other. Then the generic bits would not have to worry about being cross-platform compatible. However, the shared bits are simple enough that maybe that is not a concern. An interesting test would be to add a 5/4 porting Erik's win32 credential helper, since that is the platform least like our other ones. So I am OK with this series, but I am also OK with leaving it at patch 1, and just keeping the implementations separate. -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: [RFC] Support for HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: Reminds me of a related issue: in compat/fnmatch/fnmatch.c there is this: #if HAVE_STRING_H || defined _LIBC # include string.h #else # include strings.h #endif There's no place where HAVE_STRING_H get set This looks wrong to me,... This is because it is a borrowed file from glibc, and we try to minimize changes to such a file. If you need HAVE_STRING_H to force inclusion of string.h on your platform, doing this: COMPAT_CFLAGS += -DHAVE_STRING_H=1 # needed in compat/fnmatch/fnmatch.c is perfectly the right thing to do. Do platforms exist without string.h? Maybe fnmatch.c should look like this instead? We try to minimize changes to such a file we borrow from upstream; especially we do not do so lightly when we have to ask do platforms exist? Of course, they do---otherwise glibc folks wouldn't have written such a conditional. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug: when applying binary diffs, -p0 is ignored
I found a bug in git's handling of binary diff segments.
When applying binary diffs using -p0, the prefix (or --strip) argument
is ignored.
For example, try this:
git apply -p0 --binary 'EOF'
diff --git a/init.tar.gz a/init.tar.gz
new file mode 100644
index ..
386b94f511a17a8a3d62eb6cec14694cb9b9b51d
GIT binary patch
literal 118
zcmb2|=3qGT-8_JS`RzGFp(Y0bmIKvX{ySXzs`-OhSfm*K#a}SG%CY!eN_LjnjGuP-
zFHV7m)|X1{OzyyaqIde8O2kg^q0P?b0-p;muDS-IC{I=+*S-L~Mn{_^zwEo=Tu
UurRDep|-nAGgFaXfQAU0L+LoA^-pY
literal 0
HcmV?d1
EOF
It will create init.tar.gz, but in the root of the repository, not in a/
(Sorry if my mailer wraps the long index line)
cheers,
Colin McCabe
--
To unsubscribe from this list: send the line 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/RFC] git svn: don't introduce new paragraph for git-svn-id
Junio C Hamano wrote: Eric Wong normalper...@yhbt.net writes: I think having svn in svn.trimsvnlog twice is redundant and not ideal. Perhaps just --trim-log and svn.trimlog? Do we ever want to trim our log when relaying the Git commits back to subversion? Having svn in trimsvnlog makes it clear that the logs from subversion side is getting trimmed. `git commit' already trims the messages (except for removing the leading whitespaces from the first non-whitespace-only line) and git svn doesn't change that. The new option affects the way the messages are imported from svn to git, with one exception when the --add-author-from option of dcommit is used (in which case it may skip adding an extra new line character before the `From: ' line). For that reason --trim-svn-log might be a better name. Regards, robert -- To unsubscribe from this list: send the line 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/RFC] git svn: optionally trim imported log messages
Junio C Hamano writes:
Hi,
Junio, for some reason I don't get mails from you, I've just discovered
your e-mails on gmane news list. Anyway many thanks for your comments,
I'll fix them and send updated patch next week.
+When committing to svn from git (as part of 'commit-diff', 'set-tree'
+or 'dcommit') and '--add-author-from' is in effect, a new line character
+is not added before the `From: ` line if the log message ends with
+a pseudo-headers section.
I think it would be saner to call them trailers to avoid
confusion.
Thanks, I haven't got any idea how to call them, especially because
existing git documentation refers to them just by using the word `line',
e.g.:
git-am.txt: Add a `Signed-off-by:` line to the commit message,
git-cherry-pick.txt:Add Signed-off-by line at the end of the
(The trailer keyword appears once in SubmittingPatches and - in a bit
different meaning in technical/pack-format.txt)
I've seen S-o-b, Cc and Change-Id there, but does anybody actually
put From: there?
Yes, `git svn dcommit --add-author-from' adds such a trailer to the svn
log message, and then resets or rebases the git index against svn, so
that the message with the trailer appears in git.
There needs an explanation to the reader why this is an optional
feature.
OK, I'll add some explanation. Basically it is optional, per Eric
request, for backward compatibility to make it possible to work on a
centralized clone of svn repository by people using both old and new
versions of git svn.
The prerequisite mechanism is to allow some tests in an environment
where they cannot be run (e.g. no SVN available on the platform);
any code that uses set_prereq unconditionally looks extremely
strange. It is OK while the patch is in RFC stage under active
debugging, but they would want to go away before the polishing is
done.
OK, I used it merely for checking that the tests work on older version
of git svn, and I didn't break the backward compatibility by accident.
Will be dropped from next version of the patch.
What does En-El stand for? We often see (but not in Git where we
prefer LF for that purpose)
NL='
' ;# newline
and using $NL to mean empty may be misleading to the readers.
NL means newline. The new line characters implicitly added after each
commit message line, that's why the value is empty. But, yes, this can
be misleading. I'd prefer to keep it short, so would EL (i.e.
`empty-line') be an acceptable name?
+N=$((N + 1))
Sorry, it was a typo, I meant to use $(($N + 1)). Thanks for catching this.
next_N () {
N=$(($N + 1))
...
}
(the above also has two style fixes).
Just to be sure: shall the `...' line start a new level of indentation
or is it a typo? (I guess that the two style fixes are just after the
function name.)
Thanks a lot,
robert
--
To unsubscribe from this list: send the line 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/RFC] git svn: optionally trim imported log messages
Robert Luberda rob...@debian.org writes:
I think it would be saner to call them trailers to avoid
confusion.
Thanks, I haven't got any idea how to call them, especially because
existing git documentation refers to them just by using the word `line',
e.g.:
git-am.txt: Add a `Signed-off-by:` line to the commit message,
git-cherry-pick.txt:Add Signed-off-by line at the end of the
Then line is fine; they never come before the body, and are
certainly not headers.
There needs an explanation to the reader why this is an optional
feature.
OK, I'll add some explanation. Basically it is optional, per Eric
request, for backward compatibility to make it possible to work on a
centralized clone of svn repository by people using both old and new
versions of git svn.
That matches my recollection. I didn't ask you to explain it to me,
by the way, as I've skimmed the discussion during the review.
I wanted the resulting history and the documentation to explain that
to git-svn users.
NL means newline. The new line characters implicitly added after each
commit message line, that's why the value is empty. But, yes, this can
be misleading. I'd prefer to keep it short, so would EL (i.e.
`empty-line') be an acceptable name?
I'd rather call it $EMPTY; $NL is already obscure, nobody uses $EL.
next_N () {
N=$(($N + 1))
...
}
(the above also has two style fixes).
Just to be sure: shall the `...' line start a new level of indentation
or is it a typo?
It was meant to align with N=, but perhaps HT and quoting
interacted badly or something.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: misleading diff-hunk header
On 08/24/12 11:44, Jeff King wrote:
With the old code, you'd get:
diff --git a/old b/new
index f384549..1066a25 100644
--- a/old
+++ b/new
@@ -2,3 +2,3 @@ one
two
-three
+three -- modified
four
So the hunk header is showing you something useful; the element just
above your context. But with my patch, you'd see:
diff --git a/old b/new
index f384549..1066a25 100644
--- a/old
+++ b/new
@@ -2,3 +2,3 @@ two
two
-three
+three -- modified
four
I.e., it shows the element just before the change, which is already in
the context anyway. So it's actually less useful. Although note that the
current behavior is not all that useful, either; it is not really giving
you any information about the change, but rather just showing one extra
line of context.
So I would say that which you would prefer might depend on exactly what
you are diffing. But I would also argue that in any case where the new
code produces a worse result, the hunk header was not all that useful to
begin with.
If the documented purpose of diff -p (and by proxy
diff.{type}.xfuncname) is to show the name of the *function*
containing the changed lines, and all you have is a list of lines
with no function names, it's pretty arbitrary to call either
behavior worse. :-)
-tkc
--
To unsubscribe from this list: send the line 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 Not a git repository error messages
From: Travis Carden travis.car...@gmail.com
The former messages changed grammatical subject in the middle.
Signed-off-by: Travis Carden travis.car...@gmail.com
---
This is my first attempt at contributing to the Git project.
I'm kind of testing the water with a simple patch to see how
friendly the community is. Thanks!
setup.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 9139bee..0cd88e5 100644
--- a/setup.c
+++ b/setup.c
@@ -599,7 +599,7 @@ static const char *setup_bare_git_dir(char *cwd, int
offset, int len, int *nongi
static const char *setup_nongit(const char *cwd, int *nongit_ok)
{
if (!nongit_ok)
- die(Not a git repository (or any of the parent directories):
%s, DEFAULT_GIT_DIR_ENVIRONMENT);
+ die(Not a git repository (or a descendant of one): %s,
DEFAULT_GIT_DIR_ENVIRONMENT);
if (chdir(cwd))
die_errno(Cannot come back to cwd);
*nongit_ok = 1;
@@ -706,7 +706,7 @@ static const char *setup_git_directory_gently_1(int
*nongit_ok)
return NULL;
}
cwd[offset] = '\0';
- die(Not a git repository (or any parent up to
mount point %s)\n
+ die(Not a git repository (or a descendant of
one up to mount point %s)\n
Stopping at filesystem boundary
(GIT_DISCOVERY_ACROSS_FILESYSTEM not set)., cwd);
}
}
--
1.7.9.5
--
To unsubscribe from this list: send the line 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: misleading diff-hunk header
Tim Chase g...@tim.thechases.com writes:
If the documented purpose of diff -p (and by proxy
diff.{type}.xfuncname) is to show the name of the *function*
containing the changed lines,
Yeah, the documentation is misleading, but I do not offhand think of
a better phrasing. Perhaps you could send in a patch to improve it.
How does GNU manual explain the option?
--
To unsubscribe from this list: send the line 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 06/17] Let fetch_pack() inform caller about number of unique heads
On 08/23/2012 10:54 AM, Jeff King wrote: On Thu, Aug 23, 2012 at 10:10:31AM +0200, mhag...@alum.mit.edu wrote: From: Michael Haggerty mhag...@alum.mit.edu fetch_pack() remotes duplicates from the list (nr_heads, heads), thereby shrinking the list. But previously, the caller was not informed about the shrinkage. This would cause a spurious error message to be emitted by cmd_fetch_pack() if git fetch-pack is called with duplicate refnames. So change the signature of fetch_pack() to accept nr_heads by reference, and if any duplicates were removed then modify it to reflect the number of remaining references. The last test of t5500 inexplicably *required* git fetch-pack to fail when fetching a list of references that contains duplicates; i.e., it insisted on the buggy behavior. So change the test to expect the correct behavior. Eek, yeah, the current behavior is obviously wrong. The remove_duplicates code comes from 310b86d (fetch-pack: do not barf when duplicate re patterns are given, 2006-11-25) and clearly meant for fetch-pack to handle this case gracefully. diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 3cc3346..0d4edcb 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -391,7 +391,7 @@ test_expect_success 'fetch mixed refs from cmdline and stdin' ' test_expect_success 'test duplicate refs from stdin' ' ( cd client -test_must_fail git fetch-pack --stdin --no-progress .. ../input.dup +git fetch-pack --stdin --no-progress .. ../input.dup ) output cut -d -f 2 output | sort actual test_cmp expect actual It's interesting that the output was the same before and after the fix. I guess that is because the error comes at the very end, when we are making sure all of the provided heads have been consumed. git fetch-pack emits information about successfully-received references regardless of whether some requested references were not received. The no such remote ref %s output goes to stderr. So the only difference between before/after fix should be what is written to stderr, whereas the test only looks at stdout. 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: bug: when applying binary diffs, -p0 is ignored
Colin McCabe cmcc...@alumni.cmu.edu writes:
I found a bug in git's handling of binary diff segments.
When applying binary diffs using -p0, the prefix (or --strip) argument
is ignored.
Thanks for a report. An ancient bug well spotted.
Perhaps this will help.
-- 8 --
Subject: apply: compute patch-def_name correctly under -p0
Back when git apply was written, we made sure that the user can
skip more than the default number of path components (i.e. 1) by
giving -pn, but the logic for doing so was built around the
notion of we skip N slashes and stop. This obviously does not
work well when running under -p0 where we do not want to skip any,
but still want to skip SP/HT that separates the pathnames of
preimage and postimage and want to reject absolute pathnames.
Stop using stop_at_slash(), and instead introduce a new helper
skip_tree_prefix() with similar logic but works correctly even for
the -p0 case.
This is an ancient bug, but has been masked for a long time because
most of the patches are text and have other clues to tell us the
name of the preimage and the postimage.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 55 ++-
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git i/builtin/apply.c w/builtin/apply.c
index 3bf71dc..4a511b3 100644
--- i/builtin/apply.c
+++ w/builtin/apply.c
@@ -1095,15 +1095,23 @@ static int gitdiff_unrecognized(const char *line,
struct patch *patch)
return -1;
}
-static const char *stop_at_slash(const char *line, int llen)
+/*
+ * Skip p_value leading components from line; as we do not accept
+ * absolute paths, return NULL in that case.
+ */
+static const char *skip_tree_prefix(const char *line, int llen)
{
- int nslash = p_value;
+ int nslash;
int i;
+ if (!p_value)
+ return (llen line[0] == '/') ? NULL : line;
+
+ nslash = p_value;
for (i = 0; i llen; i++) {
int ch = line[i];
if (ch == '/' --nslash = 0)
- return line[i];
+ return (i == 0) ? NULL : line[i + 1];
}
return NULL;
}
@@ -1133,12 +1141,11 @@ static char *git_header_name(const char *line, int llen)
if (unquote_c_style(first, line, second))
goto free_and_fail1;
- /* advance to the first slash */
- cp = stop_at_slash(first.buf, first.len);
- /* we do not accept absolute paths */
- if (!cp || cp == first.buf)
+ /* strip the a/b prefix including trailing slash */
+ cp = skip_tree_prefix(first.buf, first.len);
+ if (!cp)
goto free_and_fail1;
- strbuf_remove(first, 0, cp + 1 - first.buf);
+ strbuf_remove(first, 0, cp - first.buf);
/*
* second points at one past closing dq of name.
@@ -1152,22 +1159,21 @@ static char *git_header_name(const char *line, int llen)
if (*second == '') {
if (unquote_c_style(sp, second, NULL))
goto free_and_fail1;
- cp = stop_at_slash(sp.buf, sp.len);
- if (!cp || cp == sp.buf)
+ cp = skip_tree_prefix(sp.buf, sp.len);
+ if (!cp)
goto free_and_fail1;
/* They must match, otherwise ignore */
- if (strcmp(cp + 1, first.buf))
+ if (strcmp(cp, first.buf))
goto free_and_fail1;
strbuf_release(sp);
return strbuf_detach(first, NULL);
}
/* unquoted second */
- cp = stop_at_slash(second, line + llen - second);
- if (!cp || cp == second)
+ cp = skip_tree_prefix(second, line + llen - second);
+ if (!cp)
goto free_and_fail1;
- cp++;
- if (line + llen - cp != first.len + 1 ||
+ if (line + llen - cp != first.len ||
memcmp(first.buf, cp, first.len))
goto free_and_fail1;
return strbuf_detach(first, NULL);
@@ -1179,10 +1185,9 @@ static char *git_header_name(const char *line, int llen)
}
/* unquoted first name */
- name = stop_at_slash(line, llen);
- if (!name || name == line)
+ name = skip_tree_prefix(line, llen);
+ if (!name)
return NULL;
- name++;
/*
* since the first name is unquoted, a dq if exists must be
@@ -1196,10 +1201,9 @@ static char *git_header_name(const char *line, int llen)
if (unquote_c_style(sp, second, NULL))
goto free_and_fail2;
-
