Re: [PATCH v2 11/11] ref-filter: introduce objectname_atom_parser()
On Wed, Dec 16, 2015 at 10:30 AM, Karthik Nayak wrote:
> Introduce objectname_atom_parser() which will parse the
> '%(objectname)' atom and store information into the 'used_atom'
> structure based on the modifiers used along with the atom.
>
> Helped-by: Ramsay Jones
> Signed-off-by: Karthik Nayak
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -43,6 +43,7 @@ static struct used_atom {
> enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
> C_SUB } option;
> unsigned int no_lines;
> } contents;
> + enum { O_FULL, O_SHORT } objectname;
> } u;
> } *used_atom;
> @@ -124,6 +125,21 @@ static void contents_atom_parser(struct used_atom *atom)
> +static void objectname_atom_parser(struct used_atom *atom)
> +{
> + const char * buf;
> +
> + if (match_atom_name(atom->str, "objectname", &buf))
> + atom->u.objectname = O_FULL;
> + if (!buf)
> + return;
Let me make sure that I understand this correctly.
make_atom_name("objectname") will return true only for "objectname" or
"objectname:", and will return false for anything else, such as
"objectnamely" or "schmorf". Furthermore, the only way
objectname_atom_parser() can be called is when %(objectname) or
%(objectname:...) is seen, thus match_atom_name() *must* return true
here, which means the above conditional is misleading, suggesting that
it could somehow return false.
And, if match_atom_name() did return false here, then that indicates a
programming error: objectname_atom_parser() somehow got called for
something other than %(objectname) or %(objectname:...). This implies
that the code should instead be structured like this:
if (!match_atom_name(..., "objectname", &buf)
die("BUG: parsing non-'objectname'")
if (!buf)
atom->u.objectname = O_FULL;
else if (!strcmp(buf, "short"))
atom->u.objectname = O_SHORT;
else
die(_("unrecognized %%(objectname) argument: %s"), buf);
However, this can be simplified further by recognizing that, following
this patch series, match_atom_name() is *only* called by these new
parse functions[1], which means that, as a convenience,
match_atom_name() itself could become a void rather than boolean
function and die() if the expected atom name is not found. Thus, the
code would become:
match_atom_name(...);
if (!buf)
...
else if (!strcmp(...))
...
...
By the way, the above commentary isn't specific to this patch and
%(objectname), but is in fact also relevant for all of the preceding
patches which introduce parse functions calling match_atom_name().
More below...
[1]: ...assuming you replace the unnecessary match_atom_name() in
populate_value() with starts_with() as suggested in my patch 7/11
review addendum[2].
[2]: http://article.gmane.org/gmane.comp.version-control.git/282700
> +
> + if (!strcmp(buf, "short"))
> + atom->u.objectname = O_SHORT;
> + else
> + die(_("unrecognized %%(objectname) argument: %s"), buf);
> +}
> +
> @@ -461,15 +477,16 @@ static void *get_obj(const unsigned char *sha1, struct
> object **obj, unsigned lo
> static int grab_objectname(const char *name, const unsigned char *sha1,
> - struct atom_value *v)
> + struct atom_value *v, struct used_atom *atom)
> {
> - if (!strcmp(name, "objectname")) {
> - v->s = xstrdup(sha1_to_hex(sha1));
> - return 1;
> - }
> - if (!strcmp(name, "objectname:short")) {
> - v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
> - return 1;
> + if (starts_with(name, "objectname")) {
> + if (atom->u.objectname == O_SHORT) {
> + v->s = xstrdup(find_unique_abbrev(sha1,
> DEFAULT_ABBREV));
> + return 1;
> + } else if (atom->u.objectname == O_FULL) {
> + v->s = xstrdup(sha1_to_hex(sha1));
> + return 1;
> + }
Since 'objectname' can only ever be O_SHORT or O_FULL wouldn't it be a
programming error if it ever falls through to this point after the
closing brace? Perhaps a die("BUG:...") would be appropriate?
> }
> return 0;
> }
> @@ -493,7 +510,7 @@ static void grab_common_values(struct atom_value *val,
> int deref, struct object
> v->s = xstrfmt("%lu", sz);
> }
> else if (deref)
> - grab_objectname(name, obj->sha1, v);
> + grab_objectname(name, obj->sha1, v, &used_atom[i]);
This patch hunk is somehow corrupt and doesn't apply. Was there some
hand-editing involved, or were some earlier patches regenerated after
this patch was made or something?
> }
> }
>
> @@ -999,7 +1016,7 @@ static void populate_value(struct ref_array_item *ref)
>
Re: [PATCH v2 07/11] ref-filter: introduce align_atom_parser()
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak wrote:
> Introduce align_atom_parser() which will parse an "align" atom and
> store the required alignment position and width in the "use_atom"
> structure for further usage in 'populate_value()'.
>
> Helped-by: Ramsay Jones
> Signed-off-by: Karthik Nayak
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
> +static void align_atom_parser(struct used_atom *atom)
> +{
> + [...]
> + match_atom_name(atom->str, "align", &buf);
> + if (!buf)
> + die(_("expected format: %%(align:,)"));
> + [...]
> +}
> @@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
> v->s = " ";
> continue;
> } else if (match_atom_name(name, "align", &valp)) {
Hmm, align_atom_parser() has already been called before we get here,
right? And it has already invoked match_atom_name() and died if the
argument to "align:" was missing, correct? If so, then this invocation
of match_atom_name() in populate_value() is unnecessary and should be
replaced with a simpler starts_with("align:").
Plus, 'valp' is not used, aside from this unnecessary
match_atom_name(), thus it can be removed as well.
> - struct align *align = &v->u.align;
> - struct strbuf **s, **to_free;
> - int width = -1;
> -
> - if (!valp)
> - die(_("expected format:
> %%(align:,)"));
> -
> - s = to_free = strbuf_split_str_omit_term(valp, ',',
> 0);
> -
> - align->position = ALIGN_LEFT;
> -
> - while (*s) {
> - if (!strtoul_ui(s[0]->buf, 10, (unsigned int
> *)&width))
> - ;
> - else if (!strcmp(s[0]->buf, "left"))
> - align->position = ALIGN_LEFT;
> - else if (!strcmp(s[0]->buf, "right"))
> - align->position = ALIGN_RIGHT;
> - else if (!strcmp(s[0]->buf, "middle"))
> - align->position = ALIGN_MIDDLE;
> - else
> - die(_("improper format entered
> align:%s"), s[0]->buf);
> - s++;
> - }
> -
> - if (width < 0)
> - die(_("positive width expected with the
> %%(align) atom"));
> - align->width = width;
> - strbuf_list_free(to_free);
> + v->u.align = atom->u.align;
> v->handler = align_atom_handler;
> continue;
> } else if (!strcmp(name, "end")) {
> --
> 2.6.4
--
To unsubscribe from this list: send the line "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 02/16] refs: add methods for misc ref operations
Junio C Hamano pobox.com> writes:
>
> David Turner twopensource.com> writes:
>
> > struct ref_be {
> > struct ref_be *next;
> > const char *name;
> > ref_transaction_commit_fn *transaction_commit;
> > +
> > + pack_refs_fn *pack_refs;
> > + peel_ref_fn *peel_ref;
> > + create_symref_fn *create_symref;
> > +
> > + resolve_ref_unsafe_fn *resolve_ref_unsafe;
> > + verify_refname_available_fn *verify_refname_available;
> > + resolve_gitlink_ref_fn *resolve_gitlink_ref;
> > };
>
> This may have been pointed out in the previous reviews by somebody
> else, but I think it is more customary to declare a struct member
> that is a pointer to a customization function without leading '*',
> i.e.
>
> typedef TYPE (*customize_fn)(ARGS);
>
> struct vtable {
> ...
> cutomize_fn fn;
> ...
> };
>
> in our codebase (cf. string_list::cmp, prio_queue::compare).
(LMDB author here, just passing by)
IMO you're making a mistake. You should always typedef the thing itself, not
a pointer to the thing. If you only typedef the pointer, you can only use
the typedef to declare pointers and never the thing itself. If you typedef
the actual thing, you can use the typedef to declare both the thing and
pointer to thing.
It's particularly useful to have function typedefs because later on you can
use the actual typedef to declare instances of that function type in header
files, and guarantee that your function definitions match what they're
intended to match. Otherwise, assigning a bare (*func) to something that
mismatches only generates a warning, instead of an error.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
--
To unsubscribe from this list: send the line "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] Malicously tampering git metadata?
On Tue, Dec 15, 2015 at 10:26:39PM -0500, Santiago Torres wrote: > An example of a malicious commit merge follows: > > 1) The attacker controlling or acting as the upstream server identifies > two branches: one in which the unsuspecting developer is working on, and > another in which a vulnerable piece of code is located. One thing to make clear here: the side branch with the vulnerable code must be a _new_ vulnerability that was not already part of the "main" branch the developer is working on. That is, I do not immediately see a way to resurrect an old vulnerability, because a merge of the old, broken commit would not result in reintroducing it. This is more about "there was experimental junk on branch X, and you tricked some developer into pulling X onto Y, and now Y unexpectedly has the junk on it". And I agree with Stefan that push-certs are the intended defense against this. Of course, in the real world things are much easier. Most projects do not sign commits at all, let alone use push certs. If developers are pulling from a compromised server, then you can simply make up whatever broken commits you want, and there's no way to tell the difference between them and the real commits. -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] Malicously tampering git metadata?
On Thu, Dec 17, 2015 at 08:06:36PM -0500, Santiago Torres wrote: > > This is what push certificates ought to solve. > > The server records all pushes and its signed certificates of pushes > > and by the difference of the > > refs (either in packed refs or as a loose ref) to the push certificate > > this tampering of > > the server can be detected. > > Is there any specification about push certificates? I would like to read > about them, but I don't seem to find documentation anywhere. Are they a > part of git's specification? Try pack-protocol.txt and protocol-capabilities.txt in the Documentation/technical directory of the git.git repo. E.g.: https://github.com/git/git/blob/bdfc6b364a51b19efbacbd46fcef5be41a5db50e/Documentation/technical/pack-protocol.txt#L489 -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: [PATCHv2] submodule: Port resolve_relative_url from shell to C
On Thu, Dec 17, 2015 at 07:55:43PM +0100, Johannes Sixt wrote:
> -static int has_same_dir_prefix(const char *str, const char **out)
> +static int starts_with_dot_slash(const char *str)
> {
> -#ifdef GIT_WINDOWS_NATIVE
> - return skip_prefix(str, "./", out)
> - || skip_prefix(str, ".\\", out);
> -#else
> - return skip_prefix(str, "./", out);
> -#endif
> + return str[0] == '.' && is_dir_sep(str[1]);
> }
>
> -static int has_upper_dir_prefix(const char *str, const char **out)
> +static int starts_with_dot_dot_slash(const char *str)
> {
> -#ifdef GIT_WINDOWS_NATIVE
> - return skip_prefix(str, "../", out)
> - || skip_prefix(str, "..\\", out);
> -#else
> - return skip_prefix(str, "../", out);
> -#endif
> + return str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);
> }
As the set of prefixes you are looking is probably bounded, it may not
be worth generalizing this. But I wondered if something like:
/*
* Like skip_prefix, but consider any "/" in the prefix as a
* directory separator for the platform.
*/
int skip_prefix_fs(const char *str, const char *prefix, const char **out)
{
while (1) {
if (!*prefix) {
*out = str;
return 1;
} else if (*prefix == '/') {
if (!is_dir_sep(*str))
return 0;
} else {
if (*str != *prefix)
return 0;
}
str++;
prefix++;
}
}
...
/* works on all platforms! */
if (skip_prefix_fs(foo, "./", &out))
...
would be helpful. I don't know if there are other opportunities in the
code base that could make use of this. If it's just these two sites,
it's probably not worth it.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/7] xread: poll on non blocking fds
On Thu, Dec 17, 2015 at 12:51:08PM -0800, Stefan Beller wrote: > > poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the > > failure may be temporaly, > > as much as poll() can see. > > But most probably we run out ouf memory > > */ > > Before this patch we would not have asked poll, but had just a continue here, > so I think we need to have it here again no matter of the return code > of the poll. > > If poll determines it is low on memory, this should not make this function > fail, > we can still do as good as we did before by just asking read > repeatedly again, though? > > So I'd be convinced now we'd want to have: > > poll(&pfd, 1, -1); /* this is only buying time > for the fd to deliver data, in case it fails > we don't care but just fall back to old > behavior before this patch with busy spinning*/ > continue; Right. I think that is the only sensible thing, and your comment explains perfectly what is going on. -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: [PATCHv2 2/7] xread: poll on non blocking fds
On Thu, Dec 17, 2015 at 09:42:01PM +0100, Torsten Bögershausen wrote: > > Or do you mean to insert another continue in here? > I was thinking that we run into similar loop as before: > read() returns -1; errno = EAGAIN /* No data to read */ > poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the > failure may be temporaly, > as much as poll() can see. > But most probably we run out ouf memory */ > > So the code would look like this: > >if (!poll(&pfd, 1, -1)) > return -1; That changes the semantics of the function. The poll() is just a convenience to avoid spinning. If it fails, with Stefan's patch[1] the worst case is that we would spin on read() and poll(), instead of actually blocking in the poll(). But if we return on poll() failure, now the caller will see errors from poll() even though they don't know or care that we called poll() in the first place. Consider what would happen with your code if read got EAGAIN and then poll got EINTR. We would report an error, even though the whole point of xread() is to loop on these conditions. -Peff [1] Stefan's patch does have a bug. It forgets to "continue" after calling poll, which means we will block with poll and _then_ exit with -1, instead of restarting the loop. -- To unsubscribe from this list: send the line "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: [PATCHv2 2/7] xread: poll on non blocking fds
On Tue, Dec 15, 2015 at 04:04:07PM -0800, Stefan Beller wrote: > If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK. > As the intent of xread is to read as much as possible either until the > fd is EOF or an actual error occurs, we can ease the feeder of the fd > by not spinning the whole time, but rather wait for it politely by not > busy waiting. I'm not sure this second sentence is true. The point of xread() is to do a single read that makes forward progress (or returns a real error), not to read as much as we can. The rationale seems more to me like: The point of xread() is to perform read() until we either get some data, or encounter a "real" error. We do not count EINTR or EAGAIN as "real" errors, as a reader trying to make forward progress would generally just repeat the read, so we loop for them as a convenience. In the case of EINTR, trying the read() again immediately is fine; a signal interrupted us, but there is no reason to think the read() would not succeed if we tried it again. For EAGAIN, however, we know that the fd must have set O_NONBLOCK. In this case reading again immediately is not likely to produce results, and we will chew up CPU spinning on read() calls. Instead, let's insert a poll() to block until we actually get data. I also don't know that this patch necessarily needs to be part of the parallel-fetch series anymore. We are not setting O_NONBLOCK ourselves, and I don't think the correctness of the new strbuf_read_once() is impacted by this. I.e., I think this optimization applies equally well to the hundreds of existing xread() calls, should they unexpectedly be fed an O_NONBLOCK descriptor. That said, I do not mind it here, but I think it could be its own separate topic if you wanted to reduce the complexity of the parallel-fetch topic. > We should not care if the call to poll failed, as we're in an infinite > loop and can only get out with the correct read(). You might want to expand on "correct" here. I think you mean "can only get out with a read() that returns data or a "real" error" (I am reusing the terminology I introduced above; I am fine with other terminology, depending on how you word the rest of it). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RefTree: Alternate ref backend
On Thu, Dec 17, 2015 at 02:28:01PM -0800, Shawn Pearce wrote: > On Thu, Dec 17, 2015 at 2:10 PM, Jeff King wrote: > > On Thu, Dec 17, 2015 at 01:02:50PM -0800, Shawn Pearce wrote: > > > >> I started playing around with the idea of storing references directly > >> in Git. Exploiting the GITLINK tree entry, we can associate a name to > >> any SHA-1. > > > > Gitlink entries don't imply reachability, though. I guess that doesn't > > matter if your ref backend says "no, really, these are the ref tips, and > > they are reachable". > > Exactly. This works with existing JGit because it swaps out the ref > backend. When GC tries to enumerate the roots (current refs), it gets > these through the ref backend by scanning the tree recursively. The > packer itself doesn't care where those roots came from. > > Same would be true for any other pluggable ref backend in git-core. GC > has to ask the ref backend, and then trust its reply. How/where that > ref backend tracks that is an implementation detail. > > > But you could not push the whole thing up to > > another server and expect it to hold the whole graph. > > Correct, pushing this to another repository doesn't transmit the > graph. If the other repository also used this for its refs backend, > its now corrupt and confused out of its mind. Just like copying the > packed-refs file with scp. Don't do that. :) > > > Which is not strictly necessary, but to me seems like the real advantage > > of using git objects versus some other system. > > One advantage is you can edit HEAD symref remotely. Commit a different > symlink value and push. :) > > I want to say more, but I'm going to hold back right now. There's more > going on in my head than just this. > > > Of course, the lack of reachability has advantages, too. You can > > drop commits pointed to by old reflogs without rewriting the ref > > history. > > Yes. Related thread: "Allowing weak references to blobs and strong references to commits" http://marc.info/?l=git&m=142779648816577&w=2 Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-C option with pull doesn't seem to be respected in aliases in git 2.6.4.
I have git project checked out at ~/llvm. Inside of there, inside of a “tools”
directory, I have another project checked out as “lldb”:
~/llvm/tools/lldb
I wrote an alias which would help me update all my projects:
all = !find . -type d -name .git | sed 's:/.git::' | xargs -I{} -t git
-C {} $1 && :
This would allow me to be inside of ~/llvm and type "git all pull" and get all
my projects updated.
It seems that at some point this broke. If try to use this alias under git
2.6.4, it only updates the llvm project.
The interesting thing is that if I pass fetch, instead of pull: "git all
fetch", then it seems to work correctly.
Here’s the GIT_TRACE output for “git all pull” when in ~/llvm:
[llvm (master)]$ GIT_TRACE=1 git all pull
17:28:43.938363 git.c:558 trace: exec: 'git-all' 'pull'
17:28:43.938813 run-command.c:335 trace: run_command: 'git-all' 'pull'
17:28:43.940167 run-command.c:335 trace: run_command: 'find . -type d
-name .git | sed '\''s:/.git::'\'' | xargs -I{} -t git -C {} $1 && :' 'pull'
17:28:43.940665 run-command.c:195 trace: exec: '/bin/sh' '-c' 'find .
-type d -name .git | sed '\''s:/.git::'\'' | xargs -I{} -t git -C {} $1 && :
"$@"' 'find . -type d -name .git | sed '\''s:/.git::'\'' | xargs -I{} -t git -C
{} $1 && :' 'pull'
git -C . pull
17:28:44.191670 git.c:558 trace: exec: 'git-pull'
17:28:44.192253 run-command.c:335 trace: run_command: 'git-pull'
17:28:44.197094 git.c:348 trace: built-in: git 'rev-parse'
'--parseopt' '--stuck-long' '--'
17:28:44.203237 git.c:348 trace: built-in: git 'rev-parse'
'--git-dir'
17:28:44.207297 git.c:348 trace: built-in: git 'rev-parse'
'--git-path' 'objects'
17:28:44.210819 git.c:348 trace: built-in: git 'rev-parse'
'--is-bare-repository'
17:28:44.213666 git.c:348 trace: built-in: git 'rev-parse'
'--show-toplevel'
17:28:44.216954 git.c:348 trace: built-in: git 'ls-files' '-u'
17:28:44.227604 git.c:348 trace: built-in: git 'symbolic-ref'
'-q' 'HEAD'
17:28:44.233503 git.c:348 trace: built-in: git 'config'
'branch.master.rebase'
17:28:44.244334 git.c:348 trace: built-in: git 'config' 'pull.ff'
17:28:44.247510 git.c:348 trace: built-in: git 'rev-parse' '-q'
'--verify' 'HEAD'
17:28:44.250365 git.c:348 trace: built-in: git 'rev-parse'
'--verify' 'HEAD'
17:28:44.253328 git.c:348 trace: built-in: git 'update-index'
'-q' '--ignore-submodules' '--refresh'
17:28:44.282912 git.c:348 trace: built-in: git 'diff-files'
'--quiet' '--ignore-submodules'
17:28:44.317433 git.c:348 trace: built-in: git 'diff-index'
'--cached' '--quiet' '--ignore-submodules' 'HEAD' '--'
17:28:44.332900 git.c:348 trace: built-in: git 'rev-parse' '-q'
'--git-dir'
17:28:44.366055 git.c:348 trace: built-in: git 'rev-parse' '-q'
'--verify' 'HEAD'
17:28:44.369245 git.c:348 trace: built-in: git 'fetch'
'--update-head-ok'
17:28:44.380626 run-command.c:335 trace: run_command: 'git-remote-http'
'origin' 'http://llvm.org/git/llvm.git'
17:28:44.575908 run-command.c:335 trace: run_command: 'rev-list'
'--objects' '--stdin' '--not' '--all' '--quiet'
17:28:44.583303 run-command.c:335 trace: run_command: 'rev-list'
'--objects' '--stdin' '--not' '--all'
17:28:44.583756 exec_cmd.c:178 trace: exec: 'git' 'rev-list'
'--objects' '--stdin' '--not' '--all'
17:28:44.585457 git.c:348 trace: built-in: git 'rev-list'
'--objects' '--stdin' '--not' '--all'
17:28:44.591116 run-command.c:335 trace: run_command: 'gc' '--auto'
17:28:44.591576 exec_cmd.c:178 trace: exec: 'git' 'gc' '--auto'
17:28:44.593252 git.c:348 trace: built-in: git 'gc' '--auto'
17:28:44.597284 git.c:348 trace: built-in: git 'rev-parse' '-q'
'--verify' 'HEAD'
17:28:44.604021 git.c:348 trace: built-in: git 'show-branch'
'--merge-base' 'refs/heads/master' 'ce00641ac5f4ca2a073b732a75169037a2d9bef0'
'ce00641ac5f4ca2a073b732a75169037a2d9bef0'
17:28:44.610870 git.c:348 trace: built-in: git 'rev-parse'
'--parseopt' '--stuck-long' '--' '--onto'
'ce00641ac5f4ca2a073b732a75169037a2d9bef0'
'ce00641ac5f4ca2a073b732a75169037a2d9bef0'
17:28:44.617283 git.c:348 trace: built-in: git 'rev-parse'
'--git-dir'
17:28:44.621510 git.c:348 trace: built-in: git 'rev-parse'
'--git-path' 'objects'
17:28:44.626588 git.c:348 trace: built-in: git 'rev-parse'
'--is-bare-repository'
17:28:44.629515 git.c:348 trace: built-in: git 'rev-parse'
'--show-toplevel'
17:28:44.633673 git.c:348 trace: built-in: git 'config' '--bool'
'rebase.stat'
17:28:44.636888 git.c:348 trace: built-in: git 'config' '--bool'
'rebase.autostash'
17:28:44.640171 git.c:348 tra
Re: [RFC] Malicously tampering git metadata?
Hi Stefan, thanks for the insight. > This is what push certificates ought to solve. > The server records all pushes and its signed certificates of pushes > and by the difference of the > refs (either in packed refs or as a loose ref) to the push certificate > this tampering of > the server can be detected. Is there any specification about push certificates? I would like to read about them, but I don't seem to find documentation anywhere. Are they a part of git's specification? > > The push certs can however not be obtained via Git itself (they are > just stored on the > server for now for later inspection or similar), because to be really > sure the client would > need to learn about these push certificates out of band. I was thinking that an in-band solution could be integrated as long as we assume a compromise would result in an complete (unreconcilable) fork attack; fork attacks aren't subtle and could be detected easily. > > The model there would be: > * A vulnerable piece of software exists. > * It get's fixed (and the fix is pushed with a signed push) > * the MITM server doesn't show the fix (show the code from before fix) nor > the push certificate thereof > * client still pulls vulnerable code Yes, this is a possible attack vector. However, a server could also present a branch pointer as different (e.g., point an experimental branch to an unsigned v1.1 tag). This has other implications, as the code is pushed/pulled from upstream, it just goes somewhere different. > > This model shows the distribution of push certs via the server itself may not > be > optimal. Yes, it might not be optimal, but it could provide protection against the attack I just described, for more complex attacks might not be so subtle. Adding to this, developers likely coordinate their efforts through other means (sic), so the lack of a push certificate (withheld by a server) could raise some yellow flags. We've made a proof of concept of such tool (in-bandh push certificates), and would like to share the basic design of it here. However, it follows our threat model: a compromised server that can't introduce malicious code (thanks to commit signing), but can modify branch pointers and other unsigned metadata to alter the repository's state. > > Thanks for researching on Git, Thanks for working in such a great tool :) -Santiago > Stefan -- To unsubscribe from this list: send the line "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 v3] git-svn: add support for prefixed globs in config
Victor Leschuk wrote: > --- /dev/null > +++ b/t/t9168-git-svn-prefixed-glob.sh > @@ -0,0 +1,136 @@ > +#!/bin/sh > +test_description='git svn globbing refspecs with prefixed globs' > +. ./lib-git-svn.sh > + > +cat >expect.end < +the end > +hi > +start a new branch > +initial > +EOF > + > +test_expect_success 'test refspec prefixed globbing' ' > +echo try to try >expect.two && > +echo nothing to see here >>expect.two && > +cat expect.end >>expect.two What happens when the cat fails? Should probably be folded into the subsequent test case. (I used to write tests with "set -e", but it's not descriptive as far as failure goes). > +test_expect_success 'test left-hand-side only prefixed globbing' ' > + git config --add svn-remote.two.url "$svnrepo" && > +echo "Only one set of wildcard directories" \ > + "(e.g. '*' or '*/*/*') is supported: 'branches/b_*/t/*'" >expect.four && > +echo "" >>expect.four Same with echo... > +test_expect_success 'test disallow prefixed multi-globs' ' -- To unsubscribe from this list: send the line "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 v3 4/4] git gui: allow for a long recentrepo list
From: "Junio C Hamano"
Philip Oakley writes:
The gui.recentrepo list may be longer than the maxrecent setting.
Use the actual length determined earlier, now saved as $lenrecent.
In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outside of the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.
The list length will be trimmed to $maxrecent after the user selects
a repo to open.
Signed-off-by: Philip Oakley
---
V2:
word usage corrected.
Eric's comments $gmane/282432
V3:
Updated list length measure following Junio's comments $gmane/282669
Replaces the previous V2 Patch 4/4.
---
git-gui/lib/choose_repository.tcl | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-gui/lib/choose_repository.tcl
b/git-gui/lib/choose_repository.tcl
index ad7a888..b4cc7dd 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -134,7 +134,8 @@ constructor pick {} {
$opts conf -state disabled
set sorted_recent [_get_recentrepos]
- if {[llength $sorted_recent] > 0} {
+ set lenrecent [llength $sorted_recent]
+ if {$lenrecent > 0} {
if {$m_repo ne {}} {
$m_repo add separator
$m_repo add command \
@@ -153,7 +154,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
- -height $maxrecent
+ -height $lenrecent
$w_recentlist tag conf link \
-foreground blue \
-underline 1
Natural questions that come to mind after seeing the above are like
these:
- Does maxrecent still get used after this change?
Yes, essentially as you describe
- Does the remainder of the code get upset if -height is set to too
small (lenrecent might turn out to be 1, for example)?
No it's OK, the code has at least checked it is positive..
And grepping for maxrecent shows there is _append_recentrepos that
does this:
It does three things:
- removes the current (selected) repo from the list of past repos (and
removes any duplicates)
- places the current repo at the end
- trims the list to back to max repos.
Nominally, it can't do anything about non-selected valid repos which may be
listed in the config twice because git config won't selectively remove just
one of two identical key/values, however in Patch 3 I do remove both older &
newer entries and re-read the config (some may consider that a small
regression).
It's not easy working around the contrary assumptions of the gui.recentrepo
list (once only) and the reality of the .gitconfig (duplicates possible) and
'git config's --unset/--unset-all limitation.
Exactly how those extra entries get there is an open question, but it does
happen (so say's google, as well as the user report, I've seen it myself ;-)
proc _append_recentrepos {path} {
set path [file normalize $path]
set recent [get_config gui.recentrepo]
...
if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
set maxrecent 10
}
while {[llength $recent] > $maxrecent} {
_unset_recentrepo [lindex $recent 0]
set recent [lrange $recent 1 end]
}
}
The last while loop looks to me like truncating the overlong recent
list down to maxrecent.
Yes
Perhaps a similar logic needs to be at the
end of _get_recentrepos?
The logic here is to show what is in the config to the user, *before* we
trim the list, hence we wait until the user has selected a repo to open and
do the purging in _append_recentrepos.
As an aside, there's also a long standing 'feature' that if one works both
on and off network, then network repo paths get trimmed as soon as the user
starts the gui while off-net (or the network goes down;-) because those
recent repo paths become 'invalid' and are purged, but let's not go there.
--
Philip
--
To unsubscribe from this list: send the line "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 issue report : issue with capital letter in folder name
Le 2015-12-17 13:29, Stefan Beller a écrit : On Thu, Dec 17, 2015 at 7:45 AM, PFDuc wrote: Hello, first of all thank you for developping git ! I had an issue with a capital block in the folder name inside my git repo. The folder in my local was named "Display" and the one at origin was named "display" resulting in error when importing python code from this folder for users who got the repo from origin. By any chance, were different operating systems or file systems involved in creation of this problem? It might be the case, a mix between windows7, ubuntu and macOS There are file systems which care about the capitalization, and others don't. So if you have a file system which doesn't care about capitalization of the folder/file name, you can use a different capitalization and it still works. If you take the code to another system then, which is a bit more careful there are problems of course. The main question which remains, is how is Git involved? i.e. would it also happen if you just transfer a tarball? Did Git itself break anything? I don't think git broke anything it is just that the folder name was not being changed to the one with a capital letter when I pushed on origin, the only way out I found was the one I described in my initial email. I thought then the problem might occur from git, but as you say it is probably a cross platform issue. The folder initially created was named display, then I changed it locally but git wouldn't recognize it as something to commit (platform windows7) I just wanted to let you know in case it would have been an issue you wouldn't be aware of. Regards, Pierre-François I tried to change the folder name on bitbucket.org but I was unable to (or wasn't smart enough to find how to). I fixed the issue by deleting the file from my local, then commit, then push, put the same folder in my local, then commit then push. I am therefore only writing to tell you that story which is not so important, but I had the thought that because it is not so important maybe nobody reports that and the bug (if any) cannot be fixed. Have a good day and happy end of year season! Regards, Pierre-François Duc PhD candidate Physics McGill university -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "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 v3 4/4] git gui: allow for a long recentrepo list
Philip Oakley writes:
> The gui.recentrepo list may be longer than the maxrecent setting.
> Use the actual length determined earlier, now saved as $lenrecent.
>
> In an ideal world, the git gui would limit the number of entries
> to the maxrecent setting, however the recentrepo config list may
> have been extended outside of the gui, or the maxrecent setting changed
> to a reduced value. Further, when testing the gui's recentrepo
> logic it is useful to show these extra, but valid, entries.
>
> The list length will be trimmed to $maxrecent after the user selects
> a repo to open.
>
> Signed-off-by: Philip Oakley
> ---
> V2:
> word usage corrected.
> Eric's comments $gmane/282432
> V3:
> Updated list length measure following Junio's comments $gmane/282669
>
> Replaces the previous V2 Patch 4/4.
>
> ---
> git-gui/lib/choose_repository.tcl | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui/lib/choose_repository.tcl
> b/git-gui/lib/choose_repository.tcl
> index ad7a888..b4cc7dd 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -134,7 +134,8 @@ constructor pick {} {
> $opts conf -state disabled
>
> set sorted_recent [_get_recentrepos]
> - if {[llength $sorted_recent] > 0} {
> + set lenrecent [llength $sorted_recent]
> + if {$lenrecent > 0} {
> if {$m_repo ne {}} {
> $m_repo add separator
> $m_repo add command \
> @@ -153,7 +154,7 @@ constructor pick {} {
> -background [get_bg_color $w_body.recentlabel] \
> -wrap none \
> -width 50 \
> - -height $maxrecent
> + -height $lenrecent
> $w_recentlist tag conf link \
> -foreground blue \
> -underline 1
Natural questions that come to mind after seeing the above are like
these:
- Does maxrecent still get used after this change?
- Does the remainder of the code get upset if -height is set to too
small (lenrecent might turn out to be 1, for example)?
And grepping for maxrecent shows there is _append_recentrepos that
does this:
proc _append_recentrepos {path} {
set path [file normalize $path]
set recent [get_config gui.recentrepo]
...
if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
set maxrecent 10
}
while {[llength $recent] > $maxrecent} {
_unset_recentrepo [lindex $recent 0]
set recent [lrange $recent 1 end]
}
}
The last while loop looks to me like truncating the overlong recent
list down to maxrecent. Perhaps a similar logic needs to be at the
end of _get_recentrepos?
--
To unsubscribe from this list: send the line "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 v3 4/4] git gui: allow for a long recentrepo list
The gui.recentrepo list may be longer than the maxrecent setting.
Use the actual length determined earlier, now saved as $lenrecent.
In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outside of the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.
The list length will be trimmed to $maxrecent after the user selects
a repo to open.
Signed-off-by: Philip Oakley
---
V2:
word usage corrected.
Eric's comments $gmane/282432
V3:
Updated list length measure following Junio's comments $gmane/282669
Replaces the previous V2 Patch 4/4.
---
git-gui/lib/choose_repository.tcl | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-gui/lib/choose_repository.tcl
b/git-gui/lib/choose_repository.tcl
index ad7a888..b4cc7dd 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -134,7 +134,8 @@ constructor pick {} {
$opts conf -state disabled
set sorted_recent [_get_recentrepos]
- if {[llength $sorted_recent] > 0} {
+ set lenrecent [llength $sorted_recent]
+ if {$lenrecent > 0} {
if {$m_repo ne {}} {
$m_repo add separator
$m_repo add command \
@@ -153,7 +154,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
- -height $maxrecent
+ -height $lenrecent
$w_recentlist tag conf link \
-foreground blue \
-underline 1
--
2.6.4.windows.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RefTree: Alternate ref backend
On Thu, Dec 17, 2015 at 2:10 PM, Jeff King wrote: > On Thu, Dec 17, 2015 at 01:02:50PM -0800, Shawn Pearce wrote: > >> I started playing around with the idea of storing references directly >> in Git. Exploiting the GITLINK tree entry, we can associate a name to >> any SHA-1. > > Gitlink entries don't imply reachability, though. I guess that doesn't > matter if your ref backend says "no, really, these are the ref tips, and > they are reachable". Exactly. This works with existing JGit because it swaps out the ref backend. When GC tries to enumerate the roots (current refs), it gets these through the ref backend by scanning the tree recursively. The packer itself doesn't care where those roots came from. Same would be true for any other pluggable ref backend in git-core. GC has to ask the ref backend, and then trust its reply. How/where that ref backend tracks that is an implementation detail. > But you could not push the whole thing up to > another server and expect it to hold the whole graph. Correct, pushing this to another repository doesn't transmit the graph. If the other repository also used this for its refs backend, its now corrupt and confused out of its mind. Just like copying the packed-refs file with scp. Don't do that. :) > Which is not strictly necessary, but to me seems like the real advantage > of using git objects versus some other system. One advantage is you can edit HEAD symref remotely. Commit a different symlink value and push. :) I want to say more, but I'm going to hold back right now. There's more going on in my head than just this. > Of course, the lack of reachability has advantages, too. You can > drop commits pointed to by old reflogs without rewriting the ref > history. Yes. > Unfortunately you cannot expunge the reflogs at all. That's > good if you like audit trails. Bad if you are worried that your reflogs > will grow large. :) At present our servers do not truncate their reflogs. Yes some are... big. I considered truncating this graph by just using a shallow marker. Add a shallow entry and repack. The ancient history will eventually be garbage collected and disappear. One advantage of this format is deleted branches can retain a reflog post deletion. Another is you can trivially copy the reflog using native Git to another system for backup purposes. Or fetch it over the network to inspect locally. So a shared group server could be exporting its reflog, you can fetch it and review locally what happened to branches without logging into the shared server. So long as you remember that copying the reflog doesn't mean you actually copied the commit histories, its works nicely. Another advantage of this format over LMDB or TDB or whatever is Git already understands it. The tools already understand it. Plumbing can inspect and repair things. You can reflog the reflog using traditional reflog ($GIT_DIR/reflogs/refs/txn/committed). >> By storing all references in a single tree, atomic transactions are >> possible. Its a simple compare-and-swap of a single 40 byte SHA-1. >> This of course leads to a bootstrapping problem, where do we store the >> 40 byte SHA-1? For this example its just $GIT_DIR/refs/txn/committed >> as a classical loose reference. > > Somehow putting it inside `refs/` seems weird to me, in an infinite > recursion kind of way. I would have picked $GIT_DIR/REFSTREE or > something. But that is a minor point. I had started with $GIT_DIR/REFS, but see above. I have more going on in my head. This is only a tiny building block. >> Configuration: >> >> [core] >> repositoryformatversion = 1 >> [extensions] >> refsBackendType = RefTree > > The semantics of extensions config keys are open-ended. The > formatVersion=1 spec only says "if there is a key you don't know about, > then you may not proceed". Now we're defining a refsBackendType > extension. It probably makes sense to write up a few rules (e.g., is > RefTree case-sensitive?). In my prototype in JGIt I parse it as case insensitive, but used CamelCase because the JavaClassNameIsNamedThatWayBecauseJava. -- To unsubscribe from this list: send the line "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: RefTree: Alternate ref backend
On Thu, Dec 17, 2015 at 1:57 PM, Junio C Hamano wrote:
> Shawn Pearce writes:
>
>> For example, recent git.git has a structure like this:
>>
>> $ git ls-tree -r refs/txn/committed
>> 12 blob 22e42fc826b437033ca444e09368f53a0b169322 ..HEAD
>> 16 commit 1ff88560c8d22bcdb528a6629239d638f927cb96 heads/maint
>> 16 commit f3adf457e046f92f039353762a78dcb3afb2cb13 heads/master
>> 16 commit 5ee9e94ccfede68f0c386c497dd85c017efa22d6 heads/next
>> 16 commit d3835d54cffb16c4362979a5be3ba9958eab4116 heads/pu
>> 16 commit 68a0f56b615b61afdbd86be01a3ca63dca70edc0 heads/todo
>> ...
>> 16 commit 17f9f635c101aef03874e1de1d8d0322187494b3 tags/v2.6.0
>> 16 commit 5bebb9057df8287684c763c59c67f25f16884ef6 tags/v2.6.0-rc0
>> 16 commit 16ffa6443e279a9b3b63d7a2bebeb07833506010 tags/v2.6.0-rc0^{}
>> ...
>> 16 commit be08dee9738eaaa0423885ed189c2b6ad8368cf0 tags/v2.6.0^{}
>>
>> Tags are stored twice, to cache the peel information for network
>> advertisements.
>
> The object 17f9f635 is not a "commit" but is a "tag". It is
> somewhat unfortunate that "ls-tree -r" reports it as a commit; as
> the command (or anything that deals with a gitlink) is not allowed
> to look at the actual object, it is the best it could do, though.
Yes; thus far GITLINK is only used for commits in submodules so its
reasonable for it to just hardcode the text "commit".
> The "..HEAD" hack looks somewhat ugly. I can guess that the
> reasoning went like "if we stored these in the most natural way, we
> always need a top-level tree that hold two and only two entries,
> HEAD and heads/, which would require us one level of tree unwrapping
> to get to most of the refs" and "HEAD is the only oddball that is
> outside refs/ hierarchy,
Correct.
> others like MERGE_HEAD are ephemeral and
> for the purpose of Gerrit that does not even do working tree
> management, those things that are not necessary in order to manage
> only the committed state can be omitted.", but still...
Yes. I was mostly looking at this from a bare repository server
perspective, not a user working tree. On a bare repository you
probably don't have those special refs like MERGE_HEAD, FETCH_HEAD,
etc.
They could be stored as "..MERGE_HEAD", if you had to. But only HEAD
really matters to hint to clients what to checkout by default on
clone.
--
To unsubscribe from this list: send the line "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] revision.c: propagate tag names from pending array
On Thu, Dec 17, 2015 at 12:28:48PM -0800, Junio C Hamano wrote: > By the way, a totally unrelated niggle I have with 2073949 is this. > > $ git describe --contains 2073949 > v2.3.1~3^2~4 > > while as you said, this dates back to at least v2.2.0-rc0 > > $ git tag --contains 2073949 > v2.2.0 > v2.2.0-rc0 > ... > v2.7.0-rc1 > > That "describe --contains" output comes from "name-rev --tags", and > I need to force it to use v2.2.0-rc0 as the source of naming, i.e. > > $ git name-rev --refs=refs/tags/v2.2.0-rc0 2073949 > 2073949 tags/v2.2.0-rc0~13^2~9 > > to get what I would expect to be more useful. > > I know "name-rev --contains" wants to describe a commit based on an > anchor point that is topologically closest, and even though I do not > offhand think of any, I am sure there are valid use cases that want > to see the current behaviour. But from time to time, I wish it did > its naming taking the topological age of the anchor points into > account. If a commit is contained in v2.2.0-rc0 and onward, even > though v2.0.0-rc0~13^2~9 describes a longer path from v2.0.0-rc0 > than v2.3.1~3^2~4 is from v2.3.1, I often want to see the name based > on the "oldest" tag (if such a thing exists, and for older commits > in this project, it always is the case, I think). Yes, I agree (and judging by the number of "git describe is confusing" threads over the years, I think it is not just us). I rarely use "git describe --contains" myself. What I typically use (and how I arrived at v2.2.0 in this instance) is: git tag --contains "$@" | grep ^v | grep -v -- -rc | sort -V | head -1 But there are some git-specific assumptions in there. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] git gui: allow for a long recentrepo list
From: "Philip Oakley"
From: "Junio C Hamano" , December 17, 2015 7:32 PM
Philip Oakley writes:
The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.
In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outside of the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.
I do not have a strong objection either way, but the magic number 5
smells like an indication that this is solving a wrong problem or
solving a problem in a wrong approach. What happens to the entries
beyond $maxrecent-th in the recent list with the current code? The
list effectively truncated and not shown? Wouldn't the same thing
happen to the entries beyond ($maxrecent+5)-th in the recent list
with your patch?
True, it was a magic number selected as being a moderate overspill
allowance.
I tried 3 and 5 as possible values, and 5 didn't look ungainly, even on my
small netbook, and covered all my test hacking cases.
The issue was that the prior code assumed that the fault could never
happen and limited the display, which is sorted, to the max value, so in a
fault condition the most recent repo could fall off the bottom.
As a bit of 'monkey patching' I wasn't sure how the dialog creation could
know in advance how long the list would be, so I simply added on a small
margin.
Truncating the list at $maxrecent-th to match the display
(i.e. declare that "$maxrecent" is really the max number of entries
we would keep track of), or allowing the UI to scroll so that
entries beyond $maxrecent-th one can be shown, perhaps?
This would certainly be a reasonable approach, but I don't know enough of
the tcl to know how to make it happen. If anyone has suggestions...
I think I see the way to do it. V3 Patch 4/4 to follow.
Signed-off-by: Philip Oakley
---
word usage corrected.
Eric's comments $gmane/282432
---
git-gui/lib/choose_repository.tcl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-gui/lib/choose_repository.tcl
b/git-gui/lib/choose_repository.tcl
index ad7a888..a08ed4f 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -153,7 +153,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
- -height $maxrecent
+ -height [expr {$maxrecent + 5}]
$w_recentlist tag conf link \
-foreground blue \
-underline 1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "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: RefTree: Alternate ref backend
On Thu, Dec 17, 2015 at 01:02:50PM -0800, Shawn Pearce wrote: > I started playing around with the idea of storing references directly > in Git. Exploiting the GITLINK tree entry, we can associate a name to > any SHA-1. Gitlink entries don't imply reachability, though. I guess that doesn't matter if your ref backend says "no, really, these are the ref tips, and they are reachable". But you could not push the whole thing up to another server and expect it to hold the whole graph. Which is not strictly necessary, but to me seems like the real advantage of using git objects versus some other system. Of course, the lack of reachability has advantages, too. You can drop commits pointed to by old reflogs without rewriting the ref history. Unfortunately you cannot expunge the reflogs at all. That's good if you like audit trails. Bad if you are worried that your reflogs will grow large. :) > By storing all references in a single tree, atomic transactions are > possible. Its a simple compare-and-swap of a single 40 byte SHA-1. > This of course leads to a bootstrapping problem, where do we store the > 40 byte SHA-1? For this example its just $GIT_DIR/refs/txn/committed > as a classical loose reference. Somehow putting it inside `refs/` seems weird to me, in an infinite recursion kind of way. I would have picked $GIT_DIR/REFSTREE or something. But that is a minor point. > Configuration: > > [core] > repositoryformatversion = 1 > [extensions] > refsBackendType = RefTree The semantics of extensions config keys are open-ended. The formatVersion=1 spec only says "if there is a key you don't know about, then you may not proceed". Now we're defining a refsBackendType extension. It probably makes sense to write up a few rules (e.g., is RefTree case-sensitive?). -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: RefTree: Alternate ref backend
Shawn Pearce writes:
> For example, recent git.git has a structure like this:
>
> $ git ls-tree -r refs/txn/committed
> 12 blob 22e42fc826b437033ca444e09368f53a0b169322 ..HEAD
> 16 commit 1ff88560c8d22bcdb528a6629239d638f927cb96 heads/maint
> 16 commit f3adf457e046f92f039353762a78dcb3afb2cb13 heads/master
> 16 commit 5ee9e94ccfede68f0c386c497dd85c017efa22d6 heads/next
> 16 commit d3835d54cffb16c4362979a5be3ba9958eab4116 heads/pu
> 16 commit 68a0f56b615b61afdbd86be01a3ca63dca70edc0 heads/todo
> ...
> 16 commit 17f9f635c101aef03874e1de1d8d0322187494b3 tags/v2.6.0
> 16 commit 5bebb9057df8287684c763c59c67f25f16884ef6 tags/v2.6.0-rc0
> 16 commit 16ffa6443e279a9b3b63d7a2bebeb07833506010 tags/v2.6.0-rc0^{}
> ...
> 16 commit be08dee9738eaaa0423885ed189c2b6ad8368cf0 tags/v2.6.0^{}
>
> Tags are stored twice, to cache the peel information for network
> advertisements.
The object 17f9f635 is not a "commit" but is a "tag". It is
somewhat unfortunate that "ls-tree -r" reports it as a commit; as
the command (or anything that deals with a gitlink) is not allowed
to look at the actual object, it is the best it could do, though.
The "..HEAD" hack looks somewhat ugly. I can guess that the
reasoning went like "if we stored these in the most natural way, we
always need a top-level tree that hold two and only two entries,
HEAD and heads/, which would require us one level of tree unwrapping
to get to most of the refs" and "HEAD is the only oddball that is
outside refs/ hierarchy, others like MERGE_HEAD are ephemeral and
for the purpose of Gerrit that does not even do working tree
management, those things that are not necessary in order to manage
only the committed state can be omitted.", but still...
--
To unsubscribe from this list: send the line "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 v3] git-svn: add support for prefixed globs in config
Introduce prefixed globs for branches and tags in git-svn.
Globs like 'release_*' allow users to avoid long lines in config like:
branches = branches/{release_20,release_21,release_22,...}
Signed-off-by: Victor Leschuk
---
Changes from v1 (in v2 I forgot to switch from `` to $() ):
* Joined implementation and test in one patch
* Fixed test code style according to current coding style guide
Documentation/git-svn.txt| 5 ++
perl/Git/SVN/GlobSpec.pm | 9 ++-
t/t9168-git-svn-prefixed-glob.sh | 136 +++
3 files changed, 145 insertions(+), 5 deletions(-)
create mode 100755 t/t9168-git-svn-prefixed-glob.sh
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 0c0f60b..529cffe 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -1034,6 +1034,7 @@ listed below are allowed:
url = http://server.org/svn
fetch = trunk/project-a:refs/remotes/project-a/trunk
branches = branches/*/project-a:refs/remotes/project-a/branches/*
+ branches = branches/release_*:refs/remotes/project-a/branches/release_*
tags = tags/*/project-a:refs/remotes/project-a/tags/*
@@ -1044,6 +1045,10 @@ independent path component (surrounded by '/' or EOL).
This
type of configuration is not automatically created by 'init' and
should be manually entered with a text-editor or using 'git config'.
+Also note that prefixed globs (e.g. 'release_*') match everything after prefix
+but do not match exact prefix. For example:
+'release_*' will match 'release_1' or 'release_v1' but will not match
'release_'.
+
It is also possible to fetch a subset of branches or tags by using a
comma-separated list of names within braces. For example:
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index c95f5d7..a136090 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -11,16 +11,15 @@ sub new {
my $die_msg = "Only one set of wildcard directories " .
"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
for my $part (split(m|/|, $glob)) {
- if ($part =~ /\*/ && $part ne "*") {
- die "Invalid pattern in '$glob': $part\n";
- } elsif ($pattern_ok && $part =~ /[{}]/ &&
+ if ($pattern_ok && $part =~ /[{}]/ &&
$part !~ /^\{[^{}]+\}/) {
die "Invalid pattern in '$glob': $part\n";
}
- if ($part eq "*") {
+ if ($part =~ /(\w*)\*/) {
die $die_msg if $state eq "right";
$state = "pattern";
- push(@patterns, "[^/]*");
+ my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+ push(@patterns, $pat);
} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
die $die_msg if $state eq "right";
$state = "pattern";
diff --git a/t/t9168-git-svn-prefixed-glob.sh b/t/t9168-git-svn-prefixed-glob.sh
new file mode 100755
index 000..a5a3227
--- /dev/null
+++ b/t/t9168-git-svn-prefixed-glob.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+cat >expect.end trunk/src/b/readme &&
+ svn_cmd import -m "initial" trunk "$svnrepo"/trunk &&
+ svn_cmd co "$svnrepo" tmp &&
+ (
+ cd tmp &&
+ mkdir branches tags &&
+ svn_cmd add branches tags &&
+ svn_cmd cp trunk branches/b_start &&
+ svn_cmd commit -m "start a new branch" &&
+ svn_cmd up &&
+ echo "hi" >>branches/b_start/src/b/readme &&
+ poke branches/b_start/src/b/readme &&
+ echo "hey" >>branches/b_start/src/a/readme &&
+ poke branches/b_start/src/a/readme &&
+ svn_cmd commit -m "hi" &&
+ svn_cmd up &&
+ svn_cmd cp branches/b_start tags/t_end &&
+ echo "bye" >>tags/t_end/src/b/readme &&
+ poke tags/t_end/src/b/readme &&
+ echo "aye" >>tags/t_end/src/a/readme &&
+ poke tags/t_end/src/a/readme &&
+ svn_cmd commit -m "the end" &&
+ echo "byebye" >>tags/t_end/src/b/readme &&
+ poke tags/t_end/src/b/readme &&
+ svn_cmd commit -m "nothing to see here"
+ ) &&
+ git config --add svn-remote.svn.url "$svnrepo" &&
+ git config --add svn-remote.svn.fetch \
+"trunk/src/a:refs/remotes/trunk" &&
+ git config --add svn-remote.svn.branches \
+"branches/b_*/src/a:refs/remotes/branches/b_*" &&
+ git config --add svn-remote.svn.tags\
+
RefTree: Alternate ref backend
I started playing around with the idea of storing references directly
in Git. Exploiting the GITLINK tree entry, we can associate a name to
any SHA-1.
By storing all references in a single tree, atomic transactions are
possible. Its a simple compare-and-swap of a single 40 byte SHA-1.
This of course leads to a bootstrapping problem, where do we store the
40 byte SHA-1? For this example its just $GIT_DIR/refs/txn/committed
as a classical loose reference.
I posted code for this to JGit (sorry):
https://git.eclipse.org/r/62970
Configuration:
[core]
repositoryformatversion = 1
[extensions]
refsBackendType = RefTree
For example, recent git.git has a structure like this:
$ git ls-tree -r refs/txn/committed
12 blob 22e42fc826b437033ca444e09368f53a0b169322 ..HEAD
16 commit 1ff88560c8d22bcdb528a6629239d638f927cb96 heads/maint
16 commit f3adf457e046f92f039353762a78dcb3afb2cb13 heads/master
16 commit 5ee9e94ccfede68f0c386c497dd85c017efa22d6 heads/next
16 commit d3835d54cffb16c4362979a5be3ba9958eab4116 heads/pu
16 commit 68a0f56b615b61afdbd86be01a3ca63dca70edc0 heads/todo
...
16 commit 17f9f635c101aef03874e1de1d8d0322187494b3 tags/v2.6.0
16 commit 5bebb9057df8287684c763c59c67f25f16884ef6 tags/v2.6.0-rc0
16 commit 16ffa6443e279a9b3b63d7a2bebeb07833506010 tags/v2.6.0-rc0^{}
16 commit bbdca2a7bd942e1d3ce517b48e6229b99f7d7b2b tags/v2.6.0-rc1
16 commit 689efb737a7b46351850eefdfa57d2ce232011fb tags/v2.6.0-rc1^{}
16 commit 7b269a793392ee3d71ecddac88a8ad63497cbc4d tags/v2.6.0-rc2
16 commit 45733fa93f287fbc04d6a6a3f5a39cc852c5cf50 tags/v2.6.0-rc2^{}
16 commit 27df6e2585060add45b32bbd46f6e92ef79d069b tags/v2.6.0-rc3
16 commit 8d530c4d64ffcc853889f7b385f554d53db375ed tags/v2.6.0-rc3^{}
16 commit be08dee9738eaaa0423885ed189c2b6ad8368cf0 tags/v2.6.0^{}
Tags are stored twice, to cache the peel information for network advertisements.
Packing the tree by itself is smaller than packed-refs, which is uncompressed:
-rw-r- 1 shawn me 60K Dec 17 12:43 packed-refs
-r--r- 1 shawn me 1.2K Dec 17 12:56 R-5533*.idx
-r--r- 1 shawn me 28K Dec 17 12:56 R-5533*.pack
By exploiting Git to store Git, we get a reflog for free:
$ git log -p refs/txn/committed -1
commit f7ec5ceeba6ca87fa112b3af70d8ac17364045f7
Author: anonymous
Date: Thu Dec 17 12:53:39 2015 -0800
push
diff --git a/heads/tmp2 b/heads/tmp2
deleted file mode 16
index f3adf45..000
--- a/heads/tmp2
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit f3adf457e046f92f039353762a78dcb3afb2cb13
:)
--
To unsubscribe from this list: send the line "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: [PATCHv2 2/7] xread: poll on non blocking fds
On Thu, Dec 17, 2015 at 12:42 PM, Torsten Bögershausen wrote:
> On 17.12.15 21:22, Stefan Beller wrote:
>> On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen wrote:
>>> On 16.12.15 01:04, Stefan Beller wrote:
The man page of read(2) says:
EAGAIN The file descriptor fd refers to a file other than a socket
and has been marked nonblocking (O_NONBLOCK), and the read
would block.
EAGAIN or EWOULDBLOCK
The file descriptor fd refers to a socket and has been marked
nonblocking (O_NONBLOCK), and the read would block. POSIX.1-2001
allows either error to be returned for this case, and does not
require these constants to have the same value, so a portable
application should check for both possibilities.
If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
As the intent of xread is to read as much as possible either until the
fd is EOF or an actual error occurs, we can ease the feeder of the fd
by not spinning the whole time, but rather wait for it politely by not
busy waiting.
We should not care if the call to poll failed, as we're in an infinite
loop and can only get out with the correct read().
>>> I'm not sure if this is valid under all circumstances:
>>> This is what "man poll" says under Linux:
>>> []
>>> ENOMEM There was no space to allocate file descriptor tables.
>>> []
>>> And this is from Mac OS, ("BSD System Calls Manual")
>>> ERRORS
>>> Poll() will fail if:
>>>
>>> [EAGAIN] Allocation of internal data structures fails. A
>>> sub-
>>> sequent request may succeed.
>>> And this is opengroup:
>>> http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
>>> [EAGAIN]
>>> The allocation of internal data structures failed but a subsequent
>>> request may succeed.
>>>
>>> read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
>>> Is it always guaranteed that the loop is terminated?
>>
>> In case poll fails (assume a no op for it), the logic should not have
>> changed by this patch?
>>
>> Looking closely:
>>
while (1) {
nr = read(fd, buf, len);
- if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
- continue;
+ if (nr < 0) {
+ if (errno == EINTR)
+ continue;
+ if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ struct pollfd pfd;
+ pfd.events = POLLIN;
+ pfd.fd = fd;
+ /*
+ * it is OK if this poll() failed; we
+ * want to leave this infinite loop
+ * only when read() returns with
+ * success, or an expected failure,
+ * which would be checked by the next
+ * call to read(2).
+ */
+ poll(&pfd, 1, -1);
>>
>> Or do you mean to insert another continue in here?
> I was thinking that we run into similar loop as before:
> read() returns -1; errno = EAGAIN /* No data to read */
which is expected for non blocking fds,
> poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the
> failure may be temporaly,
> as much as poll() can see.
> But most probably we run out ouf memory */
Before this patch we would not have asked poll, but had just a continue here,
so I think we need to have it here again no matter of the return code
of the poll.
If poll determines it is low on memory, this should not make this function fail,
we can still do as good as we did before by just asking read
repeatedly again, though?
So I'd be convinced now we'd want to have:
poll(&pfd, 1, -1); /* this is only buying time
for the fd to deliver data, in case it fails
we don't care but just fall back to old
behavior before this patch with busy spinning*/
continue;
>
> So the code would look like this:
>
>if (!poll(&pfd, 1, -1))
> return -1;
>
>
>>
>
+ }
+ }
return nr;
}
}
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
--
To unsubscribe from this list: send the line "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 15/16] refs: add LMDB refs backend
David Turner wrote: > On Wed, 2015-12-16 at 17:00 -0800, Jonathan Nieder wrote: >> David Turner wrote: >>> +core.refsBackendType:: >>> + Type of refs backend. Default is to use the original files >>> + based backend. Set to 'lmdb' to activate the lmdb database >>> + backend. If you use the lmdb backend, >>> + core.repositoryFormatVersion must be set to 1, and >>> + extensions.refBackend must be set to 'lmdb'. >> >> If core.refsBackendType names one backend and extensions.refBackend >> names another, which wins? > > I've simplified this to just use extensions.refsBackendType for > everything. Will re-roll once I finish getting comments on the rest of > this series. Perfect. Thanks for the quick reply. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/7] xread: poll on non blocking fds
On 17.12.15 21:22, Stefan Beller wrote:
> On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen wrote:
>> On 16.12.15 01:04, Stefan Beller wrote:
>>> The man page of read(2) says:
>>>
>>> EAGAIN The file descriptor fd refers to a file other than a socket
>>>and has been marked nonblocking (O_NONBLOCK), and the read
>>>would block.
>>>
>>> EAGAIN or EWOULDBLOCK
>>>The file descriptor fd refers to a socket and has been marked
>>>nonblocking (O_NONBLOCK), and the read would block. POSIX.1-2001
>>>allows either error to be returned for this case, and does not
>>>require these constants to have the same value, so a portable
>>>application should check for both possibilities.
>>>
>>> If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
>>> As the intent of xread is to read as much as possible either until the
>>> fd is EOF or an actual error occurs, we can ease the feeder of the fd
>>> by not spinning the whole time, but rather wait for it politely by not
>>> busy waiting.
>>>
>>> We should not care if the call to poll failed, as we're in an infinite
>>> loop and can only get out with the correct read().
>> I'm not sure if this is valid under all circumstances:
>> This is what "man poll" says under Linux:
>> []
>> ENOMEM There was no space to allocate file descriptor tables.
>> []
>> And this is from Mac OS, ("BSD System Calls Manual")
>> ERRORS
>> Poll() will fail if:
>>
>> [EAGAIN] Allocation of internal data structures fails. A sub-
>> sequent request may succeed.
>> And this is opengroup:
>> http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
>> [EAGAIN]
>> The allocation of internal data structures failed but a subsequent
>> request may succeed.
>>
>> read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
>> Is it always guaranteed that the loop is terminated?
>
> In case poll fails (assume a no op for it), the logic should not have
> changed by this patch?
>
> Looking closely:
>
>>> while (1) {
>>> nr = read(fd, buf, len);
>>> - if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
>>> - continue;
>>> + if (nr < 0) {
>>> + if (errno == EINTR)
>>> + continue;
>>> + if (errno == EAGAIN || errno == EWOULDBLOCK) {
>>> + struct pollfd pfd;
>>> + pfd.events = POLLIN;
>>> + pfd.fd = fd;
>>> + /*
>>> + * it is OK if this poll() failed; we
>>> + * want to leave this infinite loop
>>> + * only when read() returns with
>>> + * success, or an expected failure,
>>> + * which would be checked by the next
>>> + * call to read(2).
>>> + */
>>> + poll(&pfd, 1, -1);
>
> Or do you mean to insert another continue in here?
I was thinking that we run into similar loop as before:
read() returns -1; errno = EAGAIN /* No data to read */
poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the failure
may be temporaly,
as much as poll() can see.
But most probably we run out ouf memory */
So the code would look like this:
if (!poll(&pfd, 1, -1))
return -1;
>
>>> + }
>>> + }
>>> return nr;
>>> }
>>> }
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "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] revision.c: propagate tag names from pending array
Jeff King writes: > When we unwrap a tag to find its commit for a traversal, we > do not propagate the "name" field of the tag in the pending > array (i.e., the ref name the user gave us in the first > place) to the commit (instead, we use an empty string). This > means that "git log --source" will never show the tag-name > for commits we reach through it. > > This was broken in 2073949 (traverse_commit_list: support > pending blobs/trees with paths, 2014-10-15). That commit > tried to be careful and avoid propagating the path > information for a tag (which would be nonsensical) to trees > and blobs. But it should not have cut off the "name" field, > which should carry forward to children. > ... > This was reported several weeks ago, but I needed to take the time to > convince myself this wasn't regressing any cases. I'm pretty sure it's > the right thing to do. > > The regression is in v2.2.0, so this is not urgent to make it into v2.7 > before release, but it is definitely maint-worthy. Makes sense, and I agree. By the way, a totally unrelated niggle I have with 2073949 is this. $ git describe --contains 2073949 v2.3.1~3^2~4 while as you said, this dates back to at least v2.2.0-rc0 $ git tag --contains 2073949 v2.2.0 v2.2.0-rc0 ... v2.7.0-rc1 That "describe --contains" output comes from "name-rev --tags", and I need to force it to use v2.2.0-rc0 as the source of naming, i.e. $ git name-rev --refs=refs/tags/v2.2.0-rc0 2073949 2073949 tags/v2.2.0-rc0~13^2~9 to get what I would expect to be more useful. I know "name-rev --contains" wants to describe a commit based on an anchor point that is topologically closest, and even though I do not offhand think of any, I am sure there are valid use cases that want to see the current behaviour. But from time to time, I wish it did its naming taking the topological age of the anchor points into account. If a commit is contained in v2.2.0-rc0 and onward, even though v2.0.0-rc0~13^2~9 describes a longer path from v2.0.0-rc0 than v2.3.1~3^2~4 is from v2.3.1, I often want to see the name based on the "oldest" tag (if such a thing exists, and for older commits in this project, it always is the case, I think). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/7] xread: poll on non blocking fds
On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen wrote:
> On 16.12.15 01:04, Stefan Beller wrote:
>> The man page of read(2) says:
>>
>> EAGAIN The file descriptor fd refers to a file other than a socket
>>and has been marked nonblocking (O_NONBLOCK), and the read
>>would block.
>>
>> EAGAIN or EWOULDBLOCK
>>The file descriptor fd refers to a socket and has been marked
>>nonblocking (O_NONBLOCK), and the read would block. POSIX.1-2001
>>allows either error to be returned for this case, and does not
>>require these constants to have the same value, so a portable
>>application should check for both possibilities.
>>
>> If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
>> As the intent of xread is to read as much as possible either until the
>> fd is EOF or an actual error occurs, we can ease the feeder of the fd
>> by not spinning the whole time, but rather wait for it politely by not
>> busy waiting.
>>
>> We should not care if the call to poll failed, as we're in an infinite
>> loop and can only get out with the correct read().
> I'm not sure if this is valid under all circumstances:
> This is what "man poll" says under Linux:
> []
> ENOMEM There was no space to allocate file descriptor tables.
> []
> And this is from Mac OS, ("BSD System Calls Manual")
> ERRORS
> Poll() will fail if:
>
> [EAGAIN] Allocation of internal data structures fails. A sub-
> sequent request may succeed.
> And this is opengroup:
> http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
> [EAGAIN]
> The allocation of internal data structures failed but a subsequent
> request may succeed.
>
> read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
> Is it always guaranteed that the loop is terminated?
In case poll fails (assume a no op for it), the logic should not have
changed by this patch?
Looking closely:
>> while (1) {
>> nr = read(fd, buf, len);
>> - if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
>> - continue;
>> + if (nr < 0) {
>> + if (errno == EINTR)
>> + continue;
>> + if (errno == EAGAIN || errno == EWOULDBLOCK) {
>> + struct pollfd pfd;
>> + pfd.events = POLLIN;
>> + pfd.fd = fd;
>> + /*
>> + * it is OK if this poll() failed; we
>> + * want to leave this infinite loop
>> + * only when read() returns with
>> + * success, or an expected failure,
>> + * which would be checked by the next
>> + * call to read(2).
>> + */
>> + poll(&pfd, 1, -1);
Or do you mean to insert another continue in here?
>> + }
>> + }
>> return nr;
>> }
>> }
>>
>
--
To unsubscribe from this list: send the line "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: [PATCHv2 2/7] xread: poll on non blocking fds
On 16.12.15 01:04, Stefan Beller wrote:
> The man page of read(2) says:
>
> EAGAIN The file descriptor fd refers to a file other than a socket
>and has been marked nonblocking (O_NONBLOCK), and the read
>would block.
>
> EAGAIN or EWOULDBLOCK
>The file descriptor fd refers to a socket and has been marked
>nonblocking (O_NONBLOCK), and the read would block. POSIX.1-2001
>allows either error to be returned for this case, and does not
>require these constants to have the same value, so a portable
>application should check for both possibilities.
>
> If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
> As the intent of xread is to read as much as possible either until the
> fd is EOF or an actual error occurs, we can ease the feeder of the fd
> by not spinning the whole time, but rather wait for it politely by not
> busy waiting.
>
> We should not care if the call to poll failed, as we're in an infinite
> loop and can only get out with the correct read().
I'm not sure if this is valid under all circumstances:
This is what "man poll" says under Linux:
[]
ENOMEM There was no space to allocate file descriptor tables.
[]
And this is from Mac OS, ("BSD System Calls Manual")
ERRORS
Poll() will fail if:
[EAGAIN] Allocation of internal data structures fails. A sub-
sequent request may succeed.
And this is opengroup:
http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
[EAGAIN]
The allocation of internal data structures failed but a subsequent request
may succeed.
read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
Is it always guaranteed that the loop is terminated?
>
> Signed-off-by: Stefan Beller
> Acked-by: Johannes Sixt
> Signed-off-by: Junio C Hamano
> ---
> wrapper.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/wrapper.c b/wrapper.c
> index 6fcaa4d..1770efa 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -236,8 +236,24 @@ ssize_t xread(int fd, void *buf, size_t len)
> len = MAX_IO_SIZE;
> while (1) {
> nr = read(fd, buf, len);
> - if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
> - continue;
> + if (nr < 0) {
> + if (errno == EINTR)
> + continue;
> + if (errno == EAGAIN || errno == EWOULDBLOCK) {
> + struct pollfd pfd;
> + pfd.events = POLLIN;
> + pfd.fd = fd;
> + /*
> + * it is OK if this poll() failed; we
> + * want to leave this infinite loop
> + * only when read() returns with
> + * success, or an expected failure,
> + * which would be checked by the next
> + * call to read(2).
> + */
> + poll(&pfd, 1, -1);
> + }
> + }
> return nr;
> }
> }
>
--
To unsubscribe from this list: send the line "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 4/4] git gui: allow for a long recentrepo list
From: "Junio C Hamano" , December 17, 2015 7:32 PM
Philip Oakley writes:
The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.
In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outside of the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.
I do not have a strong objection either way, but the magic number 5
smells like an indication that this is solving a wrong problem or
solving a problem in a wrong approach. What happens to the entries
beyond $maxrecent-th in the recent list with the current code? The
list effectively truncated and not shown? Wouldn't the same thing
happen to the entries beyond ($maxrecent+5)-th in the recent list
with your patch?
True, it was a magic number selected as being a moderate overspill
allowance.
I tried 3 and 5 as possible values, and 5 didn't look ungainly, even on my
small netbook, and covered all my test hacking cases.
The issue was that the prior code assumed that the fault could never happen
and limited the display, which is sorted, to the max value, so in a fault
condition the most recent repo could fall off the bottom.
As a bit of 'monkey patching' I wasn't sure how the dialog creation could
know in advance how long the list would be, so I simply added on a small
margin.
Truncating the list at $maxrecent-th to match the display
(i.e. declare that "$maxrecent" is really the max number of entries
we would keep track of), or allowing the UI to scroll so that
entries beyond $maxrecent-th one can be shown, perhaps?
This would certainly be a reasonable approach, but I don't know enough of
the tcl to know how to make it happen. If anyone has suggestions...
Signed-off-by: Philip Oakley
---
word usage corrected.
Eric's comments $gmane/282432
---
git-gui/lib/choose_repository.tcl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-gui/lib/choose_repository.tcl
b/git-gui/lib/choose_repository.tcl
index ad7a888..a08ed4f 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -153,7 +153,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
- -height $maxrecent
+ -height [expr {$maxrecent + 5}]
$w_recentlist tag conf link \
-foreground blue \
-underline 1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "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 4/4] git gui: allow for a long recentrepo list
Philip Oakley writes:
> The gui.recentrepo list may be longer than the maxrecent setting.
> Allow extra space to show any extra entries.
>
> In an ideal world, the git gui would limit the number of entries
> to the maxrecent setting, however the recentrepo config list may
> have been extended outside of the gui, or the maxrecent setting changed
> to a reduced value. Further, when testing the gui's recentrepo
> logic it is useful to show these extra, but valid, entries.
I do not have a strong objection either way, but the magic number 5
smells like an indication that this is solving a wrong problem or
solving a problem in a wrong approach. What happens to the entries
beyond $maxrecent-th in the recent list with the current code? The
list effectively truncated and not shown? Wouldn't the same thing
happen to the entries beyond ($maxrecent+5)-th in the recent list
with your patch?
Truncating the list at $maxrecent-th to match the display
(i.e. declare that "$maxrecent" is really the max number of entries
we would keep track of), or allowing the UI to scroll so that
entries beyond $maxrecent-th one can be shown, perhaps?
>
> Signed-off-by: Philip Oakley
> ---
> word usage corrected.
> Eric's comments $gmane/282432
> ---
> git-gui/lib/choose_repository.tcl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-gui/lib/choose_repository.tcl
> b/git-gui/lib/choose_repository.tcl
> index ad7a888..a08ed4f 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -153,7 +153,7 @@ constructor pick {} {
> -background [get_bg_color $w_body.recentlabel] \
> -wrap none \
> -width 50 \
> - -height $maxrecent
> + -height [expr {$maxrecent + 5}]
> $w_recentlist tag conf link \
> -foreground blue \
> -underline 1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mingw: emulate write(2) that fails with a EPIPE
Johannes Schindelin writes:
> On Windows, when writing to a pipe fails, errno is always
> EINVAL. However, Git expects it to be EPIPE.
>
> According to the documentation, there are two cases in which write()
> triggers EINVAL: the buffer is NULL, or the length is odd but the mode
> is 16-bit Unicode (the broken pipe is not mentioned as possible cause).
> Git never sets the file mode to anything but binary, therefore we know
> that errno should actually be EPIPE if it is EINVAL and the buffer is
> not NULL.
>
> See https://msdn.microsoft.com/en-us/library/1570wh78.aspx for more
> details.
>
> This works around t5571.11 failing with v2.6.4 on Windows.
>
> Signed-off-by: Johannes Schindelin
> ---
> compat/mingw.c | 17 +
> compat/mingw.h | 3 +++
> 2 files changed, 20 insertions(+)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 90bdb1e..5edea29 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -394,6 +394,23 @@ int mingw_fflush(FILE *stream)
> return ret;
> }
>
> +#undef write
> +ssize_t mingw_write(int fd, const void *buf, size_t len)
> +{
> + ssize_t result = write(fd, buf, len);
> +
> + if (result < 0 && errno == EINVAL && buf) {
> + /* check if fd is a pipe */
> + HANDLE h = (HANDLE) _get_osfhandle(fd);
> + if (GetFileType(h) == FILE_TYPE_PIPE)
> + errno = EPIPE;
> + else
> + errno = EINVAL;
> + }
> +
> + return result;
> +}
> +
> int mingw_access(const char *filename, int mode)
> {
> wchar_t wfilename[MAX_PATH];
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 738865c..57ca477 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -210,6 +210,9 @@ FILE *mingw_freopen (const char *filename, const char
> *otype, FILE *stream);
> int mingw_fflush(FILE *stream);
> #define fflush mingw_fflush
>
> +ssize_t mingw_write(int fd, const void *buf, size_t len);
> +#define write mingw_write
> +
> int mingw_access(const char *filename, int mode);
> #undef access
> #define access mingw_access
PLEASE DON'T DO THE BELOW TO THE SAME MESSAGE AS THE PATCH ITSELF.
"git apply" would not read and understand the next line as a natural
language sentence for obvious reasons.
> Interdiff vs v1:
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 90bdb1e..5edea29 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -394,6 +394,23 @@ int mingw_fflush(FILE *stream)
> return ret;
> }
>
> +#undef write
> +ssize_t mingw_write(int fd, const void *buf, size_t len)
> +{
> + ssize_t result = write(fd, buf, len);
> +
> + if (result < 0 && errno == EINVAL && buf) {
> + /* check if fd is a pipe */
> + HANDLE h = (HANDLE) _get_osfhandle(fd);
> + if (GetFileType(h) == FILE_TYPE_PIPE)
> + errno = EPIPE;
> + else
> + errno = EINVAL;
> + }
> +
> + return result;
> +}
> +
> int mingw_access(const char *filename, int mode)
> {
> wchar_t wfilename[MAX_PATH];
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 2aca347..57ca477 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -210,22 +210,7 @@ FILE *mingw_freopen (const char *filename, const char
> *otype, FILE *stream);
> int mingw_fflush(FILE *stream);
> #define fflush mingw_fflush
>
> -static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> -{
> - ssize_t result = write(fd, buf, len);
> -
> - if (result < 0 && errno == EINVAL && buf) {
> - /* check if fd is a pipe */
> - HANDLE h = (HANDLE) _get_osfhandle(fd);
> - if (GetFileType(h) == FILE_TYPE_PIPE)
> - errno = EPIPE;
> - else
> - errno = EINVAL;
> - }
> -
> - return result;
> -}
> -
> +ssize_t mingw_write(int fd, const void *buf, size_t len);
> #define write mingw_write
>
> int mingw_access(const char *filename, int mode);
--
To unsubscribe from this list: send the line "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: [PATCHv2] submodule: Port resolve_relative_url from shell to C
Am 17.12.2015 um 19:55 schrieb Johannes Sixt: As to the implementation, find a patch below that removes the ifdefs and a few other suggestions. The word "offers" is missing in this last half-sentence ;) -- To unsubscribe from this list: send the line "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: [PATCHv2] submodule: Port resolve_relative_url from shell to C
Am 17.12.2015 um 01:26 schrieb Stefan Beller:
> This reimplements the helper function `resolve_relative_url` in shell
> in C. This functionality is needed in C for introducing the groups
> feature later on. When using groups, the user should not need to run
> `git submodule init`, but it should be implicit at all appropriate places,
> which are all in C code. As the we would not just call out to `git
> submodule init`, but do a more fine grained structure there, we actually
> need all the init functionality in C before attempting the groups
> feature. To get the init functionality in C, rewriting the
> resolve_relative_url subfunction is a major step.
>
> This also improves the performance:
> (Best out of 3) time ./t7400-submodule-basic.sh
> Before:
> real 0m9.575s
> user 0m2.683s
> sys 0m6.773s
> After:
> real 0m9.293s
> user 0m2.691s
> sys 0m6.549s
>
> That's about 3%.
I appreciate this effort as it should help us on Windows. Although the
numbers (and my own timings) suggest that this is only a small step
forward. That's not surprising as the patch removes only two forks.
As to the implementation, find a patch below that removes the ifdefs
and a few other suggestions. It is a mechanical conversion without
understanding what relative_url() does. I have the gut feeling that the
two strbuf_addf towards the end of the function can be contracted and
the temporarily allocate copy in 'out' can be removed.
If there were a few examples in the comment above the function, it
would be much simpler to understand.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b925bed..8ec0975 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -11,6 +11,7 @@
#include "run-command.h"
#include "remote.h"
#include "refs.h"
+#include "connect.h"
static const char *get_default_remote(void)
{
@@ -31,34 +32,23 @@ static const char *get_default_remote(void)
return xstrdup(dest);
}
-static int has_same_dir_prefix(const char *str, const char **out)
+static int starts_with_dot_slash(const char *str)
{
-#ifdef GIT_WINDOWS_NATIVE
- return skip_prefix(str, "./", out)
- || skip_prefix(str, ".\\", out);
-#else
- return skip_prefix(str, "./", out);
-#endif
+ return str[0] == '.' && is_dir_sep(str[1]);
}
-static int has_upper_dir_prefix(const char *str, const char **out)
+static int starts_with_dot_dot_slash(const char *str)
{
-#ifdef GIT_WINDOWS_NATIVE
- return skip_prefix(str, "../", out)
- || skip_prefix(str, "..\\", out);
-#else
- return skip_prefix(str, "../", out);
-#endif
+ return str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);
}
-static char *last_dir_separator(const char *str)
+static char *last_dir_separator(char *str)
{
-#ifdef GIT_WINDOWS_NATIVE
- return strrchr(str, "/")
- || strrchr(str, "\\");
-#else
- return strrchr(str, '/');
-#endif
+ char* p = str + strlen(str);
+ while (p-- != str)
+ if (is_dir_sep(*p))
+ return p;
+ return NULL;
}
/*
@@ -85,9 +75,10 @@ static const char *relative_url(const char *url, const char
*up_path)
size_t len;
char *remoteurl = NULL;
char *sep = "/";
- const char *out;
+ char *out;
struct strbuf sb = STRBUF_INIT;
const char *remote = get_default_remote();
+
strbuf_addf(&sb, "remote.%s.url", remote);
if (git_config_get_string(sb.buf, &remoteurl))
@@ -98,22 +89,23 @@ static const char *relative_url(const char *url, const char
*up_path)
if (is_dir_sep(remoteurl[len]))
remoteurl[len] = '\0';
- if (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))
+ if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
is_relative = 0;
- else if (has_same_dir_prefix(remoteurl, &out) ||
- has_upper_dir_prefix(remoteurl, &out))
+ else if (starts_with_dot_slash(remoteurl) ||
+ starts_with_dot_dot_slash(remoteurl))
is_relative = 1;
else {
is_relative = 1;
strbuf_reset(&sb);
strbuf_addf(&sb, "./%s", remoteurl);
+ free(remoteurl);
remoteurl = strbuf_detach(&sb, NULL);
}
while (url) {
- if (has_upper_dir_prefix(url, &out)) {
+ if (starts_with_dot_dot_slash(url)) {
char *rfind;
- url = out;
+ url += 3;
rfind = last_dir_separator(remoteurl);
if (rfind)
@@ -130,22 +122,23 @@ static const char *relative_url(const char *url, const
char *up_path)
remoteurl = ".";
}
}
- } else if (has_same_dir_prefix(url, &out))
-
Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore
Christian Couder writes:
> In the "git worktree" documentation there is:
>
> "If you move a linked working tree to another file system, or within a
> file system that does not support hard links, you need to run at least
> one git command inside the linked working tree (e.g. git status) in
> order to update its administrative files in the repository so that
> they do not get automatically pruned."
>
> It looks like git can detect when a worktree created with "git
> worktree" has been moved and I wonder if it would be possible to
> detect if the main worktree pointed to by GIT_WORK_TREE as moved.
As I personally do not find "moving a working tree" a very
compelling use case, I'd be fine if cached information is not used
when the actual worktree and the root of the cached untracked paths
are different.
If you are going to change the in-index data of untracked cache
anyway (like you attempted with 10/10 patch), I think a lot more
sensible simplification may be to make the mechanism _always_ keep
track of the worktree that is rooted one level above the index, and
not use the cache in all other cases. That way, if you move the
working tree in its entirety (i.e. $foo/{Makefile,.git/,untracked}
all move to $bar/. at the same time), the untracked cache data that
was in $foo/.git/index, which knew about $foo/untracked, will now
know about $bar/untracked when the index is moved to $bar/.git/index
automatically.
--
To unsubscribe from this list: send the line "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 issue report : issue with capital letter in folder name
On Thu, Dec 17, 2015 at 7:45 AM, PFDuc wrote: > Hello, > > first of all thank you for developping git ! > > I had an issue with a capital block in the folder name inside my git repo. > The folder in my local was named "Display" and the one at origin was named > "display" resulting in error when importing python code from this folder for > users who got the repo from origin. By any chance, were different operating systems or file systems involved in creation of this problem? There are file systems which care about the capitalization, and others don't. So if you have a file system which doesn't care about capitalization of the folder/file name, you can use a different capitalization and it still works. If you take the code to another system then, which is a bit more careful there are problems of course. The main question which remains, is how is Git involved? i.e. would it also happen if you just transfer a tarball? Did Git itself break anything? > > I tried to change the folder name on bitbucket.org but I was unable to (or > wasn't smart enough to find how to). > > I fixed the issue by deleting the file from my local, then commit, then > push, put the same folder in my local, then commit then push. > > I am therefore only writing to tell you that story which is not so > important, but I had the thought that because it is not so important maybe > nobody reports that and the bug (if any) cannot be fixed. > > Have a good day and happy end of year season! > > Regards, > > Pierre-François Duc > PhD candidate Physics McGill university > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "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] rebase: add --verify-signatures
Alexander 'z33ky' Hirsch <1ze...@gmail.com> writes: > As I understand it, this is the same reason for the existence of > --verify-signatures for git-merge. Otherwise the same argument could be > made for git-merge I suspect that you are missing the bigger workflow issues, if you think this and merge are the same. git-merge will check the other history on the side branch that you are merging _into_ the trunk, to give you an opportunity to reject what does not pass and keep the trunk sane without doing anything else. How you (or others who asked you to pull) clean up the side branch is outside the scope of its verification. Your change to "git pull --rebase" checks the other way---the history, which is already the trunk, onto which your work will be rebased. There is nothing you can do without messing with the trunk if the validation did not pass, be it with a rewind-and-rebuild or a sealing empty commit which is pointless. -- To unsubscribe from this list: send the line "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 issue report : issue with capital letter in folder name
Hello, first of all thank you for developping git ! I had an issue with a capital block in the folder name inside my git repo. The folder in my local was named "Display" and the one at origin was named "display" resulting in error when importing python code from this folder for users who got the repo from origin. I tried to change the folder name on bitbucket.org but I was unable to (or wasn't smart enough to find how to). I fixed the issue by deleting the file from my local, then commit, then push, put the same folder in my local, then commit then push. I am therefore only writing to tell you that story which is not so important, but I had the thought that because it is not so important maybe nobody reports that and the bug (if any) cannot be fixed. Have a good day and happy end of year season! Regards, Pierre-François Duc PhD candidate Physics McGill university -- To unsubscribe from this list: send the line "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 02/10] update-index: add --test-untracked-cache
On Tue, 2015-12-15 at 17:28 +0100, Christian Couder wrote: > +--test-untracked-cache:: > + Only perform tests on the working directory to make sure > + untracked cache can be used. You have to manually enable > + untracked cache using `--force-untracked-cache` (or > + `--untracked-cache` but this will run the tests again) > + afterwards if you really want to use it. It would be nice if this said how the result would be reported (by exit code, it appears). -- To unsubscribe from this list: send the line "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] mingw: emulate write(2) that fails with a EPIPE
Hi Junio, On Thu, 17 Dec 2015, Junio C Hamano wrote: > Johannes Schindelin writes: > > > My intuition (which I honestly did not verify using performance tests) was > > that write() is called *much* more often than, say, open(),... > > My gut feeling agrees with yours, but I do not think the frequency > at which write() is called should be the primary factor when you > decide to make its wrapper inlined. Once you call write(2), you > will hit either the disk or the network doing I/O, and at that point > I'd expect that the cost of making an extra layer of wrapper call > would be lost in the noise. I'd worry a lot more about from how > many callsites write() is called---by inlining the extra code that > is run only when the underlying write(2) returns an error to many > callsites, we would make the program as a whole bigger, and as the > result other code needs to be evicted out of the instruction cache, > which also would hurt performance. That argument convinced me. v2 coming shortly. Ciao, Dscho -- To unsubscribe from this list: send the line "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] mingw: emulate write(2) that fails with a EPIPE
On Windows, when writing to a pipe fails, errno is always
EINVAL. However, Git expects it to be EPIPE.
According to the documentation, there are two cases in which write()
triggers EINVAL: the buffer is NULL, or the length is odd but the mode
is 16-bit Unicode (the broken pipe is not mentioned as possible cause).
Git never sets the file mode to anything but binary, therefore we know
that errno should actually be EPIPE if it is EINVAL and the buffer is
not NULL.
See https://msdn.microsoft.com/en-us/library/1570wh78.aspx for more
details.
This works around t5571.11 failing with v2.6.4 on Windows.
Signed-off-by: Johannes Schindelin
---
compat/mingw.c | 17 +
compat/mingw.h | 3 +++
2 files changed, 20 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 90bdb1e..5edea29 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -394,6 +394,23 @@ int mingw_fflush(FILE *stream)
return ret;
}
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t len)
+{
+ ssize_t result = write(fd, buf, len);
+
+ if (result < 0 && errno == EINVAL && buf) {
+ /* check if fd is a pipe */
+ HANDLE h = (HANDLE) _get_osfhandle(fd);
+ if (GetFileType(h) == FILE_TYPE_PIPE)
+ errno = EPIPE;
+ else
+ errno = EINVAL;
+ }
+
+ return result;
+}
+
int mingw_access(const char *filename, int mode)
{
wchar_t wfilename[MAX_PATH];
diff --git a/compat/mingw.h b/compat/mingw.h
index 738865c..57ca477 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -210,6 +210,9 @@ FILE *mingw_freopen (const char *filename, const char
*otype, FILE *stream);
int mingw_fflush(FILE *stream);
#define fflush mingw_fflush
+ssize_t mingw_write(int fd, const void *buf, size_t len);
+#define write mingw_write
+
int mingw_access(const char *filename, int mode);
#undef access
#define access mingw_access
Interdiff vs v1:
diff --git a/compat/mingw.c b/compat/mingw.c
index 90bdb1e..5edea29 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -394,6 +394,23 @@ int mingw_fflush(FILE *stream)
return ret;
}
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t len)
+{
+ ssize_t result = write(fd, buf, len);
+
+ if (result < 0 && errno == EINVAL && buf) {
+ /* check if fd is a pipe */
+ HANDLE h = (HANDLE) _get_osfhandle(fd);
+ if (GetFileType(h) == FILE_TYPE_PIPE)
+ errno = EPIPE;
+ else
+ errno = EINVAL;
+ }
+
+ return result;
+}
+
int mingw_access(const char *filename, int mode)
{
wchar_t wfilename[MAX_PATH];
diff --git a/compat/mingw.h b/compat/mingw.h
index 2aca347..57ca477 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -210,22 +210,7 @@ FILE *mingw_freopen (const char *filename, const char
*otype, FILE *stream);
int mingw_fflush(FILE *stream);
#define fflush mingw_fflush
-static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
-{
- ssize_t result = write(fd, buf, len);
-
- if (result < 0 && errno == EINVAL && buf) {
- /* check if fd is a pipe */
- HANDLE h = (HANDLE) _get_osfhandle(fd);
- if (GetFileType(h) == FILE_TYPE_PIPE)
- errno = EPIPE;
- else
- errno = EINVAL;
- }
-
- return result;
-}
-
+ssize_t mingw_write(int fd, const void *buf, size_t len);
#define write mingw_write
int mingw_access(const char *filename, int mode);
--
2.6.3.windows.1.300.g1c25e49
--
To unsubscribe from this list: send the line "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 10/10] dir: do not use untracked cache ident anymore
On Tue, Dec 15, 2015 at 8:49 PM, Junio C Hamano wrote: > Christian Couder writes: > >> +/* >> + * We used to save the location of the work tree and the kernel version, >> + * but it was not a good idea, so we now just save an empty string. >> + */ > > I do agree that storing the kernel version (or hostname or whatever > specific to the machine) was not a good idea. I however suspect > that you must save and check the location of the working tree, > though, for correctness. If you use one GIT_DIR and GIT_WORK_TREE > to do "git add" or whatever, and then use the same GIT_DIR but a > different GIT_WORK_TREE, you should be able to notice that a > directory D in the old GIT_WORK_TREE whose modification time you > recorded is different from the directory D with the same name but in > the new GIT_WORK_TREE, no? Yeah, if people use many worktrees made using "git worktree", the code above should be fine because there is one index per worktree, but if they just use one GIT_DIR with many GIT_WORK_TREE then there will be only one index used. I am wondering about the case when the worktree is moved and then GIT_WORK_TREE is changed to reflect the new path were the worktree has been moved. In the "git worktree" documentation there is: "If you move a linked working tree to another file system, or within a file system that does not support hard links, you need to run at least one git command inside the linked working tree (e.g. git status) in order to update its administrative files in the repository so that they do not get automatically pruned." It looks like git can detect when a worktree created with "git worktree" has been moved and I wonder if it would be possible to detect if the main worktree pointed to by GIT_WORK_TREE as moved. -- To unsubscribe from this list: send the line "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] push: Fully test --recurse-submodules on command line overrides config
Stefan Beller writes: >>> This looks good to me. >>> >> Thanks. Does "This" refer to 1/2 alone or the whole series? > > Yes. :) > > "This" is applicable to both patches. We had the discussion on 2/2 about me > misreading a line a few days earlier, but apart from that it looked good, too. 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] mingw: emulate write(2) that fails with a EPIPE
Johannes Schindelin writes: > My intuition (which I honestly did not verify using performance tests) was > that write() is called *much* more often than, say, open(),... My gut feeling agrees with yours, but I do not think the frequency at which write() is called should be the primary factor when you decide to make its wrapper inlined. Once you call write(2), you will hit either the disk or the network doing I/O, and at that point I'd expect that the cost of making an extra layer of wrapper call would be lost in the noise. I'd worry a lot more about from how many callsites write() is called---by inlining the extra code that is run only when the underlying write(2) returns an error to many callsites, we would make the program as a whole bigger, and as the result other code needs to be evicted out of the instruction cache, which also would hurt performance. -- To unsubscribe from this list: send the line "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] git-svn: add support for prefixed globs in config
Introduce prefixed globs for branches and tags in git-svn.
Globs like 'release_*' allow users to avoid long lines in config like:
branches = branches/{release_20,release_21,release_22,...}
Signed-off-by: Victor Leschuk
---
Changes from v1:
* Joined implementation and test in one patch
* Fixed test code style according to current coding style guide
Documentation/git-svn.txt| 5 ++
perl/Git/SVN/GlobSpec.pm | 9 ++-
t/t9168-git-svn-prefixed-glob.sh | 136 +++
3 files changed, 145 insertions(+), 5 deletions(-)
create mode 100755 t/t9168-git-svn-prefixed-glob.sh
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 0c0f60b..529cffe 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -1034,6 +1034,7 @@ listed below are allowed:
url = http://server.org/svn
fetch = trunk/project-a:refs/remotes/project-a/trunk
branches = branches/*/project-a:refs/remotes/project-a/branches/*
+ branches = branches/release_*:refs/remotes/project-a/branches/release_*
tags = tags/*/project-a:refs/remotes/project-a/tags/*
@@ -1044,6 +1045,10 @@ independent path component (surrounded by '/' or EOL).
This
type of configuration is not automatically created by 'init' and
should be manually entered with a text-editor or using 'git config'.
+Also note that prefixed globs (e.g. 'release_*') match everything after prefix
+but do not match exact prefix. For example:
+'release_*' will match 'release_1' or 'release_v1' but will not match
'release_'.
+
It is also possible to fetch a subset of branches or tags by using a
comma-separated list of names within braces. For example:
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index c95f5d7..a136090 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -11,16 +11,15 @@ sub new {
my $die_msg = "Only one set of wildcard directories " .
"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
for my $part (split(m|/|, $glob)) {
- if ($part =~ /\*/ && $part ne "*") {
- die "Invalid pattern in '$glob': $part\n";
- } elsif ($pattern_ok && $part =~ /[{}]/ &&
+ if ($pattern_ok && $part =~ /[{}]/ &&
$part !~ /^\{[^{}]+\}/) {
die "Invalid pattern in '$glob': $part\n";
}
- if ($part eq "*") {
+ if ($part =~ /(\w*)\*/) {
die $die_msg if $state eq "right";
$state = "pattern";
- push(@patterns, "[^/]*");
+ my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+ push(@patterns, $pat);
} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
die $die_msg if $state eq "right";
$state = "pattern";
diff --git a/t/t9168-git-svn-prefixed-glob.sh b/t/t9168-git-svn-prefixed-glob.sh
new file mode 100755
index 000..b8a059b
--- /dev/null
+++ b/t/t9168-git-svn-prefixed-glob.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+cat >expect.end trunk/src/b/readme &&
+ svn_cmd import -m "initial" trunk "$svnrepo"/trunk &&
+ svn_cmd co "$svnrepo" tmp &&
+ (
+ cd tmp &&
+ mkdir branches tags &&
+ svn_cmd add branches tags &&
+ svn_cmd cp trunk branches/b_start &&
+ svn_cmd commit -m "start a new branch" &&
+ svn_cmd up &&
+ echo "hi" >>branches/b_start/src/b/readme &&
+ poke branches/b_start/src/b/readme &&
+ echo "hey" >>branches/b_start/src/a/readme &&
+ poke branches/b_start/src/a/readme &&
+ svn_cmd commit -m "hi" &&
+ svn_cmd up &&
+ svn_cmd cp branches/b_start tags/t_end &&
+ echo "bye" >>tags/t_end/src/b/readme &&
+ poke tags/t_end/src/b/readme &&
+ echo "aye" >>tags/t_end/src/a/readme &&
+ poke tags/t_end/src/a/readme &&
+ svn_cmd commit -m "the end" &&
+ echo "byebye" >>tags/t_end/src/b/readme &&
+ poke tags/t_end/src/b/readme &&
+ svn_cmd commit -m "nothing to see here"
+ ) &&
+ git config --add svn-remote.svn.url "$svnrepo" &&
+ git config --add svn-remote.svn.fetch \
+"trunk/src/a:refs/remotes/trunk" &&
+ git config --add svn-remote.svn.branches \
+"branches/b_*/src/a:refs/remotes/branches/b_*" &&
+ git config --add svn-remote.svn.tags\
+"tags/t_*/src/a:refs/remotes/tags/t_
RE: CruiseControl.NET hangs on git-submodule bad file descriptor
OK, no problem I have workaround :) Thanks a lot for your help Best wishes -- Alexander Skrinnik -Original Message- From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de] Sent: Thursday, December 17, 2015 3:55 PM To: Alexander Skrinnik Cc: git@vger.kernel.org Subject: RE: CruiseControl.NET hangs on git-submodule bad file descriptor Hi Alexander, you might want to refrain from top-posting on this list in the future. Just sayin' ;-) On Thu, 17 Dec 2015, Alexander Skrinnik wrote: > I found workaround, CC.NET invokes bat-file with command: > git submodule foreach git checkout "myBranch" > It works fine. But looks like the issue is in git. Not in CC.NET :) If > you have a chance, could you look into it, please? Unfortunately, this is one of those problems where the bug reporter expects me not only to fix the problem, but also to come up with an easy way to reproduce the issue. And even more unfortunate is the fact that I am really pressed on some other, more critical issues. Is there really no chance for you to have a crack at resolving this issue? Ciao, Johannes -- To unsubscribe from this list: send the line "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: CruiseControl.NET hangs on git-submodule bad file descriptor
Hi Alexander, you might want to refrain from top-posting on this list in the future. Just sayin' ;-) On Thu, 17 Dec 2015, Alexander Skrinnik wrote: > I found workaround, CC.NET invokes bat-file with command: > git submodule foreach git checkout "myBranch" > It works fine. But looks like the issue is in git. Not in CC.NET :) > If you have a chance, could you look into it, please? Unfortunately, this is one of those problems where the bug reporter expects me not only to fix the problem, but also to come up with an easy way to reproduce the issue. And even more unfortunate is the fact that I am really pressed on some other, more critical issues. Is there really no chance for you to have a crack at resolving this issue? Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] config: add core.untrackedCache
On Wed, Dec 16, 2015 at 4:53 AM, Ævar Arnfjörð Bjarmason wrote: > On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> I still have a problem with the approach from "design cleanliness" >> point of view[...] >> >> In any case I think we already have agreed to disagree on this >> point, so there is no use discussing it any longer from my side. I >> am not closing the door to this series, but I am not convinced, >> either. At least not yet. > > In general the fantastic thing about the git configuration facility is > that it provides both systems administrators and normal users with > what they want. It's possible to configure things system-wide and > override those on a user or repository basis. > > Of course hindsight is 20/20, but I think that given what's been > covered in this thread it's been established that it's categorically > better that if we introduce features like these that they be > configured through the normal configuration facility rather than the > configuration being sticky to the index. A minor note for implementers. We need to check that config is loaded first. read-cache.c, being part of the core, does not bother itself with config loading. And I think so far it has not used any config vars. If a command forgets (*) to load the config, the cache may be deleted (if we do it the safe way). (*) is there any command deliberately avoid loading config? git-clone and git-init are special, but for this particular case it's probably fine. -- 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: CruiseControl.NET hangs on git-submodule bad file descriptor
Hi Johannes, Thank you for your answer. Yes, this is the same issue. I found workaround, CC.NET invokes bat-file with command: git submodule foreach git checkout "myBranch" mailto:johannes.schinde...@gmx.de] Sent: Thursday, December 17, 2015 11:46 AM To: Alexander Skrinnik Cc: git@vger.kernel.org Subject: Re: CruiseControl.NET hangs on git-submodule bad file descriptor Hi Alexander, On Thu, 17 Dec 2015, Alexander Skrinnik wrote: > I had installed CruiseControl.NET 1.8.5 and Git-1.9.5-preview20150319 > CC.NET invokes bat-file which invokes git submodule foreach git > checkout "myBranch" > > It worked good. > Today I upgraded git to 2.6.4 and CC.NET fails on this command with > error: C:\Program Files > (x86)\Git\mingw32/libexec/git-core\git-submodule: line 544: 0: Bad > file descriptor This sounds very similar (maybe identical) to this bug report: https://github.com/git-for-windows/git/issues/181 Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] config: add core.untrackedCache
On Thu, Dec 17, 2015 at 2:44 PM, Jeff King wrote: > I think we may actually be thinking of the same thing. Naively, I would > expect: > > .. > - if there is cache data in the index but that config flag is not set, > presumably we would not update it (we could even explicitly drop it, > but my understanding is that is not necessary for correctness, but > only as a possible optimization). No, if somebody adds or removes something from the index, we either update or drop it, or it's stale. There's the invalidate_untracked() or something in dir.c that we can hook in, check config var and do that. And because config is cached recently, it should be a cheap operation. Apart from that the idea is fine. > You could have a config option for "if there is a cache there, pretend > it isn't and ignore it", but I don't see much point. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] update-index: use enum for untracked cache options
On 15.12.15 17:28, Christian Couder wrote:
> Signed-off-by: Christian Couder
> ---
> builtin/update-index.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 7431938..2430a68 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
> #define UNMARK_FLAG 2
> static struct strbuf mtime_dir = STRBUF_INIT;
>
> +/* Untracked cache mode */
> +enum uc_mode {
> + UC_UNSPECIFIED = -1,
> + UC_DISABLE = 0,
> + UC_ENABLE,
> + UC_FORCE
> +};
> +
> __attribute__((format (printf, 1, 2)))
> static void report(const char *fmt, ...)
> {
> @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
> int cmd_update_index(int argc, const char **argv, const char *prefix)
> {
> int newfd, entries, has_errors = 0, line_termination = '\n';
> - int untracked_cache = -1;
> + enum uc_mode untracked_cache = UC_UNSPECIFIED;
> int read_from_stdin = 0;
> int prefix_length = prefix ? strlen(prefix) : 0;
> int preferred_index_format = 0;
> @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const
> char *prefix)
> OPT_BOOL(0, "untracked-cache", &untracked_cache,
> N_("enable/disable untracked cache")),
> OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
> - N_("enable untracked cache without testing the
> filesystem"), 2),
> + N_("enable untracked cache without testing the
> filesystem"), UC_FORCE),
> OPT_END()
> };
>
> @@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv,
> const char *prefix)
> the_index.split_index = NULL;
> the_index.cache_changed |= SOMETHING_CHANGED;
> }
> - if (untracked_cache > 0) {
> + if (untracked_cache > UC_DISABLE) {
I think the "correct way" for a comparison between an enum and an int is to
cast the enum
into an int.
But, if we have an enum, these kind of comparision should go away, and replaced
with a switch-case (or if) statemant.
> struct untracked_cache *uc;
>
> - if (untracked_cache < 2) {
> + if (untracked_cache < UC_FORCE) {
Same here
--
To unsubscribe from this list: send the line "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: No auto CRLF conversion in Commit Message comments
Hi Johannes, On 16/12/2015 16:45, Johannes Schindelin wrote: However, if you have `core.editor = notepad`, it should Just Work because there is a `notepad` helper that performs the LF<->CR/LF translation transparently. The problem in my case was that I had `core.editor = notepad.exe` This wasn't triggering the helper, I've changed it, dropping the '.exe' and it now works nicely. Thanks for your help, Andy -- Andy Harfoot GeoData Institute University of Southampton Southampton SO17 1BJ Tel: +44 (0)23 8059 2719 Fax: +44 (0)23 8059 2849 www.geodata.soton.ac.uk -- To unsubscribe from this list: send the line "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: CruiseControl.NET hangs on git-submodule bad file descriptor
Hi Alexander, On Thu, 17 Dec 2015, Alexander Skrinnik wrote: > I had installed CruiseControl.NET 1.8.5 and Git-1.9.5-preview20150319 > CC.NET invokes bat-file which invokes > git submodule foreach git checkout "myBranch" > > It worked good. > Today I upgraded git to 2.6.4 and CC.NET fails on this command with > error: C:\Program Files > (x86)\Git\mingw32/libexec/git-core\git-submodule: line 544: 0: Bad file > descriptor This sounds very similar (maybe identical) to this bug report: https://github.com/git-for-windows/git/issues/181 Ciao, Johannes -- To unsubscribe from this list: send the line "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 10/11] ref-filter: introduce contents_atom_parser()
On Wed, Dec 16, 2015 at 10:30 AM, Karthik Nayak wrote:
> Introduce contents_atom_parser() which will parse the '%(contents)'
> atom and store information into the 'used_atom' structure based on the
> modifiers used along with the atom.
>
> Helped-by: Ramsay Jones
> Signed-off-by: Karthik Nayak
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -39,6 +39,10 @@ static struct used_atom {
> struct align align;
> enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
> remote_ref;
> + struct {
> + enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
> C_SUB } option;
> + unsigned int no_lines;
'no_lines' sounds like "no lines!". How about 'nlines' instead?
> + } contents;
> } u;
> } *used_atom;
> static int used_atom_cnt, need_tagged, need_symref;
> @@ -90,6 +94,36 @@ static void remote_ref_atom_parser(struct used_atom *atom)
> +static void contents_atom_parser(struct used_atom *atom)
> +{
> + const char * buf;
> +
> + if (match_atom_name(atom->str, "contents", &buf))
> + atom->u.contents.option = C_BARE;
> + else if (match_atom_name(atom->str, "subject", &buf)) {
The original code used strcmp() and matched only "subject", however
the new code will incorrectly match both "subject" and
"subject:whatever". Therefore, you should be using strcmp() here
rather than match_atom_name().
Ditto for "body".
> + atom->u.contents.option = C_SUB;
> + return;
> + } else if (match_atom_name(atom->str, "body", &buf)) {
> + atom->u.contents.option = C_BODY_DEP;
> + return;
> + }
> + if (!buf)
> + return;
It's not easy to see that this 'if (!buf)' check relates to the
"contents" check at the very top of the if/else if/ chain since there
are entirely unrelated checks in between. Reorganizing it can improve
clarity:
if (!strcmp("subject")) {
...
return;
} else if (!strcmp("body")) {
...
return;
} else if (!match_atom_name(...,"contents", &buf))
die("BUG: expected 'contents' or 'contents:'");
if (!buf) {
atom->u.contents.option = C_BARE;
return;
}
> + if (!strcmp(buf, "body"))
> + atom->u.contents.option = C_BODY;
> + else if (!strcmp(buf, "signature"))
> + atom->u.contents.option = C_SIG;
> + else if (!strcmp(buf, "subject"))
> + atom->u.contents.option = C_SUB;
> + else if (skip_prefix(buf, "lines=", &buf)) {
> + atom->u.contents.option = C_LINES;
> + if (strtoul_ui(buf, 10, &atom->u.contents.no_lines))
> + die(_("positive value expected contents:lines=%s"),
> buf);
> + } else
> + die(_("unrecognized %%(contents) argument: %s"), buf);
> +}
> @@ -777,28 +801,23 @@ static void grab_sub_body_contents(struct atom_value
> *val, int deref, struct obj
> &bodypos, &bodylen, &nonsiglen,
> &sigpos, &siglen);
>
> - if (!strcmp(name, "subject"))
> + if (atom->u.contents.option == C_SUB)
> v->s = copy_subject(subpos, sublen);
> - else if (!strcmp(name, "contents:subject"))
> - v->s = copy_subject(subpos, sublen);
> - else if (!strcmp(name, "body"))
> + else if (atom->u.contents.option == C_BODY_DEP)
> v->s = xmemdupz(bodypos, bodylen);
> - else if (!strcmp(name, "contents:body"))
> + else if (atom->u.contents.option == C_BODY)
> v->s = xmemdupz(bodypos, nonsiglen);
> - else if (!strcmp(name, "contents:signature"))
> + else if (atom->u.contents.option == C_SIG)
> v->s = xmemdupz(sigpos, siglen);
> - else if (!strcmp(name, "contents"))
> - v->s = xstrdup(subpos);
> - else if (skip_prefix(name, "contents:lines=", &valp)) {
> + else if (atom->u.contents.option == C_LINES) {
> struct strbuf s = STRBUF_INIT;
> const char *contents_end = bodylen + bodypos - siglen;
>
> - if (strtoul_ui(valp, 10, &v->u.contents.lines))
> - die(_("positive value expected
> contents:lines=%s"), valp);
> /* Size is the length of the message after removing
> the signature */
> - append_lines(&s, subpos, contents_end - subpos,
> v->u.contents.lines);
> + append_lines(&s, subpos, contents_end - subpos,
> atom->u.contents.no_lines);
> v->s = strbuf_detach(&s, NULL);
> - }
> + } else if (atom->u.contents.
Re: [PATCH] mingw: emulate write(2) that fails with a EPIPE
Hi Junio,
On Wed, 16 Dec 2015, Junio C Hamano wrote:
> Johannes Schindelin writes:
>
> > int mingw_fflush(FILE *stream);
> > #define fflush mingw_fflush
> >
> > +static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> > +{
> > + ssize_t result = write(fd, buf, len);
> > +
> > + if (result < 0 && errno == EINVAL && buf) {
> > + /* check if fd is a pipe */
> > + HANDLE h = (HANDLE) _get_osfhandle(fd);
> > + if (GetFileType(h) == FILE_TYPE_PIPE)
> > + errno = EPIPE;
> > + else
> > + errno = EINVAL;
> > + }
> > +
> > + return result;
> > +}
> > +
> > +#define write mingw_write
> > +
>
> It strikes me a bit strange to see this inlined compared to what
> appears in the context. Shouldn't the implementation be done in
> compat/mingw.c like all others?
My intuition (which I honestly did not verify using performance tests) was
that write() is called *much* more often than, say, open(), and therefore
I wanted to interfere as little as possible with the performance penalty.
Hence the choice of an inlined function as opposed to a non-optimizable
increment of the call chain.
If it bothers you a lot I will set aside time to perform performance
tests.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "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] mingw: emulate write(2) that fails with a EPIPE
Hi Eric,
On Wed, 16 Dec 2015, Eric Sunshine wrote:
> On Wednesday, December 16, 2015, Johannes Schindelin
> wrote:
> > diff --git a/compat/mingw.h b/compat/mingw.h
> > @@ -210,6 +210,24 @@ FILE *mingw_freopen (const char *filename, const char
> > *otype, FILE *stream);
> > +static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> > +{
> > + ssize_t result = write(fd, buf, len);
> > +
> > + if (result < 0 && errno == EINVAL && buf) {
>
> Here, errno is EINVAL...
>
> > + /* check if fd is a pipe */
> > + HANDLE h = (HANDLE) _get_osfhandle(fd);
> > + if (GetFileType(h) == FILE_TYPE_PIPE)
> > + errno = EPIPE;
> > + else
> > + errno = EINVAL;
>
> Does any of the code between the outer 'if' and this point clobber
> errno, or are you merely assigning EINVAL for robustness against
> future changes?
Yes, it is proofing the code against future changes. And TBH I did not
even bother to check whether _get_osfhandle() or GetFileType() can modify
the errno value, since I *really* wanted to make sure that errno is either
EPIPE or EINVAL in this execution path.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "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 09/11] ref-filter: introduce remote_ref_atom_parser()
On Wed, Dec 16, 2015 at 10:30 AM, Karthik Nayak wrote:
> Introduce remote_ref_atom_parser() which will parse the '%(upstream)'
> and '%(push)' atoms and store information into the 'used_atom'
> structure based on the modifiers used along with the corresponding
> atom.
>
> Helped-by: Ramsay Jones
> Signed-off-by: Karthik Nayak
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -37,6 +37,8 @@ static struct used_atom {
> union {
> const char *color;
> struct align align;
> + enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
Nit: I'd have expected to see the normal/plain case first rather than
last (but not itself worth a re-roll).
> + remote_ref;
> } u;
> } *used_atom;
> static int used_atom_cnt, need_tagged, need_symref;
> @@ -69,6 +71,25 @@ static void color_atom_parser(struct used_atom *atom)
> +static void remote_ref_atom_parser(struct used_atom *atom)
> +{
> + const char *buf;
> +
> + buf = strchr(atom->str, ':');
> + atom->u.remote_ref = RR_NORMAL;
> + if (!buf)
> + return;
This code is not as clear as it could be due to the way the 'buf'
assignment and check for NULL are split apart. It can be made clearer
either by doing this:
atom->u.remote_ref = RR_NORMAL;
buf = strchr(...);
if (!buf)
return;
or (even better) this:
buf = strchr(...);
if (!buf) {
atom->u.remote_ref = RR_NORMAL;
return;
}
> + buf++;
> + if (!strcmp(buf, "short"))
> + atom->u.remote_ref = RR_SHORTEN;
> + else if (!strcmp(buf, "track"))
> + atom->u.remote_ref = RR_TRACK;
> + else if (!strcmp(buf, "trackshort"))
> + atom->u.remote_ref = RR_TRACKSHORT;
> + else
> + die(_("unrecognized format: %%(%s)"), atom->str);
> +}
> +
> @@ -835,6 +856,42 @@ static inline char *copy_advance(char *dst, const char
> *src)
> +static void fill_remote_ref_details(struct used_atom *atom, const char
> *refname,
> + struct branch *branch, const char **s)
> +{
> + int num_ours, num_theirs;
> + if (atom->u.remote_ref == RR_SHORTEN)
> + *s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
> + else if (atom->u.remote_ref == RR_TRACK) {
> + if (stat_tracking_info(branch, &num_ours,
> + &num_theirs, NULL))
> + return;
The RR_TRACKSHORT branch below has a blank line following the
'return', but this branch lacks it, which is inconsistent.
> + if (!num_ours && !num_theirs)
> + *s = "";
> + else if (!num_ours)
> + *s = xstrfmt("[behind %d]", num_theirs);
> + else if (!num_theirs)
> + *s = xstrfmt("[ahead %d]", num_ours);
> + else
> + *s = xstrfmt("[ahead %d, behind %d]",
> +num_ours, num_theirs);
> + } else if (atom->u.remote_ref == RR_TRACKSHORT) {
> + if (stat_tracking_info(branch, &num_ours,
> + &num_theirs, NULL))
What happened to the assert(branch) which was in the original code
from which this was derived (below)? Is it no longer needed?
> + return;
> +
> + if (!num_ours && !num_theirs)
> + *s = "=";
> + else if (!num_ours)
> + *s = "<";
> + else if (!num_theirs)
> + *s = ">";
> + else
> + *s = "<>";
> + } else if (atom->u.remote_ref == RR_NORMAL)
> + *s = refname;
> +}
> +
> /*
> * Parse the object referred by ref, and grab needed value.
> */
> @@ -889,6 +946,8 @@ static void populate_value(struct ref_array_item *ref)
> refname = branch_get_upstream(branch, NULL);
> if (!refname)
> continue;
> + fill_remote_ref_details(atom, refname, branch, &v->s);
> + continue;
> } else if (starts_with(name, "push")) {
> const char *branch_name;
> if (!skip_prefix(ref->refname, "refs/heads/",
> @@ -899,6 +958,8 @@ static void populate_value(struct ref_array_item *ref)
> refname = branch_get_push(branch, NULL);
> if (!refname)
> continue;
> + fill_remote_ref_details(atom, refname, branch, &v->s);
> + continue;
> } else if (starts_with(name, "color:")) {
> char color[COLOR_MAXLEN] = "";
>
> @@ -944,49 +1005,11 @@ static void populate_value(struct ref_array_item *ref)
>
> format
Re: [PATCH v2 08/11] ref-filter: introduce prefixes for the align atom
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak wrote:
> ref-filter: introduce prefixes for the align atom
The prefixes are actually for the arguments to the 'align' atom, not
for the atom itself. However, it might be better to describe this at a
bit higher level. Perhaps:
ref-filter: align: introduce long-form syntax
or something.
> Introduce optional prefixes "width=" and "position=" for the align atom
> so that the atom can be used as "%(align:width=,position=)".
>
> Add Documetation and tests for the same.
>
> Signed-off-by: Karthik Nayak
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -96,10 +96,19 @@ static void align_atom_parser(struct used_atom *atom)
>
> while (*s) {
> int position;
> + buf = s[0]->buf;
It probably would be better to do this assignment in the previous
patch (7/11) since its presence here introduces unwanted noise
(textual replacement of "s[0]->buf" with "buf") in several locations
below which slightly obscure the real changes of this patch.
> - if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&width))
> + if (skip_prefix(buf, "position=", &buf)) {
> + position = parse_align_position(buf);
> + if (position == -1)
It may be more idiomatic in this codebase to detect errors via '< 0'
rather than explicit '== -1'. Ditto below.
> + die(_("unrecognized position:%s"), buf);
> + align->position = position;
> + } else if (skip_prefix(buf, "width=", &buf)) {
> + if (strtoul_ui(buf, 10, (unsigned int *)&width))
> + die(_("unrecognized width:%s"), buf);
> + } else if (!strtoul_ui(buf, 10, (unsigned int *)&width))
> ;
> - else if ((position = parse_align_position(s[0]->buf)) > 0)
> + else if ((position = parse_align_position(buf)) != -1)
> align->position = position;
> else
> die(_("unrecognized %%(align) argument: %s"),
> s[0]->buf);
s/s[0]->//
More below...
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> @@ -133,6 +133,168 @@ test_expect_success 'right alignment' '
> test_cmp expect actual
> '
>
> +test_expect_success 'alignment with "position" prefix' '
> + cat >expect <<-\EOF &&
> + | refname is refs/heads/master|refs/heads/master
> + |refname is refs/heads/side|refs/heads/side
> + | refname is refs/odd/spot|refs/odd/spot
> + |refname is refs/tags/double-tag|refs/tags/double-tag
> + | refname is refs/tags/four|refs/tags/four
> + | refname is refs/tags/one|refs/tags/one
> + |refname is refs/tags/signed-tag|refs/tags/signed-tag
> + |refname is refs/tags/three|refs/tags/three
> + | refname is refs/tags/two|refs/tags/two
> + EOF
> + git for-each-ref --format="|%(align:30,position=right)refname is
> %(refname)%(end)|%(refname)" >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'alignment with "position" prefix' '
> + cat >expect <<-\EOF &&
> + | refname is refs/heads/master|refs/heads/master
> + |refname is refs/heads/side|refs/heads/side
> + | refname is refs/odd/spot|refs/odd/spot
> + |refname is refs/tags/double-tag|refs/tags/double-tag
> + | refname is refs/tags/four|refs/tags/four
> + | refname is refs/tags/one|refs/tags/one
> + |refname is refs/tags/signed-tag|refs/tags/signed-tag
> + |refname is refs/tags/three|refs/tags/three
> + | refname is refs/tags/two|refs/tags/two
> + EOF
> + git for-each-ref --format="|%(align:position=right,30)refname is
> %(refname)%(end)|%(refname)" >actual &&
> + test_cmp expect actual
> +'
This (and below) is a lot of copy/paste code which makes it difficult
to read the tests and maintain (change) them. Since 'expect' doesn't
appear to change from test to test, one way to eliminate some of this
noisiness would be to create 'expect' once outside of the tests.
However, even better, especially from a comprehension,
maintainability, and extensibility standpoints would be to make this
all table-driven. In particular, I'd expect to see a table with
exactly the list of test inputs mentioned in my earlier review[1], and
have that table passed to a shell function which performs the test for
each element of the table. For instance, something like:
test_align_permutations <<-\EOF
middle,42
42,middle
position=middle,42
42,position=middle
middle,width=42
width=42,middle
position=middle,width=42
width=42,position=middle
EOF
where test_align_permutations is the name of the shell function which
reads each line of it stdin and performs the "git for-each-
Re: [PATCH v2 01/11] strbuf: introduce strbuf_split_str_without_term()
On Thu, Dec 17, 2015 at 2:05 AM, Eric Sunshine wrote:
> On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak wrote:
>> strbuf: introduce strbuf_split_str_without_term()
>
> s/without/omit/
>
>> The current implementation of 'strbuf_split_buf()' includes the
>> terminator at the end of each strbuf post splitting. Add an option
>> wherein we can drop the terminator if desired. In this context
>> introduce a wrapper function 'strbuf_split_str_without_term()' which
>
> s/without/omit/
>
Oops! will change that,
>> splits a given string into strbufs without including the terminator.
>>
>> Signed-off-by: Karthik Nayak
>> ---
>> diff --git a/strbuf.c b/strbuf.c
>> @@ -115,7 +115,7 @@ void strbuf_tolower(struct strbuf *sb)
>> struct strbuf **strbuf_split_buf(const char *str, size_t slen,
>> -int terminator, int max)
>> +int terminator, int max, int omit_term)
>> {
>> struct strbuf **ret = NULL;
>> size_t nr = 0, alloc = 0;
>> @@ -123,14 +123,15 @@ struct strbuf **strbuf_split_buf(const char *str,
>> size_t slen,
>>
>> while (slen) {
>> int len = slen;
>> + const char *end = NULL;
>> if (max <= 0 || nr + 1 < max) {
>> - const char *end = memchr(str, terminator, slen);
>> + end = memchr(str, terminator, slen);
>> if (end)
>> len = end - str + 1;
>> }
>> t = xmalloc(sizeof(struct strbuf));
>> strbuf_init(t, len);
>> - strbuf_add(t, str, len);
>> + strbuf_add(t, str, len - !!end * !!omit_term);
>
> This version of the patch with its minimal[1] change is much easier to
> review for correctness than the original attempt[2].
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/281882
> [2]: http://article.gmane.org/gmane.comp.version-control.git/281189
>
>> ALLOC_GROW(ret, nr + 2, alloc);
>> ret[nr++] = t;
>> str += len;
>> diff --git a/strbuf.h b/strbuf.h
>> index 6ae7a72..a865a74 100644
>> --- a/strbuf.h
>> +++ b/strbuf.h
>> @@ -450,11 +450,12 @@ static inline int strbuf_strip_suffix(struct strbuf
>> *sb, const char *suffix)
>> /**
>> * Split str (of length slen) at the specified terminator character.
>> * Return a null-terminated array of pointers to strbuf objects
>> - * holding the substrings. The substrings include the terminator,
>> - * except for the last substring, which might be unterminated if the
>> - * original string did not end with a terminator. If max is positive,
>> - * then split the string into at most max substrings (with the last
>> - * substring containing everything following the (max-1)th terminator
>> + * holding the substrings. The substrings include the terminator if
>> + * the value of omit_term is '0' else the terminator is excluded. The
>
> Perhaps just say "if omit_term is false" rather than "if the value of ... is
> 0".
>
>> + * last substring, might be unterminated if the original string did
>> + * not end with a terminator. If max is positive, then split the
>
> This bit about the last substring is perhaps too disconnected from the
> previous sentence. What if you re-word the entire thing something like
> this:
>
> If omit_term is true, the terminator will be stripped from all
> substrings. Otherwise, substrings will include the terminator,
> except for the final substring, if the original string lacked a
> terminator.
>
Im quite bad at this, thanks :)
>>
>> This version of the patch with its minimal[1] change is much easier to
>> review for correctness than the original attempt[2].
>
> Perhaps worthy of a Helped-by:?
Definitely! Sorry for not including that, slipped off my mind.
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
CruiseControl.NET hangs on git-submodule bad file descriptor
Hi guys, I had installed CruiseControl.NET 1.8.5 and Git-1.9.5-preview20150319 CC.NET invokes bat-file which invokes git submodule foreach git checkout "myBranch" It worked good. Today I upgraded git to 2.6.4 and CC.NET fails on this command with error: C:\Program Files (x86)\Git\mingw32/libexec/git-core\git-submodule: line 544: 0: Bad file descriptor When I run the same command from cmd window all works without error For now I have to return to 1.9.5 version. Could you, please, help to fix the issue? Thank you in advance -- Alexander Skrinnik -- To unsubscribe from this list: send the line "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: [PATCHv2] submodule: Port resolve_relative_url from shell to C
A rather superficial review...
On Wed, Dec 16, 2015 at 7:26 PM, Stefan Beller wrote:
> This reimplements the helper function `resolve_relative_url` in shell
s/This reimplements/Reimplement/
> in C. This functionality is needed in C for introducing the groups
> feature later on. When using groups, the user should not need to run
> `git submodule init`, but it should be implicit at all appropriate places,
> which are all in C code. As the we would not just call out to `git
I guess you mean "As then we..." or something?
> submodule init`, but do a more fine grained structure there, we actually
> need all the init functionality in C before attempting the groups
> feature. To get the init functionality in C, rewriting the
> resolve_relative_url subfunction is a major step.
>
> Signed-off-by: Stefan Beller
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -9,6 +9,156 @@
> +static const char *get_default_remote(void)
> +{
> + char *dest = NULL;
> + unsigned char sha1[20];
> + int flag;
> + struct strbuf sb = STRBUF_INIT;
> + const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
> +
> + if (!refname)
> + die("No such ref: HEAD");
> +
> + refname = shorten_unambiguous_ref(refname, 0);
> + strbuf_addf(&sb, "branch.%s.remote", refname);
> + if (git_config_get_string(sb.buf, &dest))
> + return "origin";
> + else
> + return xstrdup(dest);
Leaking the strbuf at both returns.
And, leaking the strdup'd dest (in the caller), but I suppose that's
intentional?
> +}
> +
> +static int has_same_dir_prefix(const char *str, const char **out)
> +{
> +#ifdef GIT_WINDOWS_NATIVE
> + return skip_prefix(str, "./", out)
> + || skip_prefix(str, ".\\", out);
> +#else
> + return skip_prefix(str, "./", out);
> +#endif
> +}
> +
> +static int has_upper_dir_prefix(const char *str, const char **out)
> +{
> +#ifdef GIT_WINDOWS_NATIVE
> + return skip_prefix(str, "../", out)
> + || skip_prefix(str, "..\\", out);
> +#else
> + return skip_prefix(str, "../", out);
> +#endif
> +}
> +
> +static char *last_dir_separator(const char *str)
> +{
> +#ifdef GIT_WINDOWS_NATIVE
> + return strrchr(str, "/")
> + || strrchr(str, "\\");
> +#else
> + return strrchr(str, '/');
> +#endif
> +}
Cleaner would be to move the #if's outside the functions:
#ifdef GIT_WINDOWS_NATIVE
/* Windows implementations */
static int has_same_dir_prefix(...) {...}
static int has_upper_dir_prefix(...) {...}
static char *last_dir_separator(...) {...}
#else
/* POSIX implementations */
static int has_same_dir_prefix(...) {...}
static int has_upper_dir_prefix(...) {...}
static char *last_dir_separator(...) {...}
#endif
> +static const char *relative_url(const char *url, const char *up_path)
> +{
> + int is_relative = 0;
> + size_t len;
> + char *remoteurl = NULL;
> + char *sep = "/";
'sep' only ever holds a single character, so why not declare it 'char'
rather than 'char *'? (And, adjust the format string of strbuf_addf(),
of course.)
> + const char *out;
> + [...]
> + while (url) {
> + if (has_upper_dir_prefix(url, &out)) {
> + [...]
> + if (rfind)
> + *rfind = '\0';
> + else {
> + rfind = strrchr(remoteurl, ':');
> + if (rfind) {
> + *rfind = '\0';
> + sep = ":";
> + } else {
> + [...]
> + }
> + }
> + } else if (has_same_dir_prefix(url, &out))
> + url = out;
> + else
> + break;
> + }
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
> +
> + if (!has_same_dir_prefix(sb.buf, &out))
> + out = sb.buf;
> + out = xstrdup(out);
> +
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s%s", is_relative && up_path ? up_path : "", out);
> +
> + free((char*)out);
Why declare 'out' const if you know you will be freeing it?
> + return strbuf_detach(&sb, NULL);
> +}
> +
> +static int resolve_relative_url(int argc, const char **argv, const char
> *prefix)
> +{
> + if (argc == 2)
> + printf("%s\n", relative_url(argv[1], NULL));
> + else if (argc == 3)
> + printf("%s\n", relative_url(argv[1], argv[2]));
> + else
> + die("BUG: resolve_relative_url only accepts one or two
> arguments");
> + return 0;
> +}
>
> struct module_list {
> const struct cache_entry **entries;
> @@ -264,6 +414,7 @@ st
