Re: [PATCH v4] clone: simplify string handling in guess_dir_name()

2015-08-03 Thread Lukas Fleischer
On Thu, 09 Jul 2015 at 20:24:08, Sebastian Schuberth wrote:
> Signed-off-by: Sebastian Schuberth 
> ---
>  builtin/clone.c | 17 +
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 00535d0..ebcb849 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int 
> *is_bundle)
>  static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
>  {
> const char *end = repo + strlen(repo), *start;
> +   size_t len;
> char *dir;
>  
> /*
> @@ -173,19 +174,11 @@ static char *guess_dir_name(const char *repo, int 
> is_bundle, int is_bare)
> /*
>  * Strip .{bundle,git}.
>  */
> -   if (is_bundle) {
> -   if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
> -   end -= 7;
> -   } else {
> -   if (end - start > 4 && !strncmp(end - 4, ".git", 4))
> -   end -= 4;
> -   }
> +   strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
> [...]

I am currently on vacation and cannot bisect or debug this but I am
pretty confident that this patch changes the behaviour of directory name
guessing. With Git 2.4.6, cloning http://foo.bar/foo.git/ results in a
directory named foo and with Git 2.5.0, the resulting directory is
called foo.git.

Note how the end variable is decreased when the repository name ends
with a slash but that isn't taken into account when simply using
strip_suffix() later...

Is this intended?
--
To unsubscribe from this list: send the line "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: Question: .idx without .pack causes performance issues?

2015-08-03 Thread Junio C Hamano
Doug Kelly  writes:

> Here's a change to prune.c that at least addresses the issue by removing
> .idx files without an associated pack, but it's by no means pretty.  If anyone
> has any feedback before I turn this into a formal patch, it's more than 
> welcome!

I'd hesitate to see removal of a file (for that matter, a creation
too) inside a "while (de = readdir)" loop.  As the original function
is about temporary files, and the new thing is not about temporary
files at all, I'd further prefer that we do not do it in the same
loop.

I am wondering if we can add a new mode to report_pack_garbage() in
sha1_file.c to allow it to remove stale and lone ".idx".  Most of
the time we are accessing packs read-only, and I do not want the
function to unconditionally remove lone ".idx", but perhaps we
can teach "prune" to set a custom report_garbage() routine and
react to a call to its custom report_garbage()?

Perhaps that custom report_garbage() can make a list of ".idx"
files, iterate over it to pick the lone one without ".pack" and
remove them.  Or the custom report_garbage() can make a list of lone
".idx" files, if you tweak the interface to report_garbage() to
contain th seen_bits value, avoiding the need to check the existence
of ".pack" for the second time.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2015, #01; Mon, 3)

2015-08-03 Thread Junio C Hamano
Paul Tan  writes:

> I'm thinking about leaving them broken for now to push this patch
> series forward,...

Options upon restarting was not working at all with the old code
anyway, so that course of action sounds sensible.

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: What's cooking in git.git (Aug 2015, #01; Mon, 3)

2015-08-03 Thread Junio C Hamano
Eric Sunshine  writes:

> On Mon, Aug 3, 2015 at 6:18 PM, Junio C Hamano  wrote:
>> I've kicked a few topics out of 'next' back to 'pu' for now.
>>  - I think es/worktree-add and es/worktree-add-cleanup are good
>>shape overall, but we probably would want to make them into a
>>single topic.
>
> Is there anything I need to do to move this along, or is it something
> you will be handling locally?

Unless you view this as a chance to rebuild the topic and want to
take advantage of it, e.g. "I'd like to reorder and replace 4-7 with
these two patches while it is outside 'next'", there is no need to
do anything on your part (this comment also applies to Paul).  I
just wanted to eyeball the result after I twiddled a few topics with
"maybe squash this in?" patches we saw recently before merging them
back into 'next'.

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 v2] remote: add get-url subcommand

2015-08-03 Thread Eric Sunshine
On Mon, Aug 3, 2015 at 8:16 PM, Ben Boeckel  wrote:
> On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
>> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel  wrote:
>> > +   argc = parse_options(argc, argv, NULL, options, 
>> > builtin_remote_geturl_usage,
>> > +PARSE_OPT_KEEP_ARGV0);
>>
>> What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?
>
> Copied from get-url; I presume for more natural argv[] usage within the
> function.
>
>> > +   if (argc < 1 || argc > 2)
>> > +   usage_with_options(builtin_remote_geturl_usage, options);
>>
>> So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).
>>
>> > +   remotename = argv[1];
>>
>> But here, argv[1] is accessed unconditionally, even though 'argc' may
>> have been 1, thus out of bounds.
>
> Yep, should be (argc < 2 || argc > 2) (or 1 if PARSE_OPT_KEEP_ARGV0 is
> removed). Off-by-one when converting from get-url.

Or, expressed more naturally:

if (argc != 1)
usage_with_options(...);

assuming the unnecessary PARSE_OPT_KEEP_ARGV0 is dropped.

> I'll reroll tomorrow morning in case there are more comments until then
> (particularly about PARSE_OPT_KEEP_ARGV0).

This new code doesn't take advantage of it, and it's very rarely used
in Git itself, thus its use here is a potential source of confusion,
so it's probably best to drop it. (The same could be said for
set_url(), but that's a separate topic.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2015, #01; Mon, 3)

2015-08-03 Thread Paul Tan
On Tue, Aug 4, 2015 at 6:18 AM, Junio C Hamano  wrote:
>  - I think pt/am-builtin is more or less ready, but the fix to the
>issue that options given to 'git am' when restarting were
>rejected must be queued on that topic before we can start
>thinking of merging it to 'master' for the next release.

Yeah, sorry for the lack of updates, but I'm working on it[1]. I've
added a patch that adds stdin pty support to test_terminal, and made
--signoff work when resuming.

However, it turns out that --keep, --message-id and --scissors do not
work as well, as they apply to the mail-parsing stage (git-mailinfo),
which comes before the patch application stage. It's tricky, as their
functionality is locked inside builtin/mailinfo.c. :-/

I'm thinking about leaving them broken for now to push this patch
series forward, until the future where the git-mailinfo functionality
gets moved into libgit.a or something so we can access the individual
functions directly and work something out.

Will send a re-roll once I look over the patches with a fresh set of eyes.

[1] 
https://github.com/pyokagan/git/compare/pt/builtin-am...pt/am-resume-override-opts

Thanks,
Paul
--
To unsubscribe from this list: send the line "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] remote: add get-url subcommand

2015-08-03 Thread Ben Boeckel
On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel  wrote:
> > +   OPT_BOOL('\0', "push", &push_mode,
> > +N_("query push URLs")),
> 
> A bit more explanatory:
> 
> "query push URLs rather than fetch URLs"

Fixed.

> > +   OPT_BOOL('\0', "all", &all_mode,
> > +N_("return all URLs")),
> > +   OPT_END()
> > +   };
> > +   argc = parse_options(argc, argv, NULL, options, 
> > builtin_remote_geturl_usage,
> > +PARSE_OPT_KEEP_ARGV0);
> 
> What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?

Copied from get-url; I presume for more natural argv[] usage within the
function.

> > +   if (argc < 1 || argc > 2)
> > +   usage_with_options(builtin_remote_geturl_usage, options);
> 
> So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).

…

> > +   remotename = argv[1];
> 
> But here, argv[1] is accessed unconditionally, even though 'argc' may
> have been 1, thus out of bounds.

Yep, should be (argc < 2 || argc > 2) (or 1 if PARSE_OPT_KEEP_ARGV0 is
removed). Off-by-one when converting from get-url.

I'll reroll tomorrow morning in case there are more comments until then
(particularly about PARSE_OPT_KEEP_ARGV0).

Thanks,

--Ben
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/10] object_id part 2

2015-08-03 Thread brian m. carlson
On Mon, Aug 03, 2015 at 01:33:56AM +0200, Michael Haggerty wrote:
> Brian, what was your experience when writing these patches? Did they
> tend to work as soon as they compiled without errors (i.e., not super
> risky) or did you often have test suite failures that you had to go back
> and fix (i.e., risky)? If the latter, what kinds of code patterns tended
> to be problematic? Your answers might help reviewers decide how much
> diligence is needed when reviewing these patches and what kind of
> changes to inspect extra carefully. Because doing a thorough review of
> all of the patches would be quite a bit of work.

In this particular branch, I think I may have had one bad patch.  (I'm
trying to recall because I've been working on patches for part 3 in the
mean time, and they all seem to group together.)  In general, over all
my patches, the conversions I've had to fix the most have been the ones
to use the GIT_SHA1_* constants, because it's very easy to get
off-by-one errors in there or mess up the values such that things break.
Extra effort on those, or additional suggestions on how to make them
cleaner and less brittle, both now and in the future, would be welcome.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2] remote: add get-url subcommand

2015-08-03 Thread Eric Sunshine
On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel  wrote:
> Expanding `insteadOf` is a part of ls-remote --url and there is no way
> to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
> to query both as well as a way to get all configured urls.
>
> Signed-off-by: Ben Boeckel 
> ---
> diff --git a/builtin/remote.c b/builtin/remote.c
> index f4a6ec9..9278a83 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -1497,6 +1503,53 @@ static int set_branches(int argc, const char **argv)
> +static int get_url(int argc, const char **argv)
> +{
> +   int i, push_mode = 0, all_mode = 0;
> +   const char *remotename = NULL;
> +   struct remote *remote;
> +   const char **url;
> +   int url_nr;
> +   struct option options[] = {
> +   OPT_BOOL('\0', "push", &push_mode,
> +N_("query push URLs")),

A bit more explanatory:

"query push URLs rather than fetch URLs"

> +   OPT_BOOL('\0', "all", &all_mode,
> +N_("return all URLs")),
> +   OPT_END()
> +   };
> +   argc = parse_options(argc, argv, NULL, options, 
> builtin_remote_geturl_usage,
> +PARSE_OPT_KEEP_ARGV0);

What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?

> +   if (argc < 1 || argc > 2)
> +   usage_with_options(builtin_remote_geturl_usage, options);

So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).

> +   remotename = argv[1];

But here, argv[1] is accessed unconditionally, even though 'argc' may
have been 1, thus out of bounds.

> +   if (!remote_is_configured(remotename))
> +   die(_("No such remote '%s'"), remotename);
> +   remote = remote_get(remotename);
> +
> +   if (push_mode) {
> +   url = remote->pushurl;
> +   url_nr = remote->pushurl_nr;
> +   } else {
> +   url = remote->url;
> +   url_nr = remote->url_nr;
> +   }
> +
> +   if (!url_nr)
> +   die(_("no URLs configured for remote '%s'"), remotename);
> +
> +   if (all_mode) {
> +   for (i = 0; i < url_nr; i++)
> +   printf_ln("%s", url[i]);
> +   } else {
> +   printf_ln("%s", *url);
> +   }
> +
> +   return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread David Turner
On Tue, 2015-08-04 at 06:09 +0700, Duy Nguyen wrote:
> On Tue, Aug 4, 2015 at 2:49 AM, David Turner  wrote:
> > Simply treating refs/worktree as per-worktree, while the rest of refs/
> > is not, would be a few dozen lines of code.  The full remapping approach
> > is likely to be a lot more. I've already got the lmdb backend working
> > with something like this approach.  If we decide on a complicated
> > approach, I am likely to run out of time to work on pluggable backends.
> 
> I think you still have another option: decide that lmdb backend does
> not (yet) support multiple worktrees (which means make "git worktree
> add" reject when lmdb backend is used). That would simplify some for
> you and we can continue on at a later time.

Some of our developers are pretty excited about multiple worktrees, so I
don't really want to do that.  Also, it's easier to develop when more of
the tests pass under the LMDB backend (no need to investigate whether
worktrees are the reason for failures when there are no failures).



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread Duy Nguyen
On Tue, Aug 4, 2015 at 2:49 AM, David Turner  wrote:
> Simply treating refs/worktree as per-worktree, while the rest of refs/
> is not, would be a few dozen lines of code.  The full remapping approach
> is likely to be a lot more. I've already got the lmdb backend working
> with something like this approach.  If we decide on a complicated
> approach, I am likely to run out of time to work on pluggable backends.

I think you still have another option: decide that lmdb backend does
not (yet) support multiple worktrees (which means make "git worktree
add" reject when lmdb backend is used). That would simplify some for
you and we can continue on at a later time.
-- 
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 v5 1/2] worktrees: add find_shared_symref

2015-08-03 Thread Eric Sunshine
On Mon, Aug 3, 2015 at 2:48 PM, David Turner  wrote:
> Add a new function, find_shared_symref, which contains the heart of
> die_if_checked_out, but works for any symref, not just HEAD.  Refactor
> die_if_checked_out to use the same infrastructure as
> find_shared_symref.
>
> Soon, we will use find_shared_symref to protect notes merges in
> worktrees.

A couple comments below. The first may be worth a re-roll; the second
is more a taste thing. Neither is blocking if you don't happen to
re-roll.

> Signed-off-by: David Turner 
> ---
> diff --git a/branch.c b/branch.c
> index c85be07..d2b3586 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -349,31 +350,53 @@ static void check_linked_checkout(const char *branch, 
> const char *id)
> strbuf_addstr(&gitdir, get_git_common_dir());
> skip_prefix(branch, "refs/heads/", &branch);
> strbuf_strip_suffix(&gitdir, ".git");
> -   die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
> +
> +   strbuf_release(&path);
> +   strbuf_release(&sb);
> +   return strbuf_detach(&gitdir, NULL);
>  done:
> strbuf_release(&path);
> strbuf_release(&sb);
> strbuf_release(&gitdir);
> +
> +   return NULL;
>  }

This would be cleaner and less redundant if you assign the existing
location to a variable and just fall through to the 'done' label:

char *existing = NULL;
...
skip_prefix(branch, "refs/heads/", &branch);
strbuf_strip_suffix(&gitdir, ".git");
existing = strbuf_detach(&gitdir, NULL);
done:
strbuf_release(&path);
strbuf_release(&sb);
strbuf_release(&gitdir);
return existing;

There's no worry that the "existing" path will be clobbered by
strbuf_release(&gitdir) since it's been detached already (and it's
safe to release the strbuf without affecting what has been detached
from it).

> -void die_if_checked_out(const char *branch)
> +char *find_shared_symref(const char *symref, const char *target)
>  {
> struct strbuf path = STRBUF_INIT;
> DIR *dir;
> struct dirent *d;
> +   char *existing;
>
> -   check_linked_checkout(branch, NULL);
> +   if ((existing = find_linked_symref(symref, target, NULL)))
> +   return existing;
>
> strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
> dir = opendir(path.buf);
> strbuf_release(&path);
> if (!dir)
> -   return;
> +   return NULL;
>
> while ((d = readdir(dir)) != NULL) {
> if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> continue;
> -   check_linked_checkout(branch, d->d_name);
> +   existing = find_linked_symref(symref, target, d->d_name);
> +   if (existing) {
> +   closedir(dir);
> +   return existing;

For consistency with code nearby, this could have been handled by
adding a 'done' label above the closedir() below and jumping to it
from here, and then 'return existing'.

> +   }
> }
> closedir(dir);
> +
> +   return NULL;
> +}
> +
> +void die_if_checked_out(const char *branch)
> +{
> +   char *existing;
> +
> +   existing = find_shared_symref("HEAD", branch);
> +   if (existing)
> +   die(_("'%s' is already checked out at '%s'"), branch, 
> existing);
>  }
--
To unsubscribe from this list: send the line "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] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Stefan Beller
Most of the submodule operations work on a set of submodules.
Calculating and using this set is usually done via:

   module_list "$@" | {
   while read mode sha1 stage sm_path
   do
# the actual operation
   done
   }

Currently the function `module_list` is implemented in the
git-submodule.sh as a shell script wrapping a perl script.
The rewrite is in C, such that it is faster and can later be
easily adapted when other functions are rewritten in C.

git-submodule.sh similar to the builtin commands will navigate
to the top most directory of the repository and keeping the
subdirectories as a variable. As the helper is called from
within the git-submodule.sh script, we are already navigated
to the root level, but the path arguments are stil relative
to the subdirectory we were in when calling git-submodule.sh.
That's why there is a `--prefix` option pointing to an alternative
path where to anchor relative path arguments.

Signed-off-by: Stefan Beller 
---
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/submodule--helper.c | 110 
 git-submodule.sh|  54 +++---
 git.c   |   1 +
 5 files changed, 119 insertions(+), 48 deletions(-)
 create mode 100644 builtin/submodule--helper.c

diff --git a/Makefile b/Makefile
index 8c3c724..6fb7484 100644
--- a/Makefile
+++ b/Makefile
@@ -898,6 +898,7 @@ BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
 BUILTIN_OBJS += builtin/stripspace.o
+BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
 BUILTIN_OBJS += builtin/tag.o
 BUILTIN_OBJS += builtin/unpack-file.o
diff --git a/builtin.h b/builtin.h
index 9e04f97..7bf9597 100644
--- a/builtin.h
+++ b/builtin.h
@@ -118,6 +118,7 @@ extern int cmd_show(int argc, const char **argv, const char 
*prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
+extern int cmd_submodule__helper(int argc, const char **argv, const char 
*prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
new file mode 100644
index 000..fbd9568
--- /dev/null
+++ b/builtin/submodule--helper.c
@@ -0,0 +1,110 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "quote.h"
+#include "pathspec.h"
+#include "dir.h"
+#include "utf8.h"
+
+static char *ps_matched;
+static const struct cache_entry **ce_entries;
+static int ce_alloc, ce_used;
+
+static void module_list_compute(int argc, const char **argv,
+   const char *prefix,
+   struct pathspec *pathspec)
+{
+   int i;
+   char *max_prefix;
+   int max_prefix_len;
+   parse_pathspec(pathspec, 0,
+  PATHSPEC_PREFER_FULL |
+  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+  prefix, argv);
+
+   /* Find common prefix for all pathspec's */
+   max_prefix = common_prefix(pathspec);
+   max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
+
+   if (pathspec->nr)
+   ps_matched = xcalloc(1, pathspec->nr);
+
+
+   if (read_cache() < 0)
+   die("index file corrupt");
+
+   for (i = 0; i < active_nr; i++) {
+   const struct cache_entry *ce = active_cache[i];
+
+   if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
+   max_prefix_len, ps_matched,
+   S_ISGITLINK(ce->ce_mode) | 
S_ISDIR(ce->ce_mode)))
+   continue;
+
+   if (S_ISGITLINK(ce->ce_mode)) {
+   ALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);
+   ce_entries[ce_used++] = ce;
+   }
+   }
+}
+
+static int module_list(int argc, const char **argv, const char *prefix)
+{
+   int i;
+   struct string_list already_printed = STRING_LIST_INIT_NODUP;
+   struct pathspec pathspec;
+   const char *alternative_path;
+
+   struct option module_list_options[] = {
+   OPT_STRING(0, "prefix", &alternative_path,
+  N_("path"),
+  N_("alternative anchor for relative paths")),
+   OPT_END()
+   };
+
+   static const char * const git_submodule_helper_usage[] = {
+   N_("git submodule--helper module_list [--prefix=] 
[...]"),
+   NULL
+   };
+
+   argc = parse_options(argc, argv, prefix, module_list_

Re: [PATCH] builtin/mv: Get rid of the last caller of get_pathspec

2015-08-03 Thread Eric Sunshine
On Mon, Aug 3, 2015 at 1:53 PM, Stefan Beller  wrote:
> `get_pathspec` is deprecated and builtin/mv is its last caller, so getting
> rid of `get_pathspec` is rather easy. By getting rid of `get_pathspec`,
> the documentation such as 'technical/api-setup.txt' becomes easier to read
> as the reader doesn't need to bear with the additional fact that
> `get_pathspec` is deprecated.
>
> The code in 'builtin/mv' still requires some work to make it less ugly.

Perhaps split this into two patches? One to free git-mv of
get_pathspec() dependence, and the second to actually retire
get_pathspec().

> Signed-off-by: Stefan Beller 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2015, #01; Mon, 3)

2015-08-03 Thread Eric Sunshine
On Mon, Aug 3, 2015 at 6:18 PM, Junio C Hamano  wrote:
> I've kicked a few topics out of 'next' back to 'pu' for now.
>  - I think es/worktree-add and es/worktree-add-cleanup are good
>shape overall, but we probably would want to make them into a
>single topic.

Is there anything I need to do to move this along, or is it something
you will be handling locally?
--
To unsubscribe from this list: send the line "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] untracked-cache: fix subdirectory handling

2015-08-03 Thread David Turner
Previously, some calls lookup_untracked would pass a full path.  But
lookup_untracked assumes that the portion of the path up to and
including to the untracked_cache_dir has been removed.  So
lookup_untracked would be looking in the untracked_cache for 'foo' for
'foo/bar' (instead of just looking for 'bar').  This would cause
untracked cache corruption.

To fix this, untracked_cache_dir learns to track its depth.  Callers
to lookup_untracked which have a full path now first trim off a
sufficient number of path prefixes.  A new helper function,
lookup_untracked_recursive, helps untracked_cache_invalidate_path to
perform this operation.

Signed-off-by: David Turner 
---

This patch applies on top of dt/untracked-sparse, presently in `pu` at
d2cd01bd.  I think only the test part depends on that patch.

Duy, let me know if you think this is the right approach.

---
 dir.c | 50 ---
 dir.h |  1 +
 t/t7063-status-untracked-cache.sh | 72 ++-
 3 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/dir.c b/dir.c
index e7b89fe..314080b 100644
--- a/dir.c
+++ b/dir.c
@@ -631,6 +631,7 @@ static struct untracked_cache_dir *lookup_untracked(struct 
untracked_cache *uc,
memset(d, 0, sizeof(*d));
memcpy(d->name, name, len);
d->name[len] = '\0';
+   d->depth = dir->depth + 1;
 
ALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc);
memmove(dir->dirs + first + 1, dir->dirs + first,
@@ -1324,7 +1325,19 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
return exclude ? path_excluded : path_untracked;
 
-   untracked = lookup_untracked(dir->untracked, untracked, dirname, len);
+   if (untracked) {
+   const char *cur = dirname;
+   int i;
+
+   for (i = 0; i < untracked->depth; i++) {
+   cur = strchr(cur, '/');
+   assert(cur);
+   cur++;
+   }
+   untracked = lookup_untracked(dir->untracked, untracked,
+cur,
+len - (cur - dirname));
+   }
return read_directory_recursive(dir, dirname, len,
untracked, 1, simplify);
 }
@@ -2431,7 +2444,7 @@ static void stat_data_from_disk(struct stat_data *to, 
const struct stat_data *fr
 }
 
 static int read_one_dir(struct untracked_cache_dir **untracked_,
-   struct read_data *rd)
+   struct read_data *rd, int depth)
 {
struct untracked_cache_dir ud, *untracked;
const unsigned char *next, *data = rd->data, *end = rd->end;
@@ -2444,6 +2457,7 @@ static int read_one_dir(struct untracked_cache_dir 
**untracked_,
value = decode_varint(&next);
if (next > end)
return -1;
+   ud.depth = depth;
ud.recurse = 1;
ud.untracked_alloc = value;
ud.untracked_nr= value;
@@ -2480,7 +2494,7 @@ static int read_one_dir(struct untracked_cache_dir 
**untracked_,
rd->data = data;
 
for (i = 0; i < untracked->dirs_nr; i++) {
-   len = read_one_dir(untracked->dirs + i, rd);
+   len = read_one_dir(untracked->dirs + i, rd, depth + 1);
if (len < 0)
return -1;
}
@@ -2577,7 +2591,7 @@ struct untracked_cache *read_untracked_extension(const 
void *data, unsigned long
rd.index  = 0;
rd.ucd= xmalloc(sizeof(*rd.ucd) * len);
 
-   if (read_one_dir(&uc->root, &rd) || rd.index != len)
+   if (read_one_dir(&uc->root, &rd, 0) || rd.index != len)
goto done;
 
next = rd.data;
@@ -2614,20 +2628,32 @@ done2:
return uc;
 }
 
+static struct untracked_cache_dir *lookup_untracked_recursive(
+   struct untracked_cache *uc, struct untracked_cache_dir *dir,
+   const char *path, int len)
+{
+   const char *rest = strchr(path, '/');
+
+   if (rest) {
+   int component_len = rest - path;
+   struct untracked_cache_dir *d =
+   lookup_untracked(uc, dir, path, component_len);
+   return lookup_untracked_recursive(uc, d, rest + 1,
+ len - (component_len + 1));
+   } else {
+   return dir;
+   }
+}
+
 void untracked_cache_invalidate_path(struct index_state *istate,
 const char *path)
 {
-   const char *sep;
struct untracked_cache_dir *d;
if (!istate->untracked || !istate->untracked->root)
return;
-   sep = strrchr(path, '/');
-   if (sep)
-   d = lookup_untracked(istate->untracked,
-is

[ANNOUNCE] Git v2.4.8

2015-08-03 Thread Junio C Hamano
A maintenance release Git v2.4.8 is now available at the usual
places.  This contains remaining fixes that are in v2.5 but have
been lacking in 2.4.x maintenance track so far.  It will hopefully
be the last 2.4.x update at least for now.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.4.8'
tag and some of them have the 'maint-2.4' branch that the tag
points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git



Git v2.4.8 Release Notes


Fixes since v2.4.7
--

 * Abandoning an already applied change in "git rebase -i" with
   "--continue" left CHERRY_PICK_HEAD and confused later steps.

 * Various fixes around "git am" that applies a patch to a history
   that is not there yet.

 * "git for-each-ref" reported "missing object" for 0{40} when it
   encounters a broken ref.  The lack of object whose name is 0{40} is
   not the problem; the ref being broken is.

 * "git commit --cleanup=scissors" was not careful enough to protect
   against getting fooled by a line that looked like scissors.

Also contains typofixes, documentation updates and trivial code
clean-ups.



Changes since v2.4.7 are as follows:

Johannes Schindelin (2):
  t3404: demonstrate CHERRY_PICK_HEAD bug
  rebase -i: do not leave a CHERRY_PICK_HEAD file behind

Junio C Hamano (1):
  Git 2.4.8

Michael Haggerty (4):
  t6301: new tests of for-each-ref error handling
  for-each-ref: report broken references correctly
  read_loose_refs(): simplify function logic
  read_loose_refs(): treat NULL_SHA1 loose references as broken

Paul Tan (6):
  am --skip: revert changes introduced by failed 3way merge
  am -3: support 3way merge on unborn branch
  am --skip: support skipping while on unborn branch
  am --abort: revert changes introduced by failed 3way merge
  am --abort: support aborting to unborn branch
  am --abort: keep unrelated commits on unborn branch

SZEDER Gábor (2):
  completion: teach 'scissors' mode to 'git commit --cleanup='
  commit: cope with scissors lines in commit message

Sebastian Schuberth (1):
  clone: simplify string handling in guess_dir_name()

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Aug 2015, #01; Mon, 3)

2015-08-03 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Accumulated fixes went to a new maintenance release 2.4.8.  The
first batch of the post-2.5 development cycle has been merged to
'master', and the tip of 'next' has been rewound.

I've kicked a few topics out of 'next' back to 'pu' for now.

 - I think pt/am-builtin is more or less ready, but the fix to the
   issue that options given to 'git am' when restarting were
   rejected must be queued on that topic before we can start
   thinking of merging it to 'master' for the next release.

 - I think es/worktree-add and es/worktree-add-cleanup are good
   shape overall, but we probably would want to make them into a
   single topic.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* as/sparse-checkout-removal (2015-07-21) 1 commit
  (merged to 'next' on 2015-07-21 at ab94680)
 + unpack-trees: don't update files with CE_WT_REMOVE set

 "sparse checkout" misbehaved for a path that is excluded from the
 checkout when switching between branches that differ at the path.


* bc/gpg-verify-raw (2015-06-22) 7 commits
  (merged to 'next' on 2015-06-24 at 08a1164)
 + verify-tag: add option to print raw gpg status information
 + verify-commit: add option to print raw gpg status information
 + gpg: centralize printing signature buffers
 + gpg: centralize signature check
 + verify-commit: add test for exit status on untrusted signature
 + verify-tag: share code with verify-commit
 + verify-tag: add tests

 "git verify-tag" and "git verify-commit" have been taught to share
 more code, and then learned to optionally show the verification
 message from the underlying GPG implementation.


* cb/parse-magnitude (2015-06-22) 2 commits
  (merged to 'next' on 2015-06-24 at 2fd7205)
 + parse-options: move unsigned long option parsing out of pack-objects.c
 + test-parse-options: update to handle negative ints

 Move machinery to parse human-readable scaled numbers like 1k, 4M,
 and 2G as an option parameter's value from pack-objects to
 parse-options API, to make it available to other codepaths.


* cb/uname-in-untracked (2015-07-17) 1 commit
  (merged to 'next' on 2015-07-21 at d867af0)
 + untracked: fix detection of uname(2) failure

 An experimental "untracked cache" feature used uname(2) in a
 slightly unportable way.


* da/subtree-date-confusion (2015-07-23) 1 commit
  (merged to 'next' on 2015-07-29 at 01016b1)
 + contrib/subtree: ignore log.date configuration

 "git subtree" (in contrib/) depended on "git log" output to be
 stable, which was a no-no.  Apply a workaround to force a
 particular date format.


* db/send-pack-user-signingkey (2015-07-21) 1 commit
  (merged to 'next' on 2015-07-29 at b0d62e9)
 + builtin/send-pack.c: respect user.signingkey

 The low-level "git send-pack" did not honor 'user.signingkey'
 configuration variable when sending a signed-push.


* dt/log-follow-config (2015-07-09) 1 commit
  (merged to 'next' on 2015-07-10 at b8fbb43)
 + log: add "log.follow" configuration variable

 Add a new configuration variable to enable "--follow" automatically
 when "git log" is run with one pathspec argument.


* dt/refs-backend-preamble (2015-07-21) 7 commits
  (merged to 'next' on 2015-07-23 at 9dac423)
 + git-stash: use update-ref --create-reflog instead of creating files
 + update-ref and tag: add --create-reflog arg
 + refs: add REF_FORCE_CREATE_REFLOG flag
 + git-reflog: add exists command
 + refs: new public ref function: safe_create_reflog
 + refs: break out check for reflog autocreation
 + refs.c: add err arguments to reflog functions
 (this branch is used by dt/refs-pseudo.)

 In preparation for allowing different "backends" to store the refs
 in a way different from the traditional "one ref per file in $GIT_DIR
 or in a $GIT_DIR/packed-refs file" filesystem storage, reduce
 direct filesystem access to ref-like things like CHERRY_PICK_HEAD
 from scripts and programs.


* ee/clean-remove-dirs (2015-06-26) 6 commits
  (merged to 'next' on 2015-06-29 at d595659)
 + read_gitfile_gently: fix use-after-free
  (merged to 'next' on 2015-06-24 at 7c27821)
 + clean: improve performance when removing lots of directories
 + p7300: add performance tests for clean
 + t7300: add tests to document behavior of clean and nested git
 + setup: sanity check file size in read_gitfile_gently
 + setup: add gentle version of read_gitfile

 Replace "is this subdirectory a separate repository that should not
 be touched?" check "git clean" does by checking if it has .git/HEAD
 using the submodule-related code with a more optimized check.


* es/doc-clean-outdated-tools (2015-07-28) 5 commits
  (merged to 'next' on 2015-07-29 at 6d80251)
 + Documentation/git-tools: retire manually-mainta

Re: Question: .idx without .pack causes performance issues?

2015-08-03 Thread Doug Kelly
On Tue, Jul 21, 2015 at 4:37 PM, Doug Kelly  wrote:
> On Tue, Jul 21, 2015 at 3:48 PM, Junio C Hamano  wrote:
>> Junio C Hamano  writes:
>>
>>> While I still think that it is more important to prevent such a
>>> situation from occurring in the first place, ignoring .idx that lack
>>> corresponding .pack should be fairly simple, perhaps like this.
>>> ...
>>
>> Sorry for the noise, but this patch is worthless.  We already have
>> an equivalent test in add_packed_git() that is called from this same
>> place.
>
> And a few extra updates from me: we found that this appears to occur
> even after update to 1.9.5, and setting core.fscache on 2.4.6 has no
> appreciable impact on the time it takes to run "git fetch", either.
> Our thought was antivirus (or something else?) might have the file
> open when git attempts to unlink the .idx, but perhaps it's something
> else, too?  In one case, we had ~560 orphaned .idx files, but 150
> seems sufficient to slow a fetch operation for a few minutes until it
> actually begins transferring objects.
>
> The "git gc" approach to cleaning up the mess is certainly looking
> more and more attractive... :)

Here's a change to prune.c that at least addresses the issue by removing
.idx files without an associated pack, but it's by no means pretty.  If anyone
has any feedback before I turn this into a formal patch, it's more than welcome!

diff --git a/builtin/prune.c b/builtin/prune.c
index 10b03d3..8a60282 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "diff.h"
+#include "dir.h"
 #include "revision.h"
 #include "builtin.h"
 #include "reachable.h"
@@ -85,15 +86,31 @@ static void remove_temporary_files(const char *path)
 {
DIR *dir;
struct dirent *de;
+   struct strbuf idx, pack;

dir = opendir(path);
if (!dir) {
fprintf(stderr, "Unable to open directory %s\n", path);
return;
}
-   while ((de = readdir(dir)) != NULL)
+   while ((de = readdir(dir)) != NULL) {
if (starts_with(de->d_name, "tmp_"))
prune_tmp_file(mkpath("%s/%s", path, de->d_name));
+   if (ends_with(de->d_name, ".idx")) {
+   strbuf_init(&idx, 0);
+   strbuf_init(&pack, 0);
+   strbuf_addstr(&idx, de->d_name);
+   strbuf_addbuf(&pack, &idx);
+   if (strbuf_strip_suffix(&pack, ".idx")) {
+   strbuf_addstr(&pack, ".pack");
+   if (!file_exists(mkpath("%s/%s", path,
pack.buf)))
+   prune_tmp_file(mkpath("%s/%s",
path, idx.buf));
+   }
+   strbuf_release(&idx);
+   strbuf_release(&pack);
+   }
+
+   }
closedir(dir);
 }

--
--
To unsubscribe from this list: send the line "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 v8 0/11] Port tag.c over to use ref-filter APIs

2015-08-03 Thread Karthik Nayak
On Tue, Aug 4, 2015 at 3:38 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> On Tue, Aug 4, 2015 at 1:51 AM, Junio C Hamano  wrote:
>> ...
>>> It is very very dissapointing to allow the "next atom only"
>>> implementation to squat on a good name "align:,",
>>> especially when I thought that the list agreed
>>>
>>>   %(align:,) any string with or without %(atom) %(end)
>>>
>>> would be the way to go.
>>
>> From what I read, I thought we wanted the next atom or string to be
>> aligned, if we need to align everything within the %(end) atom.
>
> Is that a serious comment?
>
> Did I read too much into your $gmane/275119, expecting that you
> understood everything you are saying "That's a good way to go" to?

Sorry, I kinda was thinking only WRT to the %(if) and %(end) part of it.
Even though you clearly mentioned about %(align) also.

>
>> I could do that :)
>
> Sure ;-)

I have it ready, will wait to see if there are more comments and send with
next iteration of the series.

-- 
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


Re: [RFC/PATCH 1/2] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Stefan Beller
On Mon, Aug 3, 2015 at 3:04 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> On Fri, Jul 31, 2015 at 6:01 PM, Junio C Hamano  wrote:
>>> Stefan Beller  writes:
>>>
 +static const char * const git_submodule_helper_usage[] = {
 + N_("git submodule--helper --module_list [...]"),
>>>
>>> Yuck.  Please do not force --multi_word_opt upon us, which is simply
>>> too ugly to live around here.  --module-list is perhaps OK,
>>
>> I agree there. The way you word it here, it sounds as if the mixture
>> of dashes and underscores are a problem.
>>
>>> but
>>> because submodule--helper would not have an default action, I'd
>>> prefer to make these just "command words", i.e.
>>>
>>> $ git submodule--helper module_list
>>
>> Why would you use an underscore in here as opposed to a dash?
>>  $ git submodule--helper module-list
>>
>> I went with --module-list for now as I see no reason real to make it
>> a command word for now ...
>
> The biggest reason why we should not add more --command-mode is to
> avoid confusion (and copy & paste misdesign by others).  If you use
> the command-word interface, it is crystal clear that
>
>  (1) the word 'module_list' must be the first token after the
>  subcommand name, no need to parse "subcmd --opt --cmd", and
>  mislead the users to think incorrectly that ...
>
>  (2) ... "cmd --optA --cmd1 --optB --cmd2" might be allowed, which
>  would lead you to add code to reject, saying "cmd1 and cmd2 are
>  incompatible".
>
> So I'd prefer to see it fixed before you start supporting more
> commands in submodule--helper.  It will need unnecessary patch noise
> to fix it later.

So we had this discussion some time ago [1] and my understanding from back
then was to rather have --command-mode instead of subcommand words because
that's what most git commands use nowadays, so we don't want to add more
of the competing style. It's also easier to work with as we have a powerful
option parsing implementation.

It seems your opinion has swayed. I'll change it then.

[1] $gmane/254076 or $gmane/231376/focus=231478
--
To unsubscribe from this list: send the line "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 v8 0/11] Port tag.c over to use ref-filter APIs

2015-08-03 Thread Junio C Hamano
Karthik Nayak  writes:

> On Tue, Aug 4, 2015 at 1:51 AM, Junio C Hamano  wrote:
> ...
>> It is very very dissapointing to allow the "next atom only"
>> implementation to squat on a good name "align:,",
>> especially when I thought that the list agreed
>>
>>   %(align:,) any string with or without %(atom) %(end)
>>
>> would be the way to go.
>
> From what I read, I thought we wanted the next atom or string to be
> aligned, if we need to align everything within the %(end) atom.

Is that a serious comment?

Did I read too much into your $gmane/275119, expecting that you
understood everything you are saying "That's a good way to go" to?

> I could do that :)

Sure ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/2] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Junio C Hamano
Stefan Beller  writes:

> On Fri, Jul 31, 2015 at 6:01 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> +static const char * const git_submodule_helper_usage[] = {
>>> + N_("git submodule--helper --module_list [...]"),
>>
>> Yuck.  Please do not force --multi_word_opt upon us, which is simply
>> too ugly to live around here.  --module-list is perhaps OK,
>
> I agree there. The way you word it here, it sounds as if the mixture
> of dashes and underscores are a problem.
>
>> but
>> because submodule--helper would not have an default action, I'd
>> prefer to make these just "command words", i.e.
>>
>> $ git submodule--helper module_list
>
> Why would you use an underscore in here as opposed to a dash?
>  $ git submodule--helper module-list
>
> I went with --module-list for now as I see no reason real to make it
> a command word for now ...

The biggest reason why we should not add more --command-mode is to
avoid confusion (and copy & paste misdesign by others).  If you use
the command-word interface, it is crystal clear that

 (1) the word 'module_list' must be the first token after the
 subcommand name, no need to parse "subcmd --opt --cmd", and
 mislead the users to think incorrectly that ...

 (2) ... "cmd --optA --cmd1 --optB --cmd2" might be allowed, which
 would lead you to add code to reject, saying "cmd1 and cmd2 are
 incompatible".

So I'd prefer to see it fixed before you start supporting more
commands in submodule--helper.  It will need unnecessary patch noise
to fix it later.
--
To unsubscribe from this list: send the line "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 v8 02/11] ref-filter: introduce ref_formatting_state

2015-08-03 Thread Karthik Nayak
On Tue, Aug 4, 2015 at 2:12 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Introduce a ref_formatting_state which will eventually hold the values
>> of modifier atoms. Implement this within ref-filter.
>>
>> Mentored-by: Christian Couder 
>> Mentored-by: Matthieu Moy 
>> Signed-off-by: Karthik Nayak 
>> ---
>>  ref-filter.c | 49 +
>>  ref-filter.h |  4 
>>  2 files changed, 41 insertions(+), 12 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index febdc45..c4c7064 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1190,9 +1190,10 @@ void ref_array_sort(struct ref_sorting *sorting, 
>> struct ref_array *array)
>>   qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
>> compare_refs);
>>  }
>>
>> -static void print_value(struct atom_value *v, int quote_style, struct 
>> strbuf *output)
>> +static void print_value(struct atom_value *v, struct ref_formatting_state 
>> *state,
>> + struct strbuf *output)
>>  {
>
> I expect that the state would eventually become a stack of states
> (i.e. the topmost one passed around, each having a pointer to the
> previous level) in order to implement that "divert" mechanism for
> (possibly nested) if ... end construct.
>

Makes sense :)

> With that in mind, I suspect that state->output should be "where the
> current level would output to", i.e. no need to pass state and
> output around separately.
>

Will do!

>> +static void apply_formatting_state(struct ref_formatting_state *state, 
>> struct strbuf *value,
>> +struct strbuf *format)
>> +{
>
> The name "format" feels quite misleading; the readers would expect
> that you would use it in "strbuf_addf(format, value)", but that is
> not what is going on here.
>

will change it to final i guess.

>> @@ -1275,12 +1299,13 @@ void show_ref_array_item(struct ref_array_item 
>> *info, const char *format, int qu
>>   if (color_parse("reset", color) < 0)
>>   die("BUG: couldn't parse 'reset' as a color");
>>   resetv.s = color;
>> - print_value(&resetv, quote_style, &output);
>> + print_value(&resetv, &state, &value);
>> + apply_formatting_state(&state, &value, &final_buf);
>>   }
>> - for (i = 0; i < output.len; i++)
>> - printf("%c", output.buf[i]);
>> + for (i = 0; i < final_buf.len; i++)
>> + printf("%c", final_buf.buf[i]);
>>   putchar('\n');
>> - strbuf_release(&output);
>> + strbuf_release(&final_buf);
>>  }
>>
>>  /*  If no sorting option is given, use refname to sort as default */
>> diff --git a/ref-filter.h b/ref-filter.h
>> index 6bf27d8..b64677f 100644
>> --- a/ref-filter.h
>> +++ b/ref-filter.h
>> @@ -16,6 +16,10 @@
>>  #define FILTER_REFS_INCLUDE_BROKEN 0x1
>>  #define FILTER_REFS_ALL 0x2
>>
>> +struct ref_formatting_state {
>> + int quote_style;
>> +};
>> +
>>  struct atom_value {
>>   const char *s;
>>   unsigned long ul; /* used for sorting when not FIELD_STR */



-- 
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


[PATCH] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Stefan Beller
Most of the submodule operations work on a set of submodules.
Calculating and using this set is usually done via:

   module_list "$@" | {
   while read mode sha1 stage sm_path
   do
# the actual operation
   done
   }

Currently the function `module_list` is implemented in the
git-submodule.sh as a shell script wrapping a perl script.
The rewrite is in C, such that it is faster and can later be
easily adapted when other functions are rewritten in C.

git-submodule.sh similar to the builtin commands will navigate
to the top most directory of the repository and keeping the
subdirectories as a variable. As the helper is called from
within the git-submodule.sh script, we are already navigated
to the root level, but the path arguments are stil relative
to the subdirectory we were in when calling git-submodule.sh.
That's why there is a `--prefix` option pointing to an alternative
path where to anchor relative path arguments.

Signed-off-by: Stefan Beller 
---

This doesn't have the `module_list` subcommand word,
but rather uses the `--module-list` mode for now.

However this is not an RFC any more, but I consider it stable.

Thanks,
Stefan

 Makefile|   1 +
 builtin.h   |   1 +
 builtin/submodule--helper.c | 119 
 git-submodule.sh|  54 +++-
 git.c   |   1 +
 5 files changed, 128 insertions(+), 48 deletions(-)
 create mode 100644 builtin/submodule--helper.c

diff --git a/Makefile b/Makefile
index 8c3c724..6fb7484 100644
--- a/Makefile
+++ b/Makefile
@@ -898,6 +898,7 @@ BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
 BUILTIN_OBJS += builtin/stripspace.o
+BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
 BUILTIN_OBJS += builtin/tag.o
 BUILTIN_OBJS += builtin/unpack-file.o
diff --git a/builtin.h b/builtin.h
index 9e04f97..7bf9597 100644
--- a/builtin.h
+++ b/builtin.h
@@ -118,6 +118,7 @@ extern int cmd_show(int argc, const char **argv, const char 
*prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
+extern int cmd_submodule__helper(int argc, const char **argv, const char 
*prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
new file mode 100644
index 000..3f6d07d
--- /dev/null
+++ b/builtin/submodule--helper.c
@@ -0,0 +1,119 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "quote.h"
+#include "pathspec.h"
+#include "dir.h"
+#include "utf8.h"
+
+static const char * const git_submodule_helper_usage[] = {
+   N_("git submodule--helper [] [...]"),
+   NULL
+};
+
+static char *ps_matched;
+static int max_prefix_len;
+static const struct cache_entry **ce_entries;
+static int ce_alloc, ce_used;
+static const char *alternative_path;
+
+static void module_list_compute(int argc, const char **argv,
+   const char *prefix,
+   struct pathspec *pathspec)
+{
+   int i;
+   char *max_prefix;
+   parse_pathspec(pathspec, 0,
+  PATHSPEC_PREFER_FULL |
+  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+  prefix, argv);
+
+   /* Find common prefix for all pathspec's */
+   max_prefix = common_prefix(pathspec);
+   max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
+
+   if (pathspec->nr)
+   ps_matched = xcalloc(1, pathspec->nr);
+
+
+   if (read_cache() < 0)
+   die("index file corrupt");
+
+   for (i = 0; i < active_nr; i++) {
+   const struct cache_entry *ce = active_cache[i];
+
+   if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
+   max_prefix_len, ps_matched,
+   S_ISGITLINK(ce->ce_mode) | 
S_ISDIR(ce->ce_mode)))
+   continue;
+
+   if (S_ISGITLINK(ce->ce_mode)) {
+   ALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);
+   ce_entries[ce_used++] = ce;
+   }
+   }
+}
+
+static int module_list(int argc, const char **argv, const char *prefix)
+{
+   int i;
+   struct string_list already_printed = STRING_LIST_INIT_NODUP;
+   struct pathspec pathspec;
+
+   module_list_compute(argc, argv, prefix, &pathspec);
+
+   if (ps_matched && report_path_error(ps_matched, &pathspec, prefix)) {
+   printf("#unmatched\n");
+   

Re: Draft of Git Rev News edition 6

2015-08-03 Thread Thomas Ferris Nicolaisen
On Mon, Aug 3, 2015 at 11:16 PM, Junio C Hamano  wrote:
>> I hope we can attract more contributors in the future, so the weight
>> of this doesn't lie too much on his shoulders. Perhaps we should send
>> out the draft earlier next time, and beckon for more contributions
>> from the list.
>
> Yeah, that is probably a good idea.  I was sort of surprised that
> you announced to publish it in a few days.

There's no deep thoughts behind the scheduling. I just followed the
pattern of how Christian has done it in the past. If anyone is
preparing something concrete for this edition, let me know before
Wednesday.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/2] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Junio C Hamano
Stefan Beller  writes:

>> $ git submodule--helper module_list
>
> Why would you use an underscore in here as opposed to a dash?

Simply because the diff would be easier to read; the callers used to
call module_list shell function, now they call the subcommand with the
same name of submodule--helper.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/2] submodule: implement `module_list` as a builtin helper

2015-08-03 Thread Stefan Beller
On Fri, Jul 31, 2015 at 6:01 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> +static const char * const git_submodule_helper_usage[] = {
>> + N_("git submodule--helper --module_list [...]"),
>
> Yuck.  Please do not force --multi_word_opt upon us, which is simply
> too ugly to live around here.  --module-list is perhaps OK,

I agree there. The way you word it here, it sounds as if the mixture
of dashes and underscores are a problem.

> but
> because submodule--helper would not have an default action, I'd
> prefer to make these just "command words", i.e.
>
> $ git submodule--helper module_list

Why would you use an underscore in here as opposed to a dash?
 $ git submodule--helper module-list

I went with --module-list for now as I see no reason real to make it
a command word for now as it is not user facing but just a helper.
I have a patch from my previous attempt to rewrite "git submodule"
as a whole to accept both command words as well as double dashed
selected modes.

>
>> +int module_list(int argc, const char **argv, const char *prefix)
>> +{
>> + int i;
>> + static struct pathspec pathspec;
>> + const struct cache_entry **ce_entries = NULL;
>> + int alloc = 0, used = 0;
>> + char *ps_matched = NULL;
>> + char *max_prefix;
>> + int max_prefix_len;
>> + struct string_list already_printed = STRING_LIST_INIT_NODUP;
>> +
>> + parse_pathspec(&pathspec, 0,
>> +PATHSPEC_PREFER_FULL,
>> +prefix, argv);
>> +
>> + /* Find common prefix for all pathspec's */
>> + max_prefix = common_prefix(&pathspec);
>> + max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
>> +
>> + if (pathspec.nr)
>> + ps_matched = xcalloc(1, pathspec.nr);
>
> Up to this point it interprets its input, and ...
>
>> + if (read_cache() < 0)
>> + die("index file corrupt");
>> +
>> + for (i = 0; i < active_nr; i++) {
>> + const struct cache_entry *ce = active_cache[i];
>> +
>> + if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
>> + max_prefix_len, ps_matched,
>> + S_ISGITLINK(ce->ce_mode) | 
>> S_ISDIR(ce->ce_mode)))
>> + continue;
>> +
>> + if (S_ISGITLINK(ce->ce_mode)) {
>> + ALLOC_GROW(ce_entries, used + 1, alloc);
>> + ce_entries[used++] = ce;
>> + }
>> + }
>> +
>> + if (ps_matched && report_path_error(ps_matched, &pathspec, prefix)) {
>> + printf("#unmatched\n");
>> + return 1;
>> + }
>
> ... does the computation, with diagnosis.
>
> And then it does the I/O with formatting.
>
>> +
>> + for (i = 0; i < used; i++) {
>> + const struct cache_entry *ce = ce_entries[i];
> ...
>> + return 0;
>> +}
>
> When you have the implementation of "foreach-parallel" to move the
> most expensive part of "submodule update" of a tree with 500
> submodules, you would want to receive more or less the same "args"
> as this thing takes and pass the ce_entries[] list to the "spawn and
> run the user script in them in parallel" engine.

That's true, I thought about splitting it up later when I actually need it.
[That seems easier to write, but not easier to review :( ]
I did split up the function just now.

>
> So I think it makes more sense to split this function into two (or
> three).  One that reads from (argc, argv) and allocates and fills
> ce_entries[] can become a helper that you can reuse later.
>
> 'int module_list()' (shouldn't it be static?), can make a call to
> that helper at the begining of it, and the remainder of the function
> would do the textual I/O.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()

2015-08-03 Thread Junio C Hamano
Max Kirillov  writes:

Here is a space to describe why this change is a good thing.  Is
this a fix to change the behaviour, and if so how is the behaviour
different with and without the patch?  Or is this just to drop the
block of code from here and replace it with a call to an existing
helper that does exactly the same thing?  I _suspect_ that it is the
latter, but please do not force reviewers to guess.

> Signed-off-by: Max Kirillov 
> ---
>  submodule.c | 28 ++--
>  1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 15e90d1..f6afe0a 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)
>  
>  static int add_submodule_odb(const char *path)
>  {
> - struct strbuf objects_directory = STRBUF_INIT;
>   struct alternate_object_database *alt_odb;
> + const char* objects_directory;

Style; asterisk sticks to the variable, not the type.  I think you
need to fix this in multiple places (not just 1/2 but also in 2/2).

Other than that I think it is a sensible "replace bulk of code with
identical helper that already exists" rewrite.

Thanks.

>   int ret = 0;
> - const char *git_dir;
>  
> - strbuf_addf(&objects_directory, "%s/.git", path);
> - git_dir = read_gitfile(objects_directory.buf);
> - if (git_dir) {
> - strbuf_reset(&objects_directory);
> - strbuf_addstr(&objects_directory, git_dir);
> - }
> - strbuf_addstr(&objects_directory, "/objects/");
> - if (!is_directory(objects_directory.buf)) {
> + objects_directory = git_path_submodule(path, "objects/");
> + if (!is_directory(objects_directory)) {
>   ret = -1;
>   goto done;
>   }
> +
>   /* avoid adding it twice */
>   for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
> - if (alt_odb->name - alt_odb->base == objects_directory.len &&
> - !strncmp(alt_odb->base, objects_directory.buf,
> - objects_directory.len))
> + if (alt_odb->name - alt_odb->base == strlen(objects_directory) 
> &&
> + !strcmp(alt_odb->base, objects_directory))
>   goto done;
>  
> - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
> + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));
>   alt_odb->next = alt_odb_list;
> - strcpy(alt_odb->base, objects_directory.buf);
> - alt_odb->name = alt_odb->base + objects_directory.len;
> + strcpy(alt_odb->base, objects_directory);
> + alt_odb->name = alt_odb->base + strlen(objects_directory);
>   alt_odb->name[2] = '/';
>   alt_odb->name[40] = '\0';
>   alt_odb->name[41] = '\0';
>   alt_odb_list = alt_odb;
>  
>   /* add possible alternates from the submodule */
> - read_info_alternates(objects_directory.buf, 0);
> + read_info_alternates(objects_directory, 0);
>   prepare_alt_odb();
>  done:
> - strbuf_release(&objects_directory);
>   return ret;
>  }
--
To unsubscribe from this list: send the line "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 v8 01/11] ref-filter: print output to strbuf for formatting

2015-08-03 Thread Karthik Nayak
On Tue, Aug 4, 2015 at 2:06 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Introduce a strbuf `output` which will act as a substitute rather than
>> printing directly to stdout. This will be used for formatting
>> eventually.
>> ---
>
> Missing sign-off; the patch looks like a good first step in a nice
> direction.
>

Will add, Thanks :)

>>  ref-filter.c | 36 ++--
>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 7561727..febdc45 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1190,30 +1190,25 @@ void ref_array_sort(struct ref_sorting *sorting, 
>> struct ref_array *array)
>>   qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
>> compare_refs);
>>  }
>>
>> -static void print_value(struct atom_value *v, int quote_style)
>> +static void print_value(struct atom_value *v, int quote_style, struct 
>> strbuf *output)
>>  {
>> - struct strbuf sb = STRBUF_INIT;
>>   switch (quote_style) {
>>   case QUOTE_NONE:
>> - fputs(v->s, stdout);
>> + strbuf_addstr(output, v->s);
>>   break;
>>   case QUOTE_SHELL:
>> - sq_quote_buf(&sb, v->s);
>> + sq_quote_buf(output, v->s);
>>   break;
>>   case QUOTE_PERL:
>> - perl_quote_buf(&sb, v->s);
>> + perl_quote_buf(output, v->s);
>>   break;
>>   case QUOTE_PYTHON:
>> - python_quote_buf(&sb, v->s);
>> + python_quote_buf(output, v->s);
>>   break;
>>   case QUOTE_TCL:
>> - tcl_quote_buf(&sb, v->s);
>> + tcl_quote_buf(output, v->s);
>>   break;
>>   }
>> - if (quote_style != QUOTE_NONE) {
>> - fputs(sb.buf, stdout);
>> - strbuf_release(&sb);
>> - }
>>  }
>>
>>  static int hex1(char ch)
>> @@ -1234,7 +1229,7 @@ static int hex2(const char *cp)
>>   return -1;
>>  }
>>
>> -static void emit(const char *cp, const char *ep)
>> +static void emit(const char *cp, const char *ep, struct strbuf *output)
>>  {
>>   while (*cp && (!ep || cp < ep)) {
>>   if (*cp == '%') {
>> @@ -1243,13 +1238,13 @@ static void emit(const char *cp, const char *ep)
>>   else {
>>   int ch = hex2(cp + 1);
>>   if (0 <= ch) {
>> - putchar(ch);
>> + strbuf_addch(output, ch);
>>   cp += 3;
>>   continue;
>>   }
>>   }
>>   }
>> - putchar(*cp);
>> + strbuf_addch(output, *cp);
>>   cp++;
>>   }
>>  }
>> @@ -1257,19 +1252,21 @@ static void emit(const char *cp, const char *ep)
>>  void show_ref_array_item(struct ref_array_item *info, const char *format, 
>> int quote_style)
>>  {
>>   const char *cp, *sp, *ep;
>> + struct strbuf output = STRBUF_INIT;
>> + int i;
>>
>>   for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
>>   struct atom_value *atomv;
>>
>>   ep = strchr(sp, ')');
>>   if (cp < sp)
>> - emit(cp, sp);
>> + emit(cp, sp, &output);
>>   get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
>> &atomv);
>> - print_value(atomv, quote_style);
>> + print_value(atomv, quote_style, &output);
>>   }
>>   if (*cp) {
>>   sp = cp + strlen(cp);
>> - emit(cp, sp);
>> + emit(cp, sp, &output);
>>   }
>>   if (need_color_reset_at_eol) {
>>   struct atom_value resetv;
>> @@ -1278,9 +1275,12 @@ void show_ref_array_item(struct ref_array_item *info, 
>> const char *format, int qu
>>   if (color_parse("reset", color) < 0)
>>   die("BUG: couldn't parse 'reset' as a color");
>>   resetv.s = color;
>> - print_value(&resetv, quote_style);
>> + print_value(&resetv, quote_style, &output);
>>   }
>> + for (i = 0; i < output.len; i++)
>> + printf("%c", output.buf[i]);
>>   putchar('\n');
>> + strbuf_release(&output);
>>  }
>>
>>  /*  If no sorting option is given, use refname to sort as default */



-- 
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


Re: [PATCH v8 0/11] Port tag.c over to use ref-filter APIs

2015-08-03 Thread Karthik Nayak
On Tue, Aug 4, 2015 at 1:51 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> This is part of my GSoC project to unify git tag -l, git branch -l,
>> git for-each-ref.  This patch series is continued from: Git (next)
>> https://github.com/git/git/commit/bf5418f49ff0cebc6e5ce04ad1417e1a47c81b61
>>
>> This series consists of porting tag.c over to using the ref-filter APIs
>>
>> Version 7 can be found here:
>> http://thread.gmane.org/gmane.comp.version-control.git/274990
>>
>> Changes:
>> * Make padright a general align atom.
>> * Make print_value() and emit() output to a strbuf rather than stdout 
>> directly.
>>
>> Interdiff:
>>
>> diff --git a/Documentation/git-for-each-ref.txt
>> b/Documentation/git-for-each-ref.txt
>> index bcf319a..e89b9b0 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -127,11 +127,12 @@ color::
>>  Change output color.  Followed by `:`, where names
>>  are described in `color.branch.*`.
>>
>> -padright::
>> -Pad succeeding atom or string to the right. Followed by
>> -`:`, where `value` states the total length of atom or
>> -string including the padding. If the `value` is lesser than
>> -the atom or string length, then no padding is performed.
>> +align::
>> +Align succeeding atoms to the right, left or middle. Followed
>> +by `:,`, where the `` is either
>> +left, right or middle and `` is the total
>> +length of the padding to be performed. If the atom length is
>> +more than the padding length then no padding is performed.
>
> It is very very dissapointing to allow the "next atom only"
> implementation to squat on a good name "align:,",
> especially when I thought that the list agreed
>
>   %(align:,) any string with or without %(atom) %(end)
>
> would be the way to go.

>From what I read, I thought we wanted the next atom or string to be
aligned, if we need to align everything within the %(end) atom. I could
do that :)

-- 
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


Windows 7 - long/short and upper/lower filename

2015-08-03 Thread JoséQuintas

I am using Windows 7 64 bits.

1)

I have a filename called ze_xmlfun.prg
Last week gitgui shows 2 changed files: ZE_XML~1.PRG and ze_xmlfun.prg
At momment I can't update this file, because get a error.
Try delete file, update one name each time, but same error.
Seems that a unique file is considered as 2 files, and this causes error.

2)

When do a search in history, upper/lower case must be the same as in git 
control.

If upper/lower is changed, is considered a new file.
How to configure git to work allways using lower case on Windows?

3)

Is there a plugin or a tool to make this, to new files and to update in 
git repository?



Note: gitgui is a gui tool for git, ok, but once it uses git, may be 
this feature/change need to be made in git.


José M. C. Quintas

--
To unsubscribe from this list: send the line "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: Draft of Git Rev News edition 6

2015-08-03 Thread Junio C Hamano
Thomas Ferris Nicolaisen  writes:

> I hope we can attract more contributors in the future, so the weight
> of this doesn't lie too much on his shoulders. Perhaps we should send
> out the draft earlier next time, and beckon for more contributions
> from the list.

Yeah, that is probably a good idea.  I was sort of surprised that
you announced to publish it in a few days.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread Junio C Hamano
David Turner  writes:

> I think making this configurable is (a) overkill and (b) dangerous.
> It's dangerous because the semantics of which refs are per-worktree is
> important to the correct operation of git, and allowing users to mess
> with it seems like a big mistake.  Instead, we should figure out a
> simple scheme and define it globally.
>
> I think refs/worktree -> refs/worktrees/[worktree]/ would do fine as a
> fixed scheme, if we go that route.

OK.

> We would need two separate views of the refs hierarchy, though: one used
> by prune (and pack-refs) that is non-mapped (that is, includes
> per-worktree refs for each worktree), and one for general use that is
> mapped.   Maybe this is just a flag to the ref traversal functions.

True.  Alternatively we could just view refs/worktree/* as if they
are symbolic refs that point into refs/worktrees/$my_worktree/*, but
that would imply making the latter always visible to all worktrees,
which would hurt when people use it to interact with outside world
(namely, refs in other people's private area should probably not be
advertised).

> As I understand it, we don't presently do many transactions that include
> both pseudorefs or per-worktree refs and other refs.  And we definitely
> don't want to move pseudorefs into the database since there's so much
> code that assumes they're files.  Also, the vast majority of refs are
> common, rather than per-worktree.  In fact, the only per-worktree refs
> I've seen mentioned so far are the bisect refs and NOTES_MERGE_REF and
> HEAD.  Of these, only HEAD is needed for pruning. Are there more that I
> haven't thought of?

I myself have come up with nothing other than the above.  Let's hear
from others.

> So I'm not sure the gain from moving per-worktree refs into the database
> is that great.

I am on the same wavelength as you are on this.

> There are some downsides of moving per-worktree refs into the database:
> ...

All good points except #3, which I cannot judge if it is good or bad.

> ...
> Simply treating refs/worktree as per-worktree, while the rest of refs/
> is not, would be a few dozen lines of code.  The full remapping approach
> is likely to be a lot more.

We may be over-engineering with Michael's and even with the more
simpler refs/worktree/* -> refs/worktrees/$mine/* fixed mapping;
I tend to agree with you.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 2/2] path: implement common_dir handling in git_path_submodule()

2015-08-03 Thread Max Kirillov
This allows making submodules a linked workdirs.

Same as for .git, but ignores the GIT_COMMON_DIR environment variable,
because it would mean common directory for the parent repository and
does not make sense for submodule.

Also add test for functionality which uses this call.

Signed-off-by: Max Kirillov 
---
 cache.h  |  1 +
 path.c   | 24 
 setup.c  | 17 -
 t/t7410-submodule-checkout-to.sh | 10 ++
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 4f55466..b87ec75 100644
--- a/cache.h
+++ b/cache.h
@@ -442,6 +442,7 @@ extern char *get_object_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
 extern int get_common_dir(struct strbuf *sb, const char *gitdir);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
diff --git a/path.c b/path.c
index 10f4cbf..f292d02 100644
--- a/path.c
+++ b/path.c
@@ -98,7 +98,7 @@ static const char *common_list[] = {
NULL
 };
 
-static void update_common_dir(struct strbuf *buf, int git_dir_len)
+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char* 
common_dir)
 {
char *base = buf->buf + git_dir_len;
const char **p;
@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int 
git_dir_len)
path++;
is_dir = 1;
}
+
+   if (!common_dir) {
+   common_dir = get_git_common_dir();
+   }
+
if (is_dir && dir_prefix(base, path)) {
-   replace_dir(buf, git_dir_len, get_git_common_dir());
+   replace_dir(buf, git_dir_len, common_dir);
return;
}
if (!is_dir && !strcmp(base, path)) {
-   replace_dir(buf, git_dir_len, get_git_common_dir());
+   replace_dir(buf, git_dir_len, common_dir);
return;
}
}
@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int 
git_dir_len)
else if (git_db_env && dir_prefix(base, "objects"))
replace_dir(buf, git_dir_len + 7, get_object_directory());
else if (git_common_dir_env)
-   update_common_dir(buf, git_dir_len);
+   update_common_dir(buf, git_dir_len, NULL);
 }
 
 static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
@@ -228,6 +233,8 @@ const char *git_path_submodule(const char *path, const char 
*fmt, ...)
 {
struct strbuf *buf = get_pathname();
const char *git_dir;
+   struct strbuf git_submodule_common_dir = STRBUF_INIT;
+   struct strbuf git_submodule_dir = STRBUF_INIT;
va_list args;
 
strbuf_addstr(buf, path);
@@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const 
char *fmt, ...)
strbuf_addstr(buf, git_dir);
}
strbuf_addch(buf, '/');
+   strbuf_addstr(&git_submodule_dir, buf->buf);
 
va_start(args, fmt);
strbuf_vaddf(buf, fmt, args);
va_end(args);
+
+   if (get_common_dir_noenv(&git_submodule_common_dir, 
git_submodule_dir.buf)) {
+   update_common_dir(buf, git_submodule_dir.len, 
git_submodule_common_dir.buf);
+   }
+
strbuf_cleanup_path(buf);
+
+   strbuf_release(&git_submodule_dir);
+   strbuf_release(&git_submodule_common_dir);
return buf->buf;
 }
 
diff --git a/setup.c b/setup.c
index 82c0cc2..39ea06b 100644
--- a/setup.c
+++ b/setup.c
@@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char 
*arg)
 
 int get_common_dir(struct strbuf *sb, const char *gitdir)
 {
+   const char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+   if (git_env_common_dir) {
+   strbuf_addstr(sb, git_env_common_dir);
+   return 1;
+   } else {
+   return get_common_dir_noenv(sb, gitdir);
+   }
+}
+
+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
+{
struct strbuf data = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
-   const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
int ret = 0;
-   if (git_common_dir) {
-   strbuf_addstr(sb, git_common_dir);
-   return 1;
-   }
+
strbuf_addf(&path, "%s/commondir", gitdir);
if (file_exists(path.buf)) {
if (strbuf_read_file(&data, path.buf, 0) <= 0)
diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh
index 3f609e8..1acef32 100755
--- a/t/t7410-submodule-checkout-to.sh
+++ b/t/t7410-submodule-checkout-to.sh
@@ -47,4 +47,

[PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()

2015-08-03 Thread Max Kirillov
Signed-off-by: Max Kirillov 
---
 submodule.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/submodule.c b/submodule.c
index 15e90d1..f6afe0a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)
 
 static int add_submodule_odb(const char *path)
 {
-   struct strbuf objects_directory = STRBUF_INIT;
struct alternate_object_database *alt_odb;
+   const char* objects_directory;
int ret = 0;
-   const char *git_dir;
 
-   strbuf_addf(&objects_directory, "%s/.git", path);
-   git_dir = read_gitfile(objects_directory.buf);
-   if (git_dir) {
-   strbuf_reset(&objects_directory);
-   strbuf_addstr(&objects_directory, git_dir);
-   }
-   strbuf_addstr(&objects_directory, "/objects/");
-   if (!is_directory(objects_directory.buf)) {
+   objects_directory = git_path_submodule(path, "objects/");
+   if (!is_directory(objects_directory)) {
ret = -1;
goto done;
}
+
/* avoid adding it twice */
for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
-   if (alt_odb->name - alt_odb->base == objects_directory.len &&
-   !strncmp(alt_odb->base, objects_directory.buf,
-   objects_directory.len))
+   if (alt_odb->name - alt_odb->base == strlen(objects_directory) 
&&
+   !strcmp(alt_odb->base, objects_directory))
goto done;
 
-   alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
+   alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));
alt_odb->next = alt_odb_list;
-   strcpy(alt_odb->base, objects_directory.buf);
-   alt_odb->name = alt_odb->base + objects_directory.len;
+   strcpy(alt_odb->base, objects_directory);
+   alt_odb->name = alt_odb->base + strlen(objects_directory);
alt_odb->name[2] = '/';
alt_odb->name[40] = '\0';
alt_odb->name[41] = '\0';
alt_odb_list = alt_odb;
 
/* add possible alternates from the submodule */
-   read_info_alternates(objects_directory.buf, 0);
+   read_info_alternates(objects_directory, 0);
prepare_alt_odb();
 done:
-   strbuf_release(&objects_directory);
return ret;
 }
 
-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 0/2] path: implement common_dir handling in git_path_submodule()

2015-08-03 Thread Max Kirillov
Rebased to latest master, also merges cleanly to pu. Otherwise no changes.

Max Kirillov (2):
  submodule refactor: use git_path_submodule() in add_submodule_odb()
  path: implement common_dir handling in git_path_submodule()

 cache.h  |  1 +
 path.c   | 24 
 setup.c  | 17 -
 submodule.c  | 28 ++--
 t/t7410-submodule-checkout-to.sh | 10 ++
 5 files changed, 53 insertions(+), 27 deletions(-)

-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line "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] remote: add get-url subcommand

2015-08-03 Thread Ben Boeckel
Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel 
---
 Documentation/git-remote.txt | 10 
 builtin/remote.c | 55 
 2 files changed, 65 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..c47b634 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' 
 'git remote set-head'  (-a | --auto | -d | --delete | )
 'git remote set-branches' [--add]  ...
+'git remote get-url' [--push] [--all] 
 'git remote set-url' [--push]   []
 'git remote set-url --add' [--push]  
 'git remote set-url --delete' [--push]  
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified 
with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried instead of fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote  that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index f4a6ec9..9278a83 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
N_("git remote prune [-n | --dry-run] "),
N_("git remote [-v | --verbose] update [-p | --prune] [( | 
)...]"),
N_("git remote set-branches [--add]  ..."),
+   N_("git remote get-url [--push] [--all] "),
N_("git remote set-url [--push]   []"),
N_("git remote set-url --add  "),
N_("git remote set-url --delete  "),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+   N_("git remote get-url [--push] [--all] "),
+   NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
N_("git remote set-url [--push]   []"),
N_("git remote set-url --add  "),
@@ -1497,6 +1503,53 @@ static int set_branches(int argc, const char **argv)
return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+   int i, push_mode = 0, all_mode = 0;
+   const char *remotename = NULL;
+   struct remote *remote;
+   const char **url;
+   int url_nr;
+   struct option options[] = {
+   OPT_BOOL('\0', "push", &push_mode,
+N_("query push URLs")),
+   OPT_BOOL('\0', "all", &all_mode,
+N_("return all URLs")),
+   OPT_END()
+   };
+   argc = parse_options(argc, argv, NULL, options, 
builtin_remote_geturl_usage,
+PARSE_OPT_KEEP_ARGV0);
+
+   if (argc < 1 || argc > 2)
+   usage_with_options(builtin_remote_geturl_usage, options);
+
+   remotename = argv[1];
+
+   if (!remote_is_configured(remotename))
+   die(_("No such remote '%s'"), remotename);
+   remote = remote_get(remotename);
+
+   if (push_mode) {
+   url = remote->pushurl;
+   url_nr = remote->pushurl_nr;
+   } else {
+   url = remote->url;
+   url_nr = remote->url_nr;
+   }
+
+   if (!url_nr)
+   die(_("no URLs configured for remote '%s'"), remotename);
+
+   if (all_mode) {
+   for (i = 0; i < url_nr; i++)
+   printf_ln("%s", url[i]);
+   } else {
+   printf_ln("%s", *url);
+   }
+
+   return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1606,6 +1659,8 @@ int cmd_remote(int argc, const char **argv, const char 
*prefix)
result = set_head(argc, argv);
else if (!strcmp(argv[0], "set-branches"))
result = set_branches(argc, argv);
+   else if (!strcmp(argv[0], "get-url"))
+   result = get_url(argc, argv);
else if (!strcmp(argv[0], "set-url"))
result = set_url(argc, argv);
else if (!strcmp(argv[0], "show"))
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] add git-url subcommand

2015-08-03 Thread Ben Boeckel
Implements a get-url subcommand to git-remote which allows for querying the
URLs for a remote while expanding insteadOf and pushInsteadOf.

--Ben

Ben Boeckel (1):
  remote: add get-url subcommand

 Documentation/git-remote.txt | 10 
 builtin/remote.c | 55 
 2 files changed, 65 insertions(+)

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Draft of Git Rev News edition 6

2015-08-03 Thread Thomas Ferris Nicolaisen
On Mon, Aug 3, 2015 at 10:07 PM, Junio C Hamano  wrote:
> Thanks; it seems that the review section is rather thin.  No new and
> interesting changes worth reviewing in the entire month of July?

Of course there are, but I can only put in a few hours, and that goes
into the links-section. The newsletter's coverage is only as good as
the coverage that gets contributed. Christian has by far been the most
active contributor on that side, and as he is unavailable these days,
those parts are scarce.

I hope we can attract more contributors in the future, so the weight
of this doesn't lie too much on his shoulders. Perhaps we should send
out the draft earlier next time, and beckon for more contributions
from the list.

We could also add a call-for-help at the bottom of the respective
section, asking people who are trawling the mailing list to
contribute.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/5] refs: add ref_type function

2015-08-03 Thread David Turner
On Mon, 2015-08-03 at 20:55 +0700, Duy Nguyen wrote:
> On Fri, Jul 31, 2015 at 1:06 PM, David Turner  
> wrote:
> > Add a function ref_type, which categorizes refs as per-worktree,
> > pseudoref, or normal ref.
> 
> For per-worktree refs, you probably should follow common_list[] in
> path.c because that's how file-based ref namespace is splitted between
> per-repo and per-worktree, even though just as simple as "everything
> outside refs/ is per-worktree" (with an exception of NOTES_MERGE_REF,
> which should be on the list as well). At least the two should be
> aligned so that the default file-based backend works the same way as
> new backends.

That would be cleaner, I think.  I'm maybe a little worried about
performance if we do this, but I guess we could optimize later. 

Before I re-roll, I'll wait until we come to a conclusion on the 
other per-worktree ref thread.

I think we discussed NOTES_MERGE_REF[1], and decided that it should work
like HEAD.  Does that seem right to you?

> Going further, I think you need to pass the "worktree identifier" to
> ref backend, at least in ref_transaction_begin_fn. Each backend is
> free to store per-worktree refs however it wants. Of course if I ask
> for refs/foo of worktree A, you should not return me refs/foo of
> worktree B. ref_transaction_begin_fn can return a fault code if it
> does not support multiple worktrees, which is fine.

If we did that, we would have to add it all over the place -- not just
ref_transaction_begin, but also update_ref.  

I think it's better to encapsulate this knowledge inside the refs code.

> > Later, we will use this in refs.c to treat pseudorefs specially.
> > Alternate ref backends may use it to treat both pseudorefs and
> > per-worktree refs differently.
> 
> I'm not so sure that this can't be hidden behind backends and they can
> have total control on falling back to file-based, or store them in
> some secondary storage. I haven't re-read your discussion with Junio
> yet (only skimmed through long ago) so I may be missing some important
> points.

The worry is symbolic refs -- a symbolic ref might be a per-worktree ref
pointing to a shared ref pointing to a per-worktree ref.  This is why
it's simpler to let backends handle things.  If we had some rules about
this, we could maybe hide this from the backend, but so far, this was
the simplest thing to do (it works great!).


[1] http://www.spinics.net/lists/git/msg256793.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 v8 02/11] ref-filter: introduce ref_formatting_state

2015-08-03 Thread Junio C Hamano
Karthik Nayak  writes:

> Introduce a ref_formatting_state which will eventually hold the values
> of modifier atoms. Implement this within ref-filter.
>
> Mentored-by: Christian Couder 
> Mentored-by: Matthieu Moy 
> Signed-off-by: Karthik Nayak 
> ---
>  ref-filter.c | 49 +
>  ref-filter.h |  4 
>  2 files changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index febdc45..c4c7064 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1190,9 +1190,10 @@ void ref_array_sort(struct ref_sorting *sorting, 
> struct ref_array *array)
>   qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
> compare_refs);
>  }
>  
> -static void print_value(struct atom_value *v, int quote_style, struct strbuf 
> *output)
> +static void print_value(struct atom_value *v, struct ref_formatting_state 
> *state,
> + struct strbuf *output)
>  {

I expect that the state would eventually become a stack of states
(i.e. the topmost one passed around, each having a pointer to the
previous level) in order to implement that "divert" mechanism for
(possibly nested) if ... end construct.

With that in mind, I suspect that state->output should be "where the
current level would output to", i.e. no need to pass state and
output around separately.

> +static void apply_formatting_state(struct ref_formatting_state *state, 
> struct strbuf *value,
> +struct strbuf *format)
> +{

The name "format" feels quite misleading; the readers would expect
that you would use it in "strbuf_addf(format, value)", but that is
not what is going on here.

> @@ -1275,12 +1299,13 @@ void show_ref_array_item(struct ref_array_item *info, 
> const char *format, int qu
>   if (color_parse("reset", color) < 0)
>   die("BUG: couldn't parse 'reset' as a color");
>   resetv.s = color;
> - print_value(&resetv, quote_style, &output);
> + print_value(&resetv, &state, &value);
> + apply_formatting_state(&state, &value, &final_buf);
>   }
> - for (i = 0; i < output.len; i++)
> - printf("%c", output.buf[i]);
> + for (i = 0; i < final_buf.len; i++)
> + printf("%c", final_buf.buf[i]);
>   putchar('\n');
> - strbuf_release(&output);
> + strbuf_release(&final_buf);
>  }
>  
>  /*  If no sorting option is given, use refname to sort as default */
> diff --git a/ref-filter.h b/ref-filter.h
> index 6bf27d8..b64677f 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -16,6 +16,10 @@
>  #define FILTER_REFS_INCLUDE_BROKEN 0x1
>  #define FILTER_REFS_ALL 0x2
>  
> +struct ref_formatting_state {
> + int quote_style;
> +};
> +
>  struct atom_value {
>   const char *s;
>   unsigned long ul; /* used for sorting when not FIELD_STR */
--
To unsubscribe from this list: send the line "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 v8 01/11] ref-filter: print output to strbuf for formatting

2015-08-03 Thread Junio C Hamano
Karthik Nayak  writes:

> Introduce a strbuf `output` which will act as a substitute rather than
> printing directly to stdout. This will be used for formatting
> eventually.
> ---

Missing sign-off; the patch looks like a good first step in a nice
direction.

>  ref-filter.c | 36 ++--
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 7561727..febdc45 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1190,30 +1190,25 @@ void ref_array_sort(struct ref_sorting *sorting, 
> struct ref_array *array)
>   qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
> compare_refs);
>  }
>  
> -static void print_value(struct atom_value *v, int quote_style)
> +static void print_value(struct atom_value *v, int quote_style, struct strbuf 
> *output)
>  {
> - struct strbuf sb = STRBUF_INIT;
>   switch (quote_style) {
>   case QUOTE_NONE:
> - fputs(v->s, stdout);
> + strbuf_addstr(output, v->s);
>   break;
>   case QUOTE_SHELL:
> - sq_quote_buf(&sb, v->s);
> + sq_quote_buf(output, v->s);
>   break;
>   case QUOTE_PERL:
> - perl_quote_buf(&sb, v->s);
> + perl_quote_buf(output, v->s);
>   break;
>   case QUOTE_PYTHON:
> - python_quote_buf(&sb, v->s);
> + python_quote_buf(output, v->s);
>   break;
>   case QUOTE_TCL:
> - tcl_quote_buf(&sb, v->s);
> + tcl_quote_buf(output, v->s);
>   break;
>   }
> - if (quote_style != QUOTE_NONE) {
> - fputs(sb.buf, stdout);
> - strbuf_release(&sb);
> - }
>  }
>  
>  static int hex1(char ch)
> @@ -1234,7 +1229,7 @@ static int hex2(const char *cp)
>   return -1;
>  }
>  
> -static void emit(const char *cp, const char *ep)
> +static void emit(const char *cp, const char *ep, struct strbuf *output)
>  {
>   while (*cp && (!ep || cp < ep)) {
>   if (*cp == '%') {
> @@ -1243,13 +1238,13 @@ static void emit(const char *cp, const char *ep)
>   else {
>   int ch = hex2(cp + 1);
>   if (0 <= ch) {
> - putchar(ch);
> + strbuf_addch(output, ch);
>   cp += 3;
>   continue;
>   }
>   }
>   }
> - putchar(*cp);
> + strbuf_addch(output, *cp);
>   cp++;
>   }
>  }
> @@ -1257,19 +1252,21 @@ static void emit(const char *cp, const char *ep)
>  void show_ref_array_item(struct ref_array_item *info, const char *format, 
> int quote_style)
>  {
>   const char *cp, *sp, *ep;
> + struct strbuf output = STRBUF_INIT;
> + int i;
>  
>   for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
>   struct atom_value *atomv;
>  
>   ep = strchr(sp, ')');
>   if (cp < sp)
> - emit(cp, sp);
> + emit(cp, sp, &output);
>   get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
> &atomv);
> - print_value(atomv, quote_style);
> + print_value(atomv, quote_style, &output);
>   }
>   if (*cp) {
>   sp = cp + strlen(cp);
> - emit(cp, sp);
> + emit(cp, sp, &output);
>   }
>   if (need_color_reset_at_eol) {
>   struct atom_value resetv;
> @@ -1278,9 +1275,12 @@ void show_ref_array_item(struct ref_array_item *info, 
> const char *format, int qu
>   if (color_parse("reset", color) < 0)
>   die("BUG: couldn't parse 'reset' as a color");
>   resetv.s = color;
> - print_value(&resetv, quote_style);
> + print_value(&resetv, quote_style, &output);
>   }
> + for (i = 0; i < output.len; i++)
> + printf("%c", output.buf[i]);
>   putchar('\n');
> + strbuf_release(&output);
>  }
>  
>  /*  If no sorting option is given, use refname to sort as default */
--
To unsubscribe from this list: send the line "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 v8 0/11] Port tag.c over to use ref-filter APIs

2015-08-03 Thread Junio C Hamano
Karthik Nayak  writes:

> This is part of my GSoC project to unify git tag -l, git branch -l,
> git for-each-ref.  This patch series is continued from: Git (next)
> https://github.com/git/git/commit/bf5418f49ff0cebc6e5ce04ad1417e1a47c81b61
>
> This series consists of porting tag.c over to using the ref-filter APIs
>
> Version 7 can be found here:
> http://thread.gmane.org/gmane.comp.version-control.git/274990
>
> Changes:
> * Make padright a general align atom.
> * Make print_value() and emit() output to a strbuf rather than stdout 
> directly.
>
> Interdiff:
>
> diff --git a/Documentation/git-for-each-ref.txt
> b/Documentation/git-for-each-ref.txt
> index bcf319a..e89b9b0 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -127,11 +127,12 @@ color::
>  Change output color.  Followed by `:`, where names
>  are described in `color.branch.*`.
>
> -padright::
> -Pad succeeding atom or string to the right. Followed by
> -`:`, where `value` states the total length of atom or
> -string including the padding. If the `value` is lesser than
> -the atom or string length, then no padding is performed.
> +align::
> +Align succeeding atoms to the right, left or middle. Followed
> +by `:,`, where the `` is either
> +left, right or middle and `` is the total
> +length of the padding to be performed. If the atom length is
> +more than the padding length then no padding is performed.

It is very very dissapointing to allow the "next atom only"
implementation to squat on a good name "align:,",
especially when I thought that the list agreed

  %(align:,) any string with or without %(atom) %(end)

would be the way to go.
--
To unsubscribe from this list: send the line "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: Draft of Git Rev News edition 6

2015-08-03 Thread Junio C Hamano
Thomas Ferris Nicolaisen  writes:

> A draft of Git Rev News edition 6 is available here:
>
> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-6.md
>
> Everyone is welcome to contribute in any section, either by editing the
> above page on GitHub and sending a pull request, or by commenting on
> this GitHub issue:
>
> https://github.com/git/git.github.io/issues/89
>
> You can also reply to this email.
>
> Me and Nicola are planning to ship this edition on Wednesday 5th of
> August (as Christian C. is away on vacation these days).

Thanks; it seems that the review section is rather thin.  No new and
interesting changes worth reviewing in the entire month of July?
--
To unsubscribe from this list: send the line "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 v1] send-email: provide whitelist of SMTP AUTH mechanisms

2015-08-03 Thread Junio C Hamano
Jan Viktorin  writes:

> I didn't find a way how to determine what mechanisms are supported by SASL.

Ok, forget the suggested approach, then X-<.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread David Turner
On Sat, 2015-08-01 at 08:51 +0200, Michael Haggerty wrote:
> On 08/01/2015 07:12 AM, Junio C Hamano wrote:
> > On Fri, Jul 31, 2015 at 8:59 PM, Michael Haggerty  
> > wrote:
> >>
> >> It seems to me that adding a new top-level "worktree-refs" directory is
> >> pretty traumatic. Lots of people and tools will have made the assumption
> >> that all "normal" references live under "refs/".
> >> ...
> >> It's all a bit frightening, frankly.
> > 
> > I actually feel the prospect of pluggable ref backend more frightening,
> > frankly ;-). These bisect refs are just like FETCH_HEAD and MERGE_HEAD,
> > not about the primary purpose of the "repository" to grow the history of 
> > refs
> > (branches), but about ephemeral pointers into the history used to help keep
> > track of what is being done in the worktree upstairs. There is no need for
> > these to be visible across worktrees. If we use the real refs that are 
> > grobal
> > in the repository (as opposed to per-worktree ones), we would hit the 
> > backend
> > databas with transactions to update these ephemeral things, which somehow
> > makes me feel stupid.
> 
> Hmm, ok, so you are thinking of a remote database with high latency. I
> was thinking more of something like LMDB, with latency comparable to
> filesystem storage.
> 
> These worktree-specific references might be ephemeral, but they also
> imply reachability, which means that they need to be visible at least
> during object pruning. Moreover, if the references don't live in the
> same database with the rest of the references, then we have to deal with
> races due to updating references in different places without atomicity.
> 
> The refs+object store is the most important thing for maintaining the
> integrity of a repo and avoiding races. To me it seems easier to do so
> if there is a single refs+objects store than if we have some references
> over here on the file system, some over there in a LMDB, etc. So my gut
> feeling is for the primary reference storage to be in a single reference
> namespace that (at least in principle) can be stored in a single ACID
> database.
>
> For each worktree, we could then create a different view of the
> references by splicing parts of the full reference namespace together.
> This could even be based on config settings so that we don't have to
> hardcode information like "refs/bisect/* is worktree-specific" deep in
> the references module. Suppose we could write
> 
> [worktree.refs]
>   map = refs/worktrees/*:
>   map = refs/bisect/*:refs/worktrees/[worktree]/refs/bisect/*
> 
> which would mean (a) hide the references under refs/worktrees", and (b)
> make it look as if the references under
> refs/worktrees/[worktree]/refs/bisect actually appear under refs/bisect
> (where "[worktree]" is replaced with the current worktree's name). By
> making these settings configurable, we allow other projects to define
> their own worktree-specific reference namespaces too.
> 
> The corresponding main repo might hide "refs/worktrees/*" but leave its
> refs/bisect namespace exposed in the usual place.
> 
> "git prune" would see the whole namespace as it really is so that it can
> compute reachability correctly.

I think making this configurable is (a) overkill and (b) dangerous.
It's dangerous because the semantics of which refs are per-worktree is
important to the correct operation of git, and allowing users to mess
with it seems like a big mistake.  Instead, we should figure out a
simple scheme and define it globally.

I think refs/worktree -> refs/worktrees/[worktree]/ would do fine as a
fixed scheme, if we go that route.

We would need two separate views of the refs hierarchy, though: one used
by prune (and pack-refs) that is non-mapped (that is, includes
per-worktree refs for each worktree), and one for general use that is
mapped.   Maybe this is just a flag to the ref traversal functions.

But I'm not sure that this is really the right way to go.

As I understand it, we don't presently do many transactions that include
both pseudorefs or per-worktree refs and other refs.  And we definitely
don't want to move pseudorefs into the database since there's so much
code that assumes they're files.  Also, the vast majority of refs are
common, rather than per-worktree.  In fact, the only per-worktree refs
I've seen mentioned so far are the bisect refs and NOTES_MERGE_REF and
HEAD.  Of these, only HEAD is needed for pruning. Are there more that I
haven't thought of?

So I'm not sure the gain from moving per-worktree refs into the database
is that great.

There are some downsides of moving per-worktree refs into the database:

1. More operations in one worktree can now contend with operations in
another worktree for the database.  LMDB only allows a single write
transaction at a time.  

2. The refs API would be more complicated: it would need to deal with
remapped vs raw ref paths.  Refs backends would need to have functions
to prune per-worktree data when a worktree is destroyed. 

Re: [RFC/PATCH 2/2] Testing the new code

2015-08-03 Thread Junio C Hamano
Stefan Beller  writes:

> On Fri, Jul 31, 2015 at 6:02 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> -module_list()
>>> +module_list_shell()
>>>  {
>>>   eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
>>>   (
>>> @@ -187,6 +187,29 @@ module_list()
>>>   '
>>>  }
>>>
>>> +module_list()
>>> +{
>>> + # call both the old and new code
>>> + module_list_shell $@ >/u/git_submodule_module_list_shell 
>>> 2>/u/git_submodule_module_list_shell2
>>> + git submodule--helper --module_list $@ >/u/git_submodule_module_list 
>>> 2>/u/git_submodule_module_list2
>>
>> You seem to be discarding the double-quote around $@ in both of
>> these two places.  Intended?
>
> No, not at all. This was a bit sloppy.

OK.

> This patch was rather showing off how I intend to test the previous patch.

Yeah, I can see what the code is doing, and you already saw that I
didn't disagree with the approach ;).  During a reimplementation
exercise, it often is a good idea, if the code structure allows you
to, to run both implementations and compare the results---but it can
go only so far.  It obviously is tricky to apply the trick to an
operation that is not idempotent to let two implementations to do it
twice in different ways and make sure they produce the same result.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 5/6] clone: fix hostname parsing when guessing dir

2015-08-03 Thread Junio C Hamano
Jeff King  writes:

> On Mon, Aug 03, 2015 at 10:34:14AM +0200, Patrick Steinhardt wrote:
>
>> One more question for backwards compatibility remains then.
>> Currently when we clone something like 'http://example.com:/'
>> we'd create a git repository '' as we'd split on the first
>> occurrence of ':'. Should we remain backwards compatible here, as
>> well, or change the behavior to use 'example.com' as repository
>> name?
>
> I don't think naming the repo "" makes much sense; I'd consider it a
> bug. The only sensible names are "example.com" or "example.com:"
> (the latter is more specific if you are going to clone the root off of
> several different ports, but that seems rather unlikely; the former is
> probably what I'd expect).

Yeah, I tend to agree.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 1/2] worktrees: add find_shared_symref

2015-08-03 Thread David Turner
Add a new function, find_shared_symref, which contains the heart of
die_if_checked_out, but works for any symref, not just HEAD.  Refactor
die_if_checked_out to use the same infrastructure as
find_shared_symref.

Soon, we will use find_shared_symref to protect notes merges in
worktrees.

Signed-off-by: David Turner 
---

This reroll fixes issues reported by Eric Sunshine: leaks
and Johan Herland: prepositions and broken &&

---
 branch.c | 45 ++---
 branch.h |  8 
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/branch.c b/branch.c
index c85be07..d2b3586 100644
--- a/branch.c
+++ b/branch.c
@@ -311,21 +311,22 @@ void remove_branch_state(void)
unlink(git_path("SQUASH_MSG"));
 }
 
-static void check_linked_checkout(const char *branch, const char *id)
+static char *find_linked_symref(const char *symref, const char *branch,
+   const char *id)
 {
struct strbuf sb = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
 
/*
-* $GIT_COMMON_DIR/HEAD is practically outside
-* $GIT_DIR so resolve_ref_unsafe() won't work (it
-* uses git_path). Parse the ref ourselves.
+* $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside
+* $GIT_DIR so resolve_ref_unsafe() won't work (it uses
+* git_path). Parse the ref ourselves.
 */
if (id)
-   strbuf_addf(&path, "%s/worktrees/%s/HEAD", 
get_git_common_dir(), id);
+   strbuf_addf(&path, "%s/worktrees/%s/%s", get_git_common_dir(), 
id, symref);
else
-   strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+   strbuf_addf(&path, "%s/%s", get_git_common_dir(), symref);
 
if (!strbuf_readlink(&sb, path.buf, 0)) {
if (!starts_with(sb.buf, "refs/") ||
@@ -349,31 +350,53 @@ static void check_linked_checkout(const char *branch, 
const char *id)
strbuf_addstr(&gitdir, get_git_common_dir());
skip_prefix(branch, "refs/heads/", &branch);
strbuf_strip_suffix(&gitdir, ".git");
-   die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
+
+   strbuf_release(&path);
+   strbuf_release(&sb);
+   return strbuf_detach(&gitdir, NULL);
 done:
strbuf_release(&path);
strbuf_release(&sb);
strbuf_release(&gitdir);
+
+   return NULL;
 }
 
-void die_if_checked_out(const char *branch)
+char *find_shared_symref(const char *symref, const char *target)
 {
struct strbuf path = STRBUF_INIT;
DIR *dir;
struct dirent *d;
+   char *existing;
 
-   check_linked_checkout(branch, NULL);
+   if ((existing = find_linked_symref(symref, target, NULL)))
+   return existing;
 
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
strbuf_release(&path);
if (!dir)
-   return;
+   return NULL;
 
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
-   check_linked_checkout(branch, d->d_name);
+   existing = find_linked_symref(symref, target, d->d_name);
+   if (existing) {
+   closedir(dir);
+   return existing;
+   }
}
closedir(dir);
+
+   return NULL;
+}
+
+void die_if_checked_out(const char *branch)
+{
+   char *existing;
+
+   existing = find_shared_symref("HEAD", branch);
+   if (existing)
+   die(_("'%s' is already checked out at '%s'"), branch, existing);
 }
diff --git a/branch.h b/branch.h
index 58aa45f..d3446ed 100644
--- a/branch.h
+++ b/branch.h
@@ -59,4 +59,12 @@ extern int read_branch_desc(struct strbuf *, const char 
*branch_name);
  */
 extern void die_if_checked_out(const char *branch);
 
+/*
+ * Check if a per-worktree symref points to a ref in the main worktree
+ * or any linked worktree, and return the path to the exising worktree
+ * if it is.  Returns NULL if there is no existing ref.  The caller is
+ * responsible for freeing the returned path.
+ */
+extern char *find_shared_symref(const char *symref, const char *target);
+
 #endif
-- 
2.0.4.315.gad8727a-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 2/2] notes: handle multiple worktrees

2015-08-03 Thread David Turner
Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
find_shared_symref and die if we find one.  This prevents simultaneous
merges to the same notes branch from different worktrees.

Signed-off-by: David Turner 
---
 builtin/notes.c  |  6 
 t/t3320-notes-merge-worktrees.sh | 72 
 2 files changed, 78 insertions(+)
 create mode 100755 t/t3320-notes-merge-worktrees.sh

diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc..0423480 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -19,6 +19,7 @@
 #include "string-list.h"
 #include "notes-merge.h"
 #include "notes-utils.h"
+#include "branch.h"
 
 static const char * const git_notes_usage[] = {
N_("git notes [--ref ] [list []]"),
@@ -825,10 +826,15 @@ static int merge(int argc, const char **argv, const char 
*prefix)
update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
   0, UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
+   char *existing;
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
   0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
+   existing = find_shared_symref("NOTES_MERGE_REF", 
default_notes_ref());
+   if (existing)
+   die(_("A notes merge into %s is already in-progress at 
%s"),
+   default_notes_ref(), existing);
if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
die("Failed to store link to current notes ref (%s)",
default_notes_ref());
diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
new file mode 100755
index 000..a7beef2
--- /dev/null
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Twitter, Inc
+#
+
+test_description='Test merging of notes trees in multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup commit' '
+   test_commit tantrum
+'
+
+commit_tantrum=$(git rev-parse tantrum^{commit})
+
+test_expect_success 'setup notes ref (x)' '
+   git config core.notesRef refs/notes/x &&
+   git notes add -m "x notes on tantrum" tantrum
+'
+
+test_expect_success 'setup local branch (y)' '
+   git update-ref refs/notes/y refs/notes/x &&
+   git config core.notesRef refs/notes/y &&
+   git notes remove tantrum
+'
+
+test_expect_success 'setup remote branch (z)' '
+   git update-ref refs/notes/z refs/notes/x &&
+   git config core.notesRef refs/notes/z &&
+   git notes add -f -m "conflicting notes on tantrum" tantrum
+'
+
+test_expect_success 'modify notes ref ourselves (x)' '
+   git config core.notesRef refs/notes/x &&
+   git notes add -f -m "more conflicting notes on tantrum" tantrum
+'
+
+test_expect_success 'create some new worktrees' '
+   git worktree add -b newbranch worktree master &&
+   git worktree add -b newbranch2 worktree2 master
+'
+
+test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
+   git config core.notesRef refs/notes/y &&
+   test_must_fail git notes merge z &&
+   echo "ref: refs/notes/y" > expect &&
+   test_cmp .git/NOTES_MERGE_REF expect
+'
+
+test_expect_success 'merge z into y while mid-merge in another workdir fails' '
+   (
+   cd worktree &&
+   git config core.notesRef refs/notes/y &&
+   test_must_fail git notes merge z 2>err &&
+   grep "A notes merge into refs/notes/y is already in-progress 
at" err
+   ) &&
+   test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+'
+
+test_expect_success 'merge z into x while mid-merge on y succeeds' '
+   (
+   cd worktree2 &&
+   git config core.notesRef refs/notes/x &&
+   test_must_fail git notes merge z 2>&1 >out &&
+   grep "Automatic notes merge failed" out &&
+   grep -v "A notes merge into refs/notes/x is already in-progress 
in" out
+   ) &&
+   echo "ref: refs/notes/x" > expect &&
+   test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
+'
+
+test_done
-- 
2.0.4.315.gad8727a-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] notes: handle multiple worktrees

2015-08-03 Thread David Turner
On Sat, 2015-08-01 at 15:51 +0200, Johan Herland wrote:
> On Sat, Aug 1, 2015 at 12:11 AM, David Turner  
> wrote:
> > Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
> > find_shared_symref and die if we find one.  This prevents simultaneous
> > merges to the same notes branch from different worktrees.
> >
> > Signed-off-by: David Turner 
> > ---
> >  builtin/notes.c  |  5 +++
> >  t/t3320-notes-merge-worktrees.sh | 72 
> > 
> >  2 files changed, 77 insertions(+)
> >  create mode 100755 t/t3320-notes-merge-worktrees.sh
> >
> > diff --git a/builtin/notes.c b/builtin/notes.c
> > index 63f95fc..e4dda79 100644
> > --- a/builtin/notes.c
> > +++ b/builtin/notes.c
> > @@ -19,6 +19,7 @@
> >  #include "string-list.h"
> >  #include "notes-merge.h"
> >  #include "notes-utils.h"
> > +#include "branch.h"
> >
> >  static const char * const git_notes_usage[] = {
> > N_("git notes [--ref ] [list []]"),
> > @@ -825,10 +826,14 @@ static int merge(int argc, const char **argv, const 
> > char *prefix)
> > update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
> >0, UPDATE_REFS_DIE_ON_ERR);
> > else { /* Merge has unresolved conflicts */
> > +   char *existing;
> > /* Update .git/NOTES_MERGE_PARTIAL with partial merge 
> > result */
> > update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, 
> > NULL,
> >0, UPDATE_REFS_DIE_ON_ERR);
> > /* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
> > +   existing = find_shared_symref("NOTES_MERGE_REF", 
> > default_notes_ref());
> 
> Please confirm my assumption here: existing originally comes from a
> strbuf_detach(), so it's the caller's (i.e. our) responsibility to
> free() it, but we don't care, as we just die()d anyway. Correct?

Confirmed.

 I will fix the other issues you reported.

--
To unsubscribe from this list: send the line "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] builtin/mv: Get rid of the last caller of get_pathspec

2015-08-03 Thread Stefan Beller
`get_pathspec` is deprecated and builtin/mv is its last caller, so getting
rid of `get_pathspec` is rather easy. By getting rid of `get_pathspec`,
the documentation such as 'technical/api-setup.txt' becomes easier to read
as the reader doesn't need to bear with the additional fact that
`get_pathspec` is deprecated.

The code in 'builtin/mv' still requires some work to make it less ugly.

CC: Johannes Schindelin 
CC: Junio C Hamano 
CC: Nguyễn Thái Ngọc Duy 
Signed-off-by: Stefan Beller 
---
 Documentation/technical/api-setup.txt |  2 --
 builtin/mv.c  | 19 ---
 cache.h   |  1 -
 pathspec.c| 30 --
 4 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/Documentation/technical/api-setup.txt 
b/Documentation/technical/api-setup.txt
index 540e455..eb1fa98 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt
@@ -27,8 +27,6 @@ parse_pathspec(). This function takes several arguments:
 
 - prefix and args come from cmd_* functions
 
-get_pathspec() is obsolete and should never be used in new code.
-
 parse_pathspec() helps catch unsupported features and reject them
 politely. At a lower level, different pathspec-related functions may
 not support the same set of features. Such pathspec-sensitive
diff --git a/builtin/mv.c b/builtin/mv.c
index d1d4316..b89d90a 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -10,6 +10,7 @@
 #include "string-list.h"
 #include "parse-options.h"
 #include "submodule.h"
+#include "pathspec.h"
 
 static const char * const builtin_mv_usage[] = {
N_("git mv [] ... "),
@@ -20,13 +21,19 @@ static const char * const builtin_mv_usage[] = {
 #define KEEP_TRAILING_SLASH 2
 
 static const char **internal_copy_pathspec(const char *prefix,
-  const char **pathspec,
+  const char **argv,
   int count, unsigned flags)
 {
int i;
+   struct pathspec ps;
const char **result = xmalloc((count + 1) * sizeof(const char *));
-   memcpy(result, pathspec, count * sizeof(const char *));
+   memcpy(result, argv, count * sizeof(const char *));
result[count] = NULL;
+
+   /*
+* NEEDSWORK: instead of preprocessing, pass the right flags to
+* parse_pathspec below.
+*/
for (i = 0; i < count; i++) {
int length = strlen(result[i]);
int to_copy = length;
@@ -42,7 +49,13 @@ static const char **internal_copy_pathspec(const char 
*prefix,
result[i] = it;
}
}
-   return get_pathspec(prefix, result);
+
+   parse_pathspec(&ps,
+  PATHSPEC_ALL_MAGIC &
+  ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
+  PATHSPEC_PREFER_CWD,
+  prefix, result);
+   return ps._raw;
 }
 
 static const char *add_slash(const char *path)
diff --git a/cache.h b/cache.h
index 4f55466..d4e22e2 100644
--- a/cache.h
+++ b/cache.h
@@ -452,7 +452,6 @@ extern void set_git_work_tree(const char *tree);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
-extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern void setup_work_tree(void);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
diff --git a/pathspec.c b/pathspec.c
index 9304ee3..b0e14e5 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -450,36 +450,6 @@ void parse_pathspec(struct pathspec *pathspec,
}
 }
 
-/*
- * N.B. get_pathspec() is deprecated in favor of the "struct pathspec"
- * based interface - see pathspec.c:parse_pathspec().
- *
- * Arguments:
- *  - prefix - a path relative to the root of the working tree
- *  - pathspec - a list of paths underneath the prefix path
- *
- * Iterates over pathspec, prepending each path with prefix,
- * and return the resulting list.
- *
- * If pathspec is empty, return a singleton list containing prefix.
- *
- * If pathspec and prefix are both empty, return an empty list.
- *
- * This is typically used by built-in commands such as add.c, in order
- * to normalize argv arguments provided to the built-in into a list of
- * paths to process, all relative to the root of the working tree.
- */
-const char **get_pathspec(const char *prefix, const char **pathspec)
-{
-   struct pathspec ps;
-   parse_pathspec(&ps,
-  PATHSPEC_ALL_MAGIC &
-  ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
-  PATHSPEC_PREFER_CWD,
-  prefix, pathspec);
-   return ps._raw;
-}
-
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
 {
*dst = *src;
-- 
2.5.0.2.g6ffee06.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in

[PATCH v8 11/11] tag.c: implement '--merged' and '--no-merged' options

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Using 'ref-filter' APIs implement the '--merged' and '--no-merged'
options into 'tag.c'. The '--merged' option lets the user to only
list tags merged into the named commit. The '--no-merged' option
lets the user to only list tags not merged into the named commit.
If no object is provided it assumes HEAD as the object.

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-tag.txt |  7 ++-
 builtin/tag.c |  6 +-
 t/t7004-tag.sh| 27 +++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 75703c5..c2785d9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
-   [--format=] [...]
+   [--format=] [--[no-]merged []] [...]
 'git tag' -v ...
 
 DESCRIPTION
@@ -171,6 +171,11 @@ This option is only applicable when listing tags without 
annotation lines.
`%0a` to `\n` (LF).  The fields are same as those in `git
for-each-ref`.
 
+--[no-]merged []::
+   Only list tags whose tips are reachable, or not reachable
+   if '--no-merged' is used, from the specified commit ('HEAD'
+   if not specified).
+
 
 CONFIGURATION
 -
diff --git a/builtin/tag.c b/builtin/tag.c
index 13c9579..529b29f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m  | -F ] 
 []"),
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
-   "\n\t\t[...]"),
+   "\n\t\t[--[no-]merged []] [...]"),
N_("git tag -v ..."),
NULL
 };
@@ -353,6 +353,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_COLUMN(0, "column", &colopts, N_("show tag list in 
columns")),
OPT_CONTAINS(&filter.with_commit, N_("print only tags that 
contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only tags that contain 
the commit")),
+   OPT_MERGED(&filter, N_("print only tags that are merged")),
+   OPT_NO_MERGED(&filter, N_("print only tags that are not 
merged")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
 N_("field name to sort on"), 
&parse_opt_ref_sorting),
{
@@ -413,6 +415,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("--contains option is only allowed with -l."));
if (filter.points_at.nr)
die(_("--points-at option is only allowed with -l."));
+   if (filter.merge_commit)
+   die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
if (cmdmode == 'v')
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1809011..5b73539 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1535,4 +1535,31 @@ test_expect_success '--format should list tags as per 
format given' '
test_cmp expect actual
 '
 
+test_expect_success 'setup --merged test tags' '
+   git tag mergetest-1 HEAD~2 &&
+   git tag mergetest-2 HEAD~1 &&
+   git tag mergetest-3 HEAD
+'
+
+test_expect_success '--merged cannot be used in non-list mode' '
+   test_must_fail git tag --merged=mergetest-2 foo
+'
+
+test_expect_success '--merged shows merged tags' '
+   cat >expect <<-\EOF &&
+   mergetest-1
+   mergetest-2
+   EOF
+   git tag -l --merged=mergetest-2 mergetest-* >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success '--no-merged show unmerged tags' '
+   cat >expect <<-\EOF &&
+   mergetest-3
+   EOF
+   git tag -l --no-merged=mergetest-2 mergetest-* >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 09/11] tag.c: use 'ref-filter' APIs

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
and printing of refs. This removes most of the code used in 'tag.c'
replacing it with calls to the 'ref-filter' library.

Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'
to filter out tags based on the options set.

For printing tags we use 'show_ref_array_item()' function provided by
'ref-filter'.

We improve the sorting option provided by 'tag.c' by using the sorting
options provided by 'ref-filter'. This causes the test 'invalid sort
parameter on command line' in t7004 to fail, as 'ref-filter' throws an
error for all sorting fields which are incorrect. The test is changed
to reflect the same.

Modify documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-tag.txt |  16 ++-
 builtin/tag.c | 342 ++
 t/t7004-tag.sh|   8 +-
 3 files changed, 50 insertions(+), 316 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 84f6496..3ac4a96 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [--create-reflog] [...]
+   [--column[=] | --no-column] [--create-reflog] [--sort=] 
[...]
 'git tag' -v ...
 
 DESCRIPTION
@@ -94,14 +94,16 @@ OPTIONS
using fnmatch(3)).  Multiple patterns may be given; if any of
them matches, the tag is shown.
 
---sort=::
-   Sort in a specific order. Supported type is "refname"
-   (lexicographic order), "version:refname" or "v:refname" (tag
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
names are treated as versions). The "version:refname" sort
order can also be affected by the
-   "versionsort.prereleaseSuffix" configuration variable. Prepend
-   "-" to reverse sort order. When this option is not given, the
-   sort order defaults to the value configured for the 'tag.sort'
+   "versionsort.prereleaseSuffix" configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+   Sort order defaults to the value configured for the 'tag.sort'
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
 
diff --git a/builtin/tag.c b/builtin/tag.c
index e96bae2..829af6f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -28,278 +28,32 @@ static const char * const git_tag_usage[] = {
NULL
 };
 
-#define STRCMP_SORT 0  /* must be zero */
-#define VERCMP_SORT 1
-#define SORT_MASK   0x7fff
-#define REVERSE_SORT0x8000
-
-static int tag_sort;
-
 static unsigned int colopts;
 
-static int match_pattern(const char **patterns, const char *ref)
-{
-   /* no pattern means match everything */
-   if (!*patterns)
-   return 1;
-   for (; *patterns; patterns++)
-   if (!wildmatch(*patterns, ref, 0, NULL))
-   return 1;
-   return 0;
-}
-
-/*
- * This is currently duplicated in ref-filter.c, and will eventually be
- * removed as we port tag.c to use the ref-filter APIs.
- */
-static const unsigned char *match_points_at(const char *refname,
-   const unsigned char *sha1,
-   struct sha1_array *points_at)
-{
-   const unsigned char *tagged_sha1 = NULL;
-   struct object *obj;
-
-   if (sha1_array_lookup(points_at, sha1) >= 0)
-   return sha1;
-   obj = parse_object(sha1);
-   if (!obj)
-   die(_("malformed object at '%s'"), refname);
-   if (obj->type == OBJ_TAG)
-   tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
-   if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
-   return tagged_sha1;
-   return NULL;
-}
-
-static int in_commit_list(const struct commit_list *want, struct commit *c)
-{
-   for (; want; want = want->next)
-   if (!hashcmp(want->item->object.sha1, c->object.sha1))
-   return 1;
-   return 0;
-}
-
-/*
- * The entire code segment for supporting the --contains option has been
- * copied over to ref-filter.{c,h}. This will be deleted evetually when
- * we port tag.c to use ref-filter APIs.
- */
-enum contains_result {
-   CONTAINS_UNKNOWN = -1,
-   CONTAINS_NO = 0,
-   CONTAINS_YES = 1
-};
-
-/*
- * Test whether the candidate or one of its parents is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
- */
-static enum contains_re

[PATCH v8 05/11] ref-filter: support printing N lines from tag annotation

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

In 'tag.c' we can print N lines from the annotation of the tag using
the '-n' option. Copy code from 'tag.c' to 'ref-filter' and
modify 'ref-filter' to support printing of N lines from the annotation
of tags.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/for-each-ref.c |  2 +-
 builtin/tag.c  |  4 
 ref-filter.c   | 54 --
 ref-filter.h   |  9 +++--
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 40f343b..e4a4f8a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -74,7 +74,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
if (!maxcount || array.nr < maxcount)
maxcount = array.nr;
for (i = 0; i < maxcount; i++)
-   show_ref_array_item(array.items[i], format, quote_style);
+   show_ref_array_item(array.items[i], format, quote_style, 0);
ref_array_clear(&array);
return 0;
 }
diff --git a/builtin/tag.c b/builtin/tag.c
index 471d6b1..0fc7557 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -185,6 +185,10 @@ static enum contains_result contains(struct commit 
*candidate,
return contains_test(candidate, want);
 }
 
+/*
+ * Currently duplicated in ref-filter, will eventually be removed as
+ * we port tag.c to use ref-filter APIs.
+ */
 static void show_tag_lines(const struct object_id *oid, int lines)
 {
int i;
diff --git a/ref-filter.c b/ref-filter.c
index 01c9097..9f3806a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1318,7 +1318,51 @@ static void apply_formatting_state(struct 
ref_formatting_state *state, struct st
 
 }
 
-void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+/*
+ * If 'lines' is greater than 0, print that many lines from the given
+ * object_id 'oid'.
+ */
+static void show_tag_lines(const struct object_id *oid, int lines)
+{
+   int i;
+   unsigned long size;
+   enum object_type type;
+   char *buf, *sp, *eol;
+   size_t len;
+
+   buf = read_sha1_file(oid->hash, &type, &size);
+   if (!buf)
+   die_errno("unable to read object %s", oid_to_hex(oid));
+   if (type != OBJ_COMMIT && type != OBJ_TAG)
+   goto free_return;
+   if (!size)
+   die("an empty %s object %s?",
+   typename(type), oid_to_hex(oid));
+
+   /* skip header */
+   sp = strstr(buf, "\n\n");
+   if (!sp)
+   goto free_return;
+
+   /* only take up to "lines" lines, and strip the signature from a tag */
+   if (type == OBJ_TAG)
+   size = parse_signature(buf, size);
+   for (i = 0, sp += 2; i < lines && sp < buf + size; i++) {
+   if (i)
+   printf("\n");
+   eol = memchr(sp, '\n', size - (sp - buf));
+   len = eol ? eol - sp : size - (sp - buf);
+   fwrite(sp, len, 1, stdout);
+   if (!eol)
+   break;
+   sp = eol + 1;
+   }
+free_return:
+   free(buf);
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format,
+int quote_style, unsigned int lines)
 {
const char *cp, *sp, *ep;
struct strbuf value = STRBUF_INIT;
@@ -1363,8 +1407,14 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
 
for (i = 0; i < final_buf.len; i++)
printf("%c", final_buf.buf[i]);
-   putchar('\n');
strbuf_release(&final_buf);
+
+   if (lines > 0) {
+   struct object_id oid;
+   hashcpy(oid.hash, info->objectname);
+   show_tag_lines(&oid, lines);
+   }
+   putchar('\n');
 }
 
 /*  If no sorting option is given, use refname to sort as default */
diff --git a/ref-filter.h b/ref-filter.h
index 8d4e348..16cffab 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -71,6 +71,7 @@ struct ref_filter {
struct commit *merge_commit;
 
unsigned int with_commit_tag_algo : 1;
+   unsigned int lines;
 };
 
 struct ref_filter_cbdata {
@@ -102,8 +103,12 @@ int parse_ref_filter_atom(const char *atom, const char 
*ep);
 int verify_ref_format(const char *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
-/*  Print the ref using the given format and quote_style */
-void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style);
+/*
+ * Print the ref using the given format and quote_style. If 'lines' > 0,
+ * print that many lines of the the given ref.
+ */
+void show_ref_array_item(struct ref_array_item *info, const char *format,
+int quote_style, unsigned int lines);
 /*  Callback function 

[PATCH v8 07/11] ref-filter: add option to match literal pattern

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Since 'ref-filter' only has an option to match path names add an
option for plain fnmatch pattern-matching.

This is to support the pattern matching options which are used in `git
tag -l` and `git branch -l` where we can match patterns like `git tag
-l foo*` which would match all tags which has a "foo*" pattern.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/for-each-ref.c |  1 +
 ref-filter.c   | 39 ---
 ref-filter.h   |  3 ++-
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e4a4f8a..3ad6a64 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -68,6 +68,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
git_config(git_default_config, NULL);
 
filter.name_patterns = argv;
+   filter.match_as_path = 1;
filter_refs(&array, &filter, FILTER_REFS_ALL | 
FILTER_REFS_INCLUDE_BROKEN);
ref_array_sort(sorting, &array);
 
diff --git a/ref-filter.c b/ref-filter.c
index 1bc6d4b..afeab37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -950,9 +950,32 @@ static int commit_contains(struct ref_filter *filter, 
struct commit *commit)
 
 /*
  * Return 1 if the refname matches one of the patterns, otherwise 0.
+ * A pattern can be a literal prefix (e.g. a refname "refs/heads/master"
+ * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
+ * matches "refs/heads/mas*", too).
+ */
+static int match_pattern(const char **patterns, const char *refname)
+{
+   /*
+* When no '--format' option is given we need to skip the prefix
+* for matching refs of tags and branches.
+*/
+   (void)(skip_prefix(refname, "refs/tags/", &refname) ||
+  skip_prefix(refname, "refs/heads/", &refname) ||
+  skip_prefix(refname, "refs/remotes/", &refname));
+
+   for (; *patterns; patterns++) {
+   if (!wildmatch(*patterns, refname, 0, NULL))
+   return 1;
+   }
+   return 0;
+}
+
+/*
+ * Return 1 if the refname matches one of the patterns, otherwise 0.
  * A pattern can be path prefix (e.g. a refname "refs/heads/master"
- * matches a pattern "refs/heads/") or a wildcard (e.g. the same ref
- * matches "refs/heads/m*",too).
+ * matches a pattern "refs/heads/" but not "refs/heads/m") or a
+ * wildcard (e.g. the same ref matches "refs/heads/m*", too).
  */
 static int match_name_as_path(const char **pattern, const char *refname)
 {
@@ -973,6 +996,16 @@ static int match_name_as_path(const char **pattern, const 
char *refname)
return 0;
 }
 
+/* Return 1 if the refname matches one of the patterns, otherwise 0. */
+static int filter_pattern_match(struct ref_filter *filter, const char *refname)
+{
+   if (!*filter->name_patterns)
+   return 1; /* No pattern always matches */
+   if (filter->match_as_path)
+   return match_name_as_path(filter->name_patterns, refname);
+   return match_pattern(filter->name_patterns, refname);
+}
+
 /*
  * Given a ref (sha1, refname), check if the ref belongs to the array
  * of sha1s. If the given ref is a tag, check if the given tag points
@@ -1041,7 +1074,7 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
return 0;
}
 
-   if (*filter->name_patterns && 
!match_name_as_path(filter->name_patterns, refname))
+   if (!filter_pattern_match(filter, refname))
return 0;
 
if (filter->points_at.nr && !match_points_at(&filter->points_at, 
oid->hash, refname))
diff --git a/ref-filter.h b/ref-filter.h
index c41432f..b3b9cd8 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -71,7 +71,8 @@ struct ref_filter {
} merge;
struct commit *merge_commit;
 
-   unsigned int with_commit_tag_algo : 1;
+   unsigned int with_commit_tag_algo : 1,
+   match_as_path : 1;
unsigned int lines;
 };
 
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 08/11] tag.c: use 'ref-filter' data structures

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Make 'tag.c' use 'ref-filter' data structures and make changes to
support the new data structures. This is a part of the process
of porting 'tag.c' to use 'ref-filter' APIs.

This is a temporary step before porting 'tag.c' to use 'ref-filter'
completely. As this is a temporary step, most of the code
introduced here will be removed when 'tag.c' is ported over to use
'ref-filter' APIs

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/tag.c | 106 +++---
 1 file changed, 57 insertions(+), 49 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 0fc7557..e96bae2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -17,6 +17,7 @@
 #include "gpg-interface.h"
 #include "sha1-array.h"
 #include "column.h"
+#include "ref-filter.h"
 
 static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m  | -F ] 
 []"),
@@ -34,15 +35,6 @@ static const char * const git_tag_usage[] = {
 
 static int tag_sort;
 
-struct tag_filter {
-   const char **patterns;
-   int lines;
-   int sort;
-   struct string_list tags;
-   struct commit_list *with_commit;
-};
-
-static struct sha1_array points_at;
 static unsigned int colopts;
 
 static int match_pattern(const char **patterns, const char *ref)
@@ -61,19 +53,20 @@ static int match_pattern(const char **patterns, const char 
*ref)
  * removed as we port tag.c to use the ref-filter APIs.
  */
 static const unsigned char *match_points_at(const char *refname,
-   const unsigned char *sha1)
+   const unsigned char *sha1,
+   struct sha1_array *points_at)
 {
const unsigned char *tagged_sha1 = NULL;
struct object *obj;
 
-   if (sha1_array_lookup(&points_at, sha1) >= 0)
+   if (sha1_array_lookup(points_at, sha1) >= 0)
return sha1;
obj = parse_object(sha1);
if (!obj)
die(_("malformed object at '%s'"), refname);
if (obj->type == OBJ_TAG)
tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
-   if (tagged_sha1 && sha1_array_lookup(&points_at, tagged_sha1) >= 0)
+   if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
return tagged_sha1;
return NULL;
 }
@@ -228,12 +221,24 @@ free_return:
free(buf);
 }
 
+static void ref_array_append(struct ref_array *array, const char *refname)
+{
+   size_t len = strlen(refname);
+   struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + 
len + 1);
+   memcpy(ref->refname, refname, len);
+   ref->refname[len] = '\0';
+   REALLOC_ARRAY(array->items, array->nr + 1);
+   array->items[array->nr++] = ref;
+}
+
 static int show_reference(const char *refname, const struct object_id *oid,
  int flag, void *cb_data)
 {
-   struct tag_filter *filter = cb_data;
+   struct ref_filter_cbdata *data = cb_data;
+   struct ref_array *array = data->array;
+   struct ref_filter *filter = data->filter;
 
-   if (match_pattern(filter->patterns, refname)) {
+   if (match_pattern(filter->name_patterns, refname)) {
if (filter->with_commit) {
struct commit *commit;
 
@@ -244,12 +249,12 @@ static int show_reference(const char *refname, const 
struct object_id *oid,
return 0;
}
 
-   if (points_at.nr && !match_points_at(refname, oid->hash))
+   if (filter->points_at.nr && !match_points_at(refname, 
oid->hash, &filter->points_at))
return 0;
 
if (!filter->lines) {
-   if (filter->sort)
-   string_list_append(&filter->tags, refname);
+   if (tag_sort)
+   ref_array_append(array, refname);
else
printf("%s\n", refname);
return 0;
@@ -264,36 +269,36 @@ static int show_reference(const char *refname, const 
struct object_id *oid,
 
 static int sort_by_version(const void *a_, const void *b_)
 {
-   const struct string_list_item *a = a_;
-   const struct string_list_item *b = b_;
-   return versioncmp(a->string, b->string);
+   const struct ref_array_item *a = *((struct ref_array_item **)a_);
+   const struct ref_array_item *b = *((struct ref_array_item **)b_);
+   return versioncmp(a->refname, b->refname);
 }
 
-static int list_tags(const char **patterns, int lines,
-struct commit_list *with_commit, int sort)
+static int list_tags(struct ref_filter *filter, int sort)
 {
-   struct tag_filter filter;
+   struct ref_array array;
+   struct ref_filter_cbdata data;
+
+   memset(&array, 0, siz

[PATCH v8 06/11] ref-filter: add support to sort by version

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Add support to sort by version using the "v:refname" and
"version:refname" option. This is achieved by using the 'versioncmp()'
function as the comparing function for qsort.

This option is included to support sorting by versions in `git tag -l`
which will eventaully be ported to use ref-filter APIs.

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  3 +++
 ref-filter.c   | 15 ++-
 ref-filter.h   |  3 ++-
 t/t6302-for-each-ref-filter.sh | 36 
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index bba6d83..e89b9b0 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -152,6 +152,9 @@ For sorting purposes, fields with numeric values sort in 
numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
 All other fields are used to sort in their byte-value order.
 
+There is also an option to sort by versions, this can be done by using
+the fieldname `version:refname` or its alias `v:refname`.
+
 In any case, a field name that refers to a field inapplicable to
 the object referred by the ref does not cause an error.  It
 returns an empty string instead.
diff --git a/ref-filter.c b/ref-filter.c
index 9f3806a..1bc6d4b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -11,6 +11,8 @@
 #include "ref-filter.h"
 #include "revision.h"
 #include "utf8.h"
+#include "git-compat-util.h"
+#include "version.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -1175,19 +1177,19 @@ static int cmp_ref_sorting(struct ref_sorting *s, 
struct ref_array_item *a, stru
 
get_ref_atom_value(a, s->atom, &va);
get_ref_atom_value(b, s->atom, &vb);
-   switch (cmp_type) {
-   case FIELD_STR:
+   if (s->version)
+   cmp = versioncmp(va->s, vb->s);
+   else if (cmp_type == FIELD_STR)
cmp = strcmp(va->s, vb->s);
-   break;
-   default:
+   else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
cmp = 0;
else
cmp = 1;
-   break;
}
+
return (s->reverse) ? -cmp : cmp;
 }
 
@@ -1446,6 +1448,9 @@ int parse_opt_ref_sorting(const struct option *opt, const 
char *arg, int unset)
s->reverse = 1;
arg++;
}
+   if (skip_prefix(arg, "version:", &arg) ||
+   skip_prefix(arg, "v:", &arg))
+   s->version = 1;
len = strlen(arg);
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
diff --git a/ref-filter.h b/ref-filter.h
index 16cffab..c41432f 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -41,7 +41,8 @@ struct atom_value {
 struct ref_sorting {
struct ref_sorting *next;
int atom; /* index into used_atom array (internal) */
-   unsigned reverse : 1;
+   unsigned reverse : 1,
+   version : 1;
 };
 
 struct ref_array_item {
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 87225dd..7332bea 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -161,4 +161,40 @@ test_expect_success 'non atom alignment' '
test_cmp expect actual
 '
 
+test_expect_success 'setup for version sort' '
+   test_commit foo1.3 &&
+   test_commit foo1.6 &&
+   test_commit foo1.10
+'
+
+test_expect_success 'version sort' '
+   git for-each-ref --sort=version:refname --format="%(refname:short)" 
refs/tags/ | grep "foo" >actual &&
+   cat >expect <<-\EOF &&
+   foo1.3
+   foo1.6
+   foo1.10
+   EOF
+   test_cmp expect actual
+'
+
+test_expect_success 'version sort (shortened)' '
+   git for-each-ref --sort=v:refname --format="%(refname:short)" 
refs/tags/ | grep "foo" >actual &&
+   cat >expect <<-\EOF &&
+   foo1.3
+   foo1.6
+   foo1.10
+   EOF
+   test_cmp expect actual
+'
+
+test_expect_success 'reverse version sort' '
+   git for-each-ref --sort=-version:refname --format="%(refname:short)" 
refs/tags/ | grep "foo" >actual &&
+   cat >expect <<-\EOF &&
+   foo1.10
+   foo1.6
+   foo1.3
+   EOF
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 02/11] ref-filter: introduce ref_formatting_state

2015-08-03 Thread Karthik Nayak
Introduce a ref_formatting_state which will eventually hold the values
of modifier atoms. Implement this within ref-filter.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 49 +
 ref-filter.h |  4 
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index febdc45..c4c7064 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1190,9 +1190,10 @@ void ref_array_sort(struct ref_sorting *sorting, struct 
ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
compare_refs);
 }
 
-static void print_value(struct atom_value *v, int quote_style, struct strbuf 
*output)
+static void print_value(struct atom_value *v, struct ref_formatting_state 
*state,
+   struct strbuf *output)
 {
-   switch (quote_style) {
+   switch (state->quote_style) {
case QUOTE_NONE:
strbuf_addstr(output, v->s);
break;
@@ -1249,24 +1250,47 @@ static void emit(const char *cp, const char *ep, struct 
strbuf *output)
}
 }
 
+static void process_formatting_state(struct atom_value *atomv, struct 
ref_formatting_state *state)
+{
+   /* Based on the atomv values, the formatting state is set */
+}
+
+static void apply_formatting_state(struct ref_formatting_state *state, struct 
strbuf *value,
+  struct strbuf *format)
+{
+   /* More formatting options to be evetually added */
+   strbuf_addbuf(format, value);
+   strbuf_release(value);
+}
+
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
 {
const char *cp, *sp, *ep;
-   struct strbuf output = STRBUF_INIT;
+   struct strbuf value = STRBUF_INIT;
+   struct strbuf final_buf = STRBUF_INIT;
+   struct ref_formatting_state state;
int i;
 
+   memset(&state, 0, sizeof(state));
+   state.quote_style = quote_style;
+
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
-   struct atom_value *atomv;
+   struct atom_value *atomv = NULL;
 
ep = strchr(sp, ')');
-   if (cp < sp)
-   emit(cp, sp, &output);
+   if (cp < sp) {
+   emit(cp, sp, &value);
+   apply_formatting_state(&state, &value, &final_buf);
+   }
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
&atomv);
-   print_value(atomv, quote_style, &output);
+   process_formatting_state(atomv, &state);
+   print_value(atomv, &state, &value);
+   apply_formatting_state(&state, &value, &final_buf);
}
if (*cp) {
sp = cp + strlen(cp);
-   emit(cp, sp, &output);
+   emit(cp, sp, &value);
+   apply_formatting_state(&state, &value, &final_buf);
}
if (need_color_reset_at_eol) {
struct atom_value resetv;
@@ -1275,12 +1299,13 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
if (color_parse("reset", color) < 0)
die("BUG: couldn't parse 'reset' as a color");
resetv.s = color;
-   print_value(&resetv, quote_style, &output);
+   print_value(&resetv, &state, &value);
+   apply_formatting_state(&state, &value, &final_buf);
}
-   for (i = 0; i < output.len; i++)
-   printf("%c", output.buf[i]);
+   for (i = 0; i < final_buf.len; i++)
+   printf("%c", final_buf.buf[i]);
putchar('\n');
-   strbuf_release(&output);
+   strbuf_release(&final_buf);
 }
 
 /*  If no sorting option is given, use refname to sort as default */
diff --git a/ref-filter.h b/ref-filter.h
index 6bf27d8..b64677f 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -16,6 +16,10 @@
 #define FILTER_REFS_INCLUDE_BROKEN 0x1
 #define FILTER_REFS_ALL 0x2
 
+struct ref_formatting_state {
+   int quote_style;
+};
+
 struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 01/11] ref-filter: print output to strbuf for formatting

2015-08-03 Thread Karthik Nayak
Introduce a strbuf `output` which will act as a substitute rather than
printing directly to stdout. This will be used for formatting
eventually.
---
 ref-filter.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 7561727..febdc45 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1190,30 +1190,25 @@ void ref_array_sort(struct ref_sorting *sorting, struct 
ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
compare_refs);
 }
 
-static void print_value(struct atom_value *v, int quote_style)
+static void print_value(struct atom_value *v, int quote_style, struct strbuf 
*output)
 {
-   struct strbuf sb = STRBUF_INIT;
switch (quote_style) {
case QUOTE_NONE:
-   fputs(v->s, stdout);
+   strbuf_addstr(output, v->s);
break;
case QUOTE_SHELL:
-   sq_quote_buf(&sb, v->s);
+   sq_quote_buf(output, v->s);
break;
case QUOTE_PERL:
-   perl_quote_buf(&sb, v->s);
+   perl_quote_buf(output, v->s);
break;
case QUOTE_PYTHON:
-   python_quote_buf(&sb, v->s);
+   python_quote_buf(output, v->s);
break;
case QUOTE_TCL:
-   tcl_quote_buf(&sb, v->s);
+   tcl_quote_buf(output, v->s);
break;
}
-   if (quote_style != QUOTE_NONE) {
-   fputs(sb.buf, stdout);
-   strbuf_release(&sb);
-   }
 }
 
 static int hex1(char ch)
@@ -1234,7 +1229,7 @@ static int hex2(const char *cp)
return -1;
 }
 
-static void emit(const char *cp, const char *ep)
+static void emit(const char *cp, const char *ep, struct strbuf *output)
 {
while (*cp && (!ep || cp < ep)) {
if (*cp == '%') {
@@ -1243,13 +1238,13 @@ static void emit(const char *cp, const char *ep)
else {
int ch = hex2(cp + 1);
if (0 <= ch) {
-   putchar(ch);
+   strbuf_addch(output, ch);
cp += 3;
continue;
}
}
}
-   putchar(*cp);
+   strbuf_addch(output, *cp);
cp++;
}
 }
@@ -1257,19 +1252,21 @@ static void emit(const char *cp, const char *ep)
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
 {
const char *cp, *sp, *ep;
+   struct strbuf output = STRBUF_INIT;
+   int i;
 
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
struct atom_value *atomv;
 
ep = strchr(sp, ')');
if (cp < sp)
-   emit(cp, sp);
+   emit(cp, sp, &output);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
&atomv);
-   print_value(atomv, quote_style);
+   print_value(atomv, quote_style, &output);
}
if (*cp) {
sp = cp + strlen(cp);
-   emit(cp, sp);
+   emit(cp, sp, &output);
}
if (need_color_reset_at_eol) {
struct atom_value resetv;
@@ -1278,9 +1275,12 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
if (color_parse("reset", color) < 0)
die("BUG: couldn't parse 'reset' as a color");
resetv.s = color;
-   print_value(&resetv, quote_style);
+   print_value(&resetv, quote_style, &output);
}
+   for (i = 0; i < output.len; i++)
+   printf("%c", output.buf[i]);
putchar('\n');
+   strbuf_release(&output);
 }
 
 /*  If no sorting option is given, use refname to sort as default */
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 04/11] ref-filter: add option to filter only tags

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Add a functions called 'for_each_tag_ref_fullpath()' to refs.{c,h}
which iterates through each tag ref without trimming the path.

Add an option in 'filter_refs()' to use 'for_each_tag_ref_fullpath()'
and filter refs. This type checking is done by adding a
'FILTER_REFS_TAGS' in 'ref-filter.h'

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 2 ++
 ref-filter.h | 1 +
 refs.c   | 5 +
 refs.h   | 1 +
 4 files changed, 9 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 46d8834..01c9097 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1155,6 +1155,8 @@ int filter_refs(struct ref_array *array, struct 
ref_filter *filter, unsigned int
ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
else if (type & FILTER_REFS_ALL)
ret = for_each_ref(ref_filter_handler, &ref_cbdata);
+   else if (type & FILTER_REFS_TAGS)
+   ret = for_each_tag_ref_fullpath(ref_filter_handler, 
&ref_cbdata);
else if (type)
die("filter_refs: invalid type");
 
diff --git a/ref-filter.h b/ref-filter.h
index 01f8cb3..8d4e348 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -15,6 +15,7 @@
 
 #define FILTER_REFS_INCLUDE_BROKEN 0x1
 #define FILTER_REFS_ALL 0x2
+#define FILTER_REFS_TAGS 0x4
 
 #define ALIGN_LEFT 0x01
 #define ALIGN_RIGHT 0x02
diff --git a/refs.c b/refs.c
index 0b96ece..23ce483 100644
--- a/refs.c
+++ b/refs.c
@@ -2108,6 +2108,11 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)
return for_each_ref_in("refs/tags/", fn, cb_data);
 }
 
+int for_each_tag_ref_fullpath(each_ref_fn fn, void *cb_data)
+{
+   return do_for_each_ref(&ref_cache, "refs/tags/", fn, 0, 0, cb_data);
+}
+
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
diff --git a/refs.h b/refs.h
index e4e46c3..9eee2de 100644
--- a/refs.h
+++ b/refs.h
@@ -174,6 +174,7 @@ extern int head_ref(each_ref_fn fn, void *cb_data);
 extern int for_each_ref(each_ref_fn fn, void *cb_data);
 extern int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
 extern int for_each_tag_ref(each_ref_fn fn, void *cb_data);
+extern int for_each_tag_ref_fullpath(each_ref_fn fn, void *cb_data);
 extern int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 extern int for_each_remote_ref(each_ref_fn fn, void *cb_data);
 extern int for_each_replace_ref(each_ref_fn fn, void *cb_data);
-- 
2.4.6

--
To unsubscribe from this list: send the line "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 v8 10/11] tag.c: implement '--format' option

2015-08-03 Thread Karthik Nayak
From: Karthik Nayak 

Implement the '--format' option provided by 'ref-filter'.
This lets the user list tags as per desired format similar
to the implementation in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-tag.txt | 15 ++-
 builtin/tag.c | 11 +++
 t/t7004-tag.sh| 16 
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 3ac4a96..75703c5 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,8 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [--create-reflog] [--sort=] 
[...]
+   [--column[=] | --no-column] [--create-reflog] [--sort=]
+   [--format=] [...]
 'git tag' -v ...
 
 DESCRIPTION
@@ -158,6 +159,18 @@ This option is only applicable when listing tags without 
annotation lines.
The object that the new tag will refer to, usually a commit.
Defaults to HEAD.
 
+::
+   A string that interpolates `%(fieldname)` from the object
+   pointed at by a ref being shown.  If `fieldname` is prefixed
+   with an asterisk (`*`) and the ref points at a tag object, the
+   value for the field in the object tag refers is used.  When
+   unspecified, defaults to `%(refname:short)`.  It also
+   interpolates `%%` to `%`, and `%xx` where `xx` are hex digits
+   interpolates to character with hex code `xx`; for example
+   `%00` interpolates to `\0` (NUL), `%09` to `\t` (TAB) and
+   `%0a` to `\n` (LF).  The fields are same as those in `git
+   for-each-ref`.
+
 
 CONFIGURATION
 -
diff --git a/builtin/tag.c b/builtin/tag.c
index 829af6f..13c9579 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,10 +30,9 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
const char *format)
 {
struct ref_array array;
-   char *format;
int i;
 
memset(&array, 0, sizeof(array));
@@ -43,7 +42,7 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting)
 
if (filter->lines)
format = "%(align:left,16)%(refname:short)";
-   else
+   else if (!format)
format = "%(refname:short)";
 
verify_ref_format(format);
@@ -327,6 +326,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+   const char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -359,6 +359,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at, 
N_("object"),
N_("print only tags of the object"), 0, 
parse_opt_object_name
},
+   OPT_STRING(  0 , "format", &format, N_("format"), N_("format to 
use for the output")),
OPT_END()
};
 
@@ -398,8 +399,10 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
copts.padding = 2;
run_column_filter(colopts, &copts);
}
+   if (format && (filter.lines != -1))
+   die(_("--format and -n are incompatible"));
filter.name_patterns = argv;
-   ret = list_tags(&filter, sorting);
+   ret = list_tags(&filter, sorting, format);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1f066aa..1809011 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1519,4 +1519,20 @@ EOF"
test_cmp expect actual
 '
 
+test_expect_success '--format cannot be used with -n' '
+   test_must_fail git tag -l -n4 --format="%(refname)"
+'
+
+test_expect_success '--format should list tags as per format given' '
+   cat >expect <<-\EOF &&
+   refname : refs/tags/foo1.10
+   refname : refs/tags/foo1.3
+   refname : refs/tags/foo1.6
+   refname : refs/tags/foo1.6-rc1
+   refname : refs/tags/foo1.6-rc2
+   EOF
+   git tag -l --format="refname : %(refname)" "foo*" >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://

[PATCH v8 03/11] ref-filter: implement an `align` atom

2015-08-03 Thread Karthik Nayak
Implement an `align` atom which will act as a modifier atom and align
succeeding atoms to the right, left or middle. It is followed by
`:,`, where the `` is either left, right or
middle and `` is the total length of the padding to be
performed. If the atom length is more than the padding length then no
padding is performed. e.g. to pad a succeeding atom to the middle with
a total padding size of 40 we can do a --format="%(align:middle,40).."

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  7 
 ref-filter.c   | 69 +---
 ref-filter.h   | 12 ++
 t/t6302-for-each-ref-filter.sh | 80 ++
 4 files changed, 162 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index e49d578..bba6d83 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,13 @@ color::
Change output color.  Followed by `:`, where names
are described in `color.branch.*`.
 
+align::
+   Align succeeding atoms to the right, left or middle. Followed
+   by `:,`, where the `` is either
+   left, right or middle and `` is the total
+   length of the padding to be performed. If the atom length is
+   more than the padding length then no padding is performed.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index c4c7064..46d8834 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
 #include "quote.h"
 #include "ref-filter.h"
 #include "revision.h"
+#include "utf8.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -53,6 +54,7 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color" },
+   { "align" },
 };
 
 /*
@@ -620,7 +622,7 @@ static void populate_value(struct ref_array_item *ref)
const char *name = used_atom[i];
struct atom_value *v = &ref->value[i];
int deref = 0;
-   const char *refname;
+   const char *refname = NULL;
const char *formatp;
struct branch *branch = NULL;
 
@@ -687,6 +689,24 @@ static void populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+   } else if (starts_with(name, "align:")) {
+   const char *valp = NULL;
+   struct align *align = xmalloc(sizeof(struct align));
+
+   skip_prefix(name, "align:", &valp);
+
+   if (skip_prefix(valp, "left,", &valp))
+   align->align_type = ALIGN_LEFT;
+   else if (skip_prefix(valp, "right,", &valp))
+   align->align_type = ALIGN_RIGHT;
+   else if (skip_prefix(valp, "middle,", &valp))
+   align->align_type = ALIGN_MIDDLE;
+   else
+   die(_("align: improper format"));
+   if (strtoul_ui(valp, 10, &align->align_value))
+   die(_("align: positive value expected"));
+   v->align = align;
+   continue;
} else
continue;
 
@@ -1252,15 +1272,48 @@ static void emit(const char *cp, const char *ep, struct 
strbuf *output)
 
 static void process_formatting_state(struct atom_value *atomv, struct 
ref_formatting_state *state)
 {
-   /* Based on the atomv values, the formatting state is set */
+   if (atomv->align) {
+   state->align = atomv->align;
+   atomv->align = NULL;
+   }
 }
 
 static void apply_formatting_state(struct ref_formatting_state *state, struct 
strbuf *value,
   struct strbuf *format)
 {
-   /* More formatting options to be evetually added */
+   if (state->align) {
+   int len = 0, buf_len = value->len;
+   struct align *align = state->align;
+
+   if (!value->buf)
+   return;
+   if (!is_utf8(value->buf)) {
+   len = value->len - utf8_strwidth(value->buf);
+   buf_len -= len;
+   }
+
+   if (align->align_value < buf_len) {
+   state->align = NULL;
+   strbuf_addbuf(format, value);
+   strbuf_release(value);
+   return;
+   }
+
+   if (align->align_type == ALIGN_LEFT)
+ 

[PATCH v8 0/11] Port tag.c over to use ref-filter APIs

2015-08-03 Thread Karthik Nayak
This is part of my GSoC project to unify git tag -l, git branch -l,
git for-each-ref.  This patch series is continued from: Git (next)
https://github.com/git/git/commit/bf5418f49ff0cebc6e5ce04ad1417e1a47c81b61

This series consists of porting tag.c over to using the ref-filter APIs

Version 7 can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/274990

Changes:
* Make padright a general align atom.
* Make print_value() and emit() output to a strbuf rather than stdout directly.

Interdiff:

diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index bcf319a..e89b9b0 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,11 +127,12 @@ color::
 Change output color.  Followed by `:`, where names
 are described in `color.branch.*`.

-padright::
-Pad succeeding atom or string to the right. Followed by
-`:`, where `value` states the total length of atom or
-string including the padding. If the `value` is lesser than
-the atom or string length, then no padding is performed.
+align::
+Align succeeding atoms to the right, left or middle. Followed
+by `:,`, where the `` is either
+left, right or middle and `` is the total
+length of the padding to be performed. If the atom length is
+more than the padding length then no padding is performed.

 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/builtin/tag.c b/builtin/tag.c
index fc01117..529b29f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -41,7 +41,7 @@ static int list_tags(struct ref_filter *filter,
struct ref_sorting *sorting, con
 filter->lines = 0;

 if (filter->lines)
-format = "%(padright:16)%(refname:short)";
+format = "%(align:left,16)%(refname:short)";
 else if (!format)
 format = "%(refname:short)";

diff --git a/ref-filter.c b/ref-filter.c
index 65d168e..afeab37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -56,7 +56,7 @@ static struct {
 { "flag" },
 { "HEAD" },
 { "color" },
-{ "padright" },
+{ "align" },
 };

 /*
@@ -624,7 +624,7 @@ static void populate_value(struct ref_array_item *ref)
 const char *name = used_atom[i];
 struct atom_value *v = &ref->value[i];
 int deref = 0;
-const char *refname;
+const char *refname = NULL;
 const char *formatp;
 struct branch *branch = NULL;

@@ -664,8 +664,6 @@ static void populate_value(struct ref_array_item *ref)
 if (color_parse(name + 6, color) < 0)
 die(_("unable to parse format"));
 v->s = xstrdup(color);
-v->color = 1;
-v->modifier_atom = 1;
 continue;
 } else if (!strcmp(name, "flag")) {
 char buf[256], *cp = buf;
@@ -693,17 +691,23 @@ static void populate_value(struct ref_array_item *ref)
 else
 v->s = " ";
 continue;
-} else if (starts_with(name, "padright:")) {
+} else if (starts_with(name, "align:")) {
 const char *valp = NULL;
+struct align *align = xmalloc(sizeof(struct align));

-skip_prefix(name, "padright:", &valp);
-if (!valp[0])
-die(_("no value given with 'padright:'"));
-if (strtoul_ui(valp, 10, (unsigned int *)&v->ul))
-die(_("positive integer expected after ':' in padright:%u\n"),
-(unsigned int)v->ul);
-v->modifier_atom = 1;
-v->pad_to_right = 1;
+skip_prefix(name, "align:", &valp);
+
+if (skip_prefix(valp, "left,", &valp))
+align->align_type = ALIGN_LEFT;
+else if (skip_prefix(valp, "right,", &valp))
+align->align_type = ALIGN_RIGHT;
+else if (skip_prefix(valp, "middle,", &valp))
+align->align_type = ALIGN_MIDDLE;
+else
+die(_("align: improper format"));
+if (strtoul_ui(valp, 10, &align->align_value))
+die(_("align: positive value expected"));
+v->align = align;
 continue;
 } else
 continue;
@@ -1243,55 +1247,26 @@ void ref_array_sort(struct ref_sorting
*sorting, struct ref_array *array)
 qsort(array->items, array->nr, sizeof(struct ref_array_item *),
compare_refs);
 }

-static void apply_formatting_state(struct ref_formatting_state *state,
-   const char *buf, struct strbuf *value)
-{
-if (state->color) {
-strbuf_addstr(value, state->color);
-state->color = NULL;
-}
-if (state->pad_to_right) {
-if (!is_utf8(buf))
-strbuf_addf(value, "%-*s", state->pad_to_right, buf);
-else {
-int utf8_compensation = strlen(buf) - utf8_strwidth(buf);
-strbuf_addf(value, "%-*s", state->pad_

Re: [PATCH v2 5/6] clone: fix hostname parsing when guessing dir

2015-08-03 Thread Jeff King
On Mon, Aug 03, 2015 at 10:34:14AM +0200, Patrick Steinhardt wrote:

> One more question for backwards compatibility remains then.
> Currently when we clone something like 'http://example.com:/'
> we'd create a git repository '' as we'd split on the first
> occurrence of ':'. Should we remain backwards compatible here, as
> well, or change the behavior to use 'example.com' as repository
> name?

I don't think naming the repo "" makes much sense; I'd consider it a
bug. The only sensible names are "example.com" or "example.com:"
(the latter is more specific if you are going to clone the root off of
several different ports, but that seems rather unlikely; the former is
probably what I'd expect).

-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/PATCH 2/2] Testing the new code

2015-08-03 Thread Stefan Beller
On Fri, Jul 31, 2015 at 6:02 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> -module_list()
>> +module_list_shell()
>>  {
>>   eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
>>   (
>> @@ -187,6 +187,29 @@ module_list()
>>   '
>>  }
>>
>> +module_list()
>> +{
>> + # call both the old and new code
>> + module_list_shell $@ >/u/git_submodule_module_list_shell 
>> 2>/u/git_submodule_module_list_shell2
>> + git submodule--helper --module_list $@ >/u/git_submodule_module_list 
>> 2>/u/git_submodule_module_list2
>
> You seem to be discarding the double-quote around $@ in both of
> these two places.  Intended?

No, not at all. This was a bit sloppy.
This patch was rather showing off how I intend to test the previous patch.
The idea came from a talk[1] at oscon which presented refactoring code with
this differential strategy (except that they used it on a larger code base and
collected the diffs between the new and old code for a while in production
instead of just relying on the test suite passing).

[1] http://www.oscon.com/open-source-2015/public/schedule/detail/41888
--
To unsubscribe from this list: send the line "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 pull --rebase" fails if pager.pull is true, after producing a colorized diff it cannot apply

2015-08-03 Thread Per Cederqvist
If you run:

git config pager.pull true

in the hope of getting the output of "git pull" via a pager, you are
in for a surpise the next time you run "git pull --rebase" and it has
to rebase your work.  It will fail with a nonsensical error message:

> Applying: First B commit
> fatal: unrecognized input
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 First B commit
> The copy of the patch that failed is found in:
>/home/cederp/badcolor/repo-b/.git/rebase-apply/patch
>
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".

Using "cat -vet" to look at the problematic patch, you can see that
there are embedded escape codes that tries to colorize the patch.

This bug is dependent on the TERM setting.  On my system (Ubuntu
14.04) it reproduces if TERM=vt220 or TERM=rxvt-unicode, but not if
TERM=dumb.  It might depend on the color.diff setting as well, but
it does reproduce with the default setting.

The following script reproduces the problem.  I've tried both git
2.4.3 and git 2.5.0.

- cut here -
#!/bin/sh
set -e -x

# All created files are created inside the "badcolor" directory.
mkdir badcolor
cd badcolor

# Create a bare repo.
mkdir upstream.git
(cd upstream.git && git init --bare)

# Make an initial commit.
git clone upstream.git repo-a
(cd repo-a && echo one > a && git add a && git commit -m"First A commit")
(cd repo-a && git push origin master)

# Make a second clone.
git clone upstream.git repo-b

# Make one more commit, that the second clone won't have for a while.
(cd repo-a && echo two > a && git add a && git commit -m"Second A commit")
(cd repo-a && git push origin master)

# Create a third commit; this make the history non-linear, but since
# the commit only touched a new file it should be trivial to linearize
# it.
(cd repo-b && echo one > b && git add b && git commit -m"First B commit")

# Set pager.pull true so that we trigger the bug.
(cd repo-b && git config pager.pull true)

# Attempt to make the history linear.  This command will fail if TERM
# specifies a color-capable terminal.

(cd repo-b && git pull --rebase)

exit 0
- cut here -

Thanks,

/ceder
-- 
Per Cederqvist 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread Duy Nguyen
On Mon, Aug 3, 2015 at 8:02 PM, Duy Nguyen  wrote:
> On Sat, Aug 1, 2015 at 10:59 AM, Michael Haggerty  
> wrote:
>> Either way, there's also the question of who should know how to find the
>> worktree-specific references--the program that wants to access them, or
>> should there be a secret invisible mapping that is done on lookup, and

After re-reading some code, I think this invisible mapping on lookup
is already done for our file-based ref backend. Take
resolve_ref_unsafe_1 for example, the call strbuf_git_path() may
return ".git/worktrees/foo/HEAD", not ".git/HEAD", given the input ref
"HEAD". So ref names are already separated from where they are stored.

>> that knows the full list of references that want to be worktree-specific
>> (for example, that in a linked worktree, "refs/bisect/*" should be
>> silently redirected to "refs/worktree//bisect/*")?

My decision to hard code these paths may turn out a bad thing (it's
common_list[] in path.c). Perhaps I should let the user extend them,
but we will not allow to override a small subset of paths because that
may break stuff horribly.

>> It's all a bit frightening, frankly.
>
> I would think all work to make pluggable backends happen should allow
> us to do this kind of transparent mapping. We can add a new file-based
> backend with just this redirection, can't we? I haven't read the
> updated refs.c though, need to do it now..
-- 
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 v5 2/5] refs: add ref_type function

2015-08-03 Thread Duy Nguyen
On Fri, Jul 31, 2015 at 1:06 PM, David Turner  wrote:
> Add a function ref_type, which categorizes refs as per-worktree,
> pseudoref, or normal ref.

For per-worktree refs, you probably should follow common_list[] in
path.c because that's how file-based ref namespace is splitted between
per-repo and per-worktree, even though just as simple as "everything
outside refs/ is per-worktree" (with an exception of NOTES_MERGE_REF,
which should be on the list as well). At least the two should be
aligned so that the default file-based backend works the same way as
new backends.

Going further, I think you need to pass the "worktree identifier" to
ref backend, at least in ref_transaction_begin_fn. Each backend is
free to store per-worktree refs however it wants. Of course if I ask
for refs/foo of worktree A, you should not return me refs/foo of
worktree B. ref_transaction_begin_fn can return a fault code if it
does not support multiple worktrees, which is fine.

> Later, we will use this in refs.c to treat pseudorefs specially.
> Alternate ref backends may use it to treat both pseudorefs and
> per-worktree refs differently.

I'm not so sure that this can't be hidden behind backends and they can
have total control on falling back to file-based, or store them in
some secondary storage. I haven't re-read your discussion with Junio
yet (only skimmed through long ago) so I may be missing some important
points.

>
> Signed-off-by: David Turner 
> ---
>  refs.c | 26 ++
>  refs.h |  8 
>  2 files changed, 34 insertions(+)
>
> diff --git a/refs.c b/refs.c
> index 0b96ece..0f87884 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2848,6 +2848,32 @@ static int delete_ref_loose(struct ref_lock *lock, int 
> flag, struct strbuf *err)
> return 0;
>  }
>
> +static int is_per_worktree_ref(const char *refname)
> +{
> +   return !strcmp(refname, "HEAD");
> +}
> +
> +static int is_pseudoref_syntax(const char *refname)
> +{
> +   const char *c;
> +
> +   for (c = refname; *c; c++) {
> +   if (!isupper(*c) && *c != '-' && *c != '_')
> +   return 0;
> +   }
> +
> +   return 1;
> +}
> +
> +enum ref_type ref_type(const char *refname)
> +{
> +   if (is_per_worktree_ref(refname))
> +   return REF_TYPE_PER_WORKTREE;
> +   if (is_pseudoref_syntax(refname))
> +   return REF_TYPE_PSEUDOREF;
> +   return REF_TYPE_NORMAL;
> +}
> +
>  int delete_ref(const char *refname, const unsigned char *old_sha1,
>unsigned int flags)
>  {
> diff --git a/refs.h b/refs.h
> index e4e46c3..dca4fb5 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -445,6 +445,14 @@ extern int parse_hide_refs_config(const char *var, const 
> char *value, const char
>
>  extern int ref_is_hidden(const char *);
>
> +enum ref_type {
> +   REF_TYPE_PER_WORKTREE,
> +   REF_TYPE_PSEUDOREF,
> +   REF_TYPE_NORMAL,
> +};
> +
> +enum ref_type ref_type(const char *refname);
> +
>  enum expire_reflog_flags {
> EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
> EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
> --
> 2.0.4.315.gad8727a-twtrsrc
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
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 1/1] po/README: split out the different contribution types

2015-08-03 Thread Philip Oakley

From: "Jiang Xin" 

2015-08-02 21:42 GMT+08:00 Philip Oakley :
Separate out the three different contribution styles for existing, 
new,
and wider mis-translation contributions, with suitable headings for 
easy

reference.

In particular highlight the appropriate action should a Git
mis-translation be from a different distribution. i.e. initially 
report

to that usptream, but also consider if it could be coordinated here.

While here, add surrounding headings, and reference the lanaguage 
codes.


Signed-off-by: Philip Oakley 
---
 po/README | 24 
 1 file changed, 24 insertions(+)

diff --git a/po/README b/po/README
index d8c9111..b54b3c6 100644
--- a/po/README
+++ b/po/README
@@ -10,19 +10,43 @@ coordinates our localization effort in the l10 
coordinator repository:


 https://github.com/git-l10n/git-po/

+The two character language translation codes are defined by 
ISO_639-1, as
+stated in the gettext(1) full manual, appendix A.1, Usual Language 
Codes.

+
+
+Contributing to an existing translation
+---
 As a contributor for a language XX, you should first check TEAMS 
file in
 this directory to see whether a dedicated repository for your 
language XX
 exists. Fork the dedicated repository and start to work if it 
exists.


+
+Creating a new language translation
+---
 If you are the first contributor for the language XX, please fork 
this
 repository, prepare and/or update the translated message file 
po/XX.po

 (described later), and ask the l10n coordinator to pull your work.

+
+Reporting mis-translations
+--
+First, confirm that your language translation is managed by this 
group.
+Initially any translation mistake should be reported to the 
appropriate
+upstream team. If not managed by this group, and the translation has 
a

+suitable licence, consider suggesting that the translation could be
+included here (see 'new language translation' above).
+
First, find the right place to report.  If there is a YourLanguage.po 
here,
and you can find the bug in it, then here is the right place for you 
to

report.  But if not, maybe your Git distribution (such as from Ubuntu)
may have its own l10n workflow, and not work well with our git
upstream. For this case, you should report bug to them...

(I'm not a native English speaker, so correct me)



Thanks. I'll work up your comments and try sending a formal V2 patch
to the list. I'll also include about other workflows for other 
distributions.


Would I be right that there were no issues with the splitting into the 
separate headings?



+
+Forming a Team
+--
 If there are multiple contributors for the same language, please 
first

 coordinate among yourselves and nominate the team leader for your
 language, so that the l10n coordinator only needs to interact with 
one

 person per language.

+
+Translation Process Flow
+
 The overall data-flow looks like this:

 +---++--+
--
2.3.1



--
Jiang Xin


--
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


Fetch and refs/remotes//HEAD

2015-08-03 Thread Dror Livne
I have noticed that when cloning a repository, I get a
refs/remotes/origin/HEAD symbolic ref.

On the other hand, when fetching a new remote, the remote HEAD is not set by
git-fetch (but can be added later by `git remote set-head ...')

I was under the impression that clone is equivalent to (at least) init+fetch.

Is there a rationale for that difference in behavior?

Dror Livne


--
To unsubscribe from this list: send the line "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] notes: handle multiple worktrees

2015-08-03 Thread Duy Nguyen
On Wed, Jul 29, 2015 at 5:50 AM, Johan Herland  wrote:
> On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano  wrote:
>> David Turner  writes:
>>> Prevent merges to the same notes branch from different worktrees.
>>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
>>> code we use to check that two HEADs in different worktrees don't point
>>> to the same branch.  Modify that code, die_if_checked_out, to take a
>>> "head" ref to examine; previously, it just looked at HEAD.
>>>
>>> Reported-by: Junio C Hamano 
>>> Signed-off-by: David Turner 
>>> ---
>>
>> Thanks for following through.  As I didn't report anything, I do not
>> deserve that label, but it's OK ;-)
>>
>> I know that it is a requirement to protect NOTES_MERGE_REF from
>> being used by multiple places for "notes merge" to play well with
>> the multi-worktree world order, but because I do not know if that is
>> sufficient, I'm asking a few people for further review.
>
> As just stated in a related thread, I don't think it makes sense to
> have NOTES_MERGE_REF per worktree, as the notes merge is always
> completely unrelated to the current worktree (or the current branch
> for that matter). AFAICS this patch is all about handling per-worktree
> NOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there
> should only be one NOTES_MERGE_REF per _repo_, and although we might
> want to expand to allow multiple concurrent notes merges in the
> future, that is still a per-repo, and not a per-worktree thing, hence
> completely unrelated to David's current effort.

I agree. Luckily sharing NOTES_MERGE_REF is as short as

diff --git a/path.c b/path.c
index 10f4cbf..52d8ee4 100644
--- a/path.c
+++ b/path.c
@@ -94,7 +94,7 @@ static void replace_dir(struct strbuf *buf, int len,
const char *newdir)
 static const char *common_list[] = {
  "/branches", "/hooks", "/info", "!/logs", "/lost-found",
  "/objects", "/refs", "/remotes", "/worktrees", "/rr-cache", "/svn",
- "config", "!gc.pid", "packed-refs", "shallow",
+ "config", "!gc.pid", "packed-refs", "shallow", "NOTES_MERGE_REF",
  NULL
 };
 --
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: [msysGit] Difficulties senting patches to Git list using msmpt (send-email)

2015-08-03 Thread Torsten Bögershausen
Seems like a greylist problem ?
(I take the freedom to forward your mail to upstream git,
in the hope that somebody has has an idea)

 
On 03.08.15 13:13, Philip Oakley wrote:
> I'm scratching my head as to why patches I try to send upstream 
> (git@vger.kernel.org) via my MSysGit install's send-email appear to be being 
> dropped at gver/gmane.
> 
> It's only the patches I originate via send-email, using the windows MSysGit 
> msmtp mailer that aren't getting through. Other emails I send in reply to 
> threads on gmane (via OE6) get through OK.
> 
> I have confirmed I'm subscribed (as per FAQs), and I've also tested the 
> auto-answer facility.
> 
> The one peculiarity of the auto-answers is that the OE6 email reply had a 
> delay:
> 
> X-Greylist: delayed 592 seconds by postgrey-1.27 at vger.kernel.org; Sat, 01 
> Aug 2015 16:11:58 EDT
> [...]
> X-Mailer: Microsoft Outlook Express 6.00.2900.5931
> 
> while msmpt did not:
> [ no X-Greylist: ]
> X-Mailer: git-send-email 2.3.1
> 
> The delay may be a red-herring as it occurred on the non-problematic route, 
> but may be an indication of some extra processing within gmane that affects 
> the patches.
> 
> I have had a reply to one of the patch emails's direct addressees 
> ($gmane/275141), and I'd also received a copy of the patch as I'd cc'd 
> myself, so at least the outbound routing is getting beyond my ISP.
> 
> Does anyone have suggestions as to how I determine where/why my patche emails 
> dropped into >dev/null ?
> 
> -- 
> 
> Philip
> 
> PS I'm using the old msysgit infrastructure as msmpt wasn't available in the 
> new SDK at this moment.
> 
> I don't believe it's anything to do with the recompile at v2.3.1 (rather than 
> the plain vanilla msysgit.1.9.5), but then again...

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/2] bisect per-worktree

2015-08-03 Thread Duy Nguyen
On Sat, Aug 1, 2015 at 10:59 AM, Michael Haggerty  wrote:
> Either way, there's also the question of who should know how to find the
> worktree-specific references--the program that wants to access them, or
> should there be a secret invisible mapping that is done on lookup, and
> that knows the full list of references that want to be worktree-specific
> (for example, that in a linked worktree, "refs/bisect/*" should be
> silently redirected to "refs/worktree//bisect/*")?
>
> It's all a bit frightening, frankly.

I would think all work to make pluggable backends happen should allow
us to do this kind of transparent mapping. We can add a new file-based
backend with just this redirection, can't we? I haven't read the
updated refs.c though, need to do it now..
-- 
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/RFC 0/2] bisect per-worktree

2015-08-03 Thread Duy Nguyen
On Sat, Aug 1, 2015 at 1:51 PM, Michael Haggerty  wrote:
> For each worktree, we could then create a different view of the
> references by splicing parts of the full reference namespace together.
> This could even be based on config settings so that we don't have to
> hardcode information like "refs/bisect/* is worktree-specific" deep in
> the references module. Suppose we could write
>
> [worktree.refs]
> map = refs/worktrees/*:
> map = refs/bisect/*:refs/worktrees/[worktree]/refs/bisect/*

Perhaps the destination should be worktrees/[worktree]/refs/bisect/*.
Does it have to start with "refs/"?

> which would mean (a) hide the references under refs/worktrees", and (b)
> make it look as if the references under
> refs/worktrees/[worktree]/refs/bisect actually appear under refs/bisect
> (where "[worktree]" is replaced with the current worktree's name). By
> making these settings configurable, we allow other projects to define
> their own worktree-specific reference namespaces too.
>
> The corresponding main repo might hide "refs/worktrees/*" but leave its
> refs/bisect namespace exposed in the usual place.
>
> "git prune" would see the whole namespace as it really is so that it can
> compute reachability correctly.

For multiple wortrees, first we basically remap paths in .git,
relocate some to worktrees/[worktree]/ (already done), then we're
going to remap config keys (submodule support, just proof of concept
so far), remapping ref paths sounds like the logical next step if some
under refs/ needs to be worktree-specific. I wonder if current ref
namespace code could be extended to do this?
-- 
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 v1] send-email: provide whitelist of SMTP AUTH mechanisms

2015-08-03 Thread Jan Viktorin
On Sun, 02 Aug 2015 11:28:49 -0700
Junio C Hamano  wrote:

> Jan Viktorin  writes:
> 
> > Authen::SASL gives:
> >
> > No SASL mechanism found
> >  at /usr/share/perl5/vendor_perl/Authen/SASL.pm line 77.
> >  at /usr/share/perl5/core_perl/Net/SMTP.pm line 207.
> >
> > The SASL library does not check validity of mechanisms'
> > names (or I did not find it). It just tries to load one
> > that matches both the ours and the server side ones.
> > ...
> > I would like to include the regex check based on RFC 4422
> > as I've already mentioned. at least, it filters out the
> > unwanted characters like '/', '.', etc.
> 
> Hmm, is there a way to ask Authen::SASL what SASL mechanism the
> installed system supports?  If so, the enhancement you are adding
> could be
> 
>   my @to_use;
>   if ($smtp_auth_whitelist is supplied) {
>   my @installed = Authen::SASL::list_mechanisms();
> for (@installed) {
>   if ($_ is whitelisted) {
>   push @to_use, $_;
>   }
>   }
>   }
> 
> and @to_use can later be supplied when we open the connection as the
> list of mechanisms we allow the library to pick.
> 
> Just my $.02

I didn't find a way how to determine what mechanisms are supported by SASL.
This is a way how it looks for a mechanism (I think) on new():

Authen/SASL/Perl.pm

 57   my @mpkg = sort {
 58 $b->_order <=> $a->_order
 59   } grep {
 60 my $have = $have{$_} ||= (eval "require $_;" and $_->can('_secflags')) 
? 1 : -1;
 61 $have > 0 and $_->_secflags(@sec) == @sec
 62   } map {
 63 (my $mpkg = __PACKAGE__ . "::$_") =~ s/-/_/g;
 64 $mpkg;
 65   } split /[^-\w]+/, $parent->mechanism
 66 or croak "No SASL mechanism found\n";

It just loads a package based on the names we provide. So it seems, the library
has no clue about the existing mechanisms. This would be possible by reading the
proper directory with packages which seems to be quite wierd anyway.

-- 
   Jan Viktorin  E-mail: vikto...@rehivetech.com
   System Architect  Web:www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic
--
To unsubscribe from this list: send the line "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] untracked-cache: support sparse checkout

2015-08-03 Thread Duy Nguyen
On Mon, Aug 3, 2015 at 5:18 PM, Duy Nguyen  wrote:
> On Sat, Aug 1, 2015 at 12:35 AM, David Turner  
> wrote:
>> Remove a check that would disable the untracked cache for sparse
>> checkouts.  Add tests that ensure that the untracked cache works with
>> sparse checkouts -- specifically considering the case that a file
>> foo/bar is checked out, but foo/.gitignore is not.
>
> I have looked some more at the code (sorry for being slow these days,
> $DAY_JOB can be exhausting). The reason 27b099a (untracked cache:
> don't open non-existent .gitignore - 2015-03-08) avoids skip-worktree
> is because when that patch is added, index changes do not affect
> untracked cache (yet). So when you delete the on-worktree .gitignore,
> untracked cache is invalidated and it falls back to the index version.
> exclude_sha1 would reflect the content in the index. If the in-index
> .gitignore is deleted, without feedback from the index, untracked
> cache remains unchanged (i.e. valid) and assumes that .gitignore is
> still there. Which is wrong.

Hmm.. I got it backward: it should be like this: delete .gitignore on
worktree and in index, exclude_sha1 is null. then add .gitignore in
index only. exclude_sha1 remains null because the cache is still valid
even though we should .gitignore from the index.

> That's fixed in e931371 (untracked cache:
> invalidate at index addition or removal - 2015-03-08).
>
> With that out of the way,
>
> Acked-by: Duy Nguyen 
>
>> Signed-off-by: David Turner 
>> ---
>>  dir.c |  17 +-
>>  t/t7063-status-untracked-cache.sh | 119 
>> ++
>>  2 files changed, 122 insertions(+), 14 deletions(-)
>>
>> diff --git a/dir.c b/dir.c
>> index 8209f8b..e7b89fe 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -1078,10 +1078,9 @@ static void prep_exclude(struct dir_struct *dir, 
>> const char *base, int baselen)
>> (!untracked || !untracked->valid ||
>>  /*
>>   * .. and .gitignore does not exist before
>> - * (i.e. null exclude_sha1 and skip_worktree is
>> - * not set). Then we can skip loading .gitignore,
>> - * which would result in ENOENT anyway.
>> - * skip_worktree is taken care in read_directory()
>> + * (i.e. null exclude_sha1). Then we can skip
>> + * loading .gitignore, which would result in
>> + * ENOENT anyway.
>>   */
>>  !is_null_sha1(untracked->exclude_sha1))) {
>> /*
>> @@ -1880,7 +1879,6 @@ static struct untracked_cache_dir 
>> *validate_untracked_cache(struct dir_struct *d
>>   const struct pathspec 
>> *pathspec)
>>  {
>> struct untracked_cache_dir *root;
>> -   int i;
>>
>> if (!dir->untracked || getenv("GIT_DISABLE_UNTRACKED_CACHE"))
>> return NULL;
>> @@ -1932,15 +1930,6 @@ static struct untracked_cache_dir 
>> *validate_untracked_cache(struct dir_struct *d
>> if (dir->exclude_list_group[EXC_CMDL].nr)
>> return NULL;
>>
>> -   /*
>> -* An optimization in prep_exclude() does not play well with
>> -* CE_SKIP_WORKTREE. It's a rare case anyway, if a single
>> -* entry has that bit set, disable the whole untracked cache.
>> -*/
>> -   for (i = 0; i < active_nr; i++)
>> -   if (ce_skip_worktree(active_cache[i]))
>> -   return NULL;
>> -
>> if (!ident_in_untracked(dir->untracked)) {
>> warning(_("Untracked cache is disabled on this system."));
>> return NULL;
>> diff --git a/t/t7063-status-untracked-cache.sh 
>> b/t/t7063-status-untracked-cache.sh
>> index bd4806c..ff23f4e 100755
>> --- a/t/t7063-status-untracked-cache.sh
>> +++ b/t/t7063-status-untracked-cache.sh
>> @@ -354,4 +354,123 @@ EOF
>> test_cmp ../expect ../actual
>>  '
>>
>> +test_expect_success 'set up for sparse checkout testing' '
>> +   echo two >done/.gitignore &&
>> +   echo three >>done/.gitignore &&
>> +   echo two >done/two &&
>> +   git add -f done/two done/.gitignore &&
>> +   git commit -m "first commit"
>> +'
>> +
>> +test_expect_success 'status after commit' '
>> +   : >../trace &&
>> +   GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
>> +   git status --porcelain >../actual &&
>> +   cat >../status.expect <> +?? .gitignore
>> +?? dtwo/
>> +EOF
>> +   test_cmp ../status.expect ../actual &&
>> +   cat >../trace.expect <> +node creation: 0
>> +gitignore invalidation: 0
>> +directory invalidation: 0
>> +opendir: 1
>> +EOF
>> +   test_cmp ../trace.expect ../trace
>> +'
>> +
>> +test_expect_success 'untracked cache correct after commit' '
>> +   test-dump-untracked-cache >../actual &&
>> +   cat >../expect <> +info/exclud

Re: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs

2015-08-03 Thread Karthik Nayak
On Thu, Jul 30, 2015 at 12:59 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
> Solving this doesn't seem much harder than
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 6c0189f..a4df287 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1117,7 +1117,7 @@ static int ref_filter_handler(const char *refname, 
> const struct object_id *oid,
> struct commit *commit = NULL;
> unsigned int kind;
>
> -   if (flag & REF_BAD_NAME) {
> +   if (!filter->show_bad_name_refs && (flag & REF_BAD_NAME)) {
>   warning("ignoring ref with broken name %s", refname);
>   return 0;
> }
> diff --git a/ref-filter.h b/ref-filter.h
> index 98ebd3b..b9d2bbc 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -79,7 +79,7 @@ struct ref_filter {
> match_as_path : 1;
> unsigned int lines, branch_kind;
> int abbrev, verbose;
> -   int detached : 1;
> +   int detached : 1, show_bad_name_refs : 1;
>  };
>
>  struct ref_filter_cbdata {
>
> and setting filter->show_bad_name_refs when needed (untested). Did I
> miss something?
>

This allows it to be added to ref_array. But it would still fail while
trying to obtain
object details prior to printing.

> IIRC, historicaly Git allowed some weirdly named refs which made some
> commands ambiguous (e.g. a branch named after an option like '-d').
> We're forbidding their creation so people shouldn't have any, but we
> it's important to continue showing them in case some people have old
> bad-named branches lying around.
>
> I'd rather have the code strictly better after your contribution than
> before.
>

Agreed. But then again the warning tells about the broken ref, as in it's name
So I think It's ok?

for e.g. t1430 :
[trash directory.t1430-bad-ref-name] ../../git branch
warning: ignoring ref with broken name refs/heads/broken...ref
* master


-- 
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


Re: [PATCH] untracked-cache: support sparse checkout

2015-08-03 Thread Duy Nguyen
On Sat, Aug 1, 2015 at 12:35 AM, David Turner  wrote:
> Remove a check that would disable the untracked cache for sparse
> checkouts.  Add tests that ensure that the untracked cache works with
> sparse checkouts -- specifically considering the case that a file
> foo/bar is checked out, but foo/.gitignore is not.

I have looked some more at the code (sorry for being slow these days,
$DAY_JOB can be exhausting). The reason 27b099a (untracked cache:
don't open non-existent .gitignore - 2015-03-08) avoids skip-worktree
is because when that patch is added, index changes do not affect
untracked cache (yet). So when you delete the on-worktree .gitignore,
untracked cache is invalidated and it falls back to the index version.
exclude_sha1 would reflect the content in the index. If the in-index
.gitignore is deleted, without feedback from the index, untracked
cache remains unchanged (i.e. valid) and assumes that .gitignore is
still there. Which is wrong. That's fixed in e931371 (untracked cache:
invalidate at index addition or removal - 2015-03-08).

With that out of the way,

Acked-by: Duy Nguyen 

> Signed-off-by: David Turner 
> ---
>  dir.c |  17 +-
>  t/t7063-status-untracked-cache.sh | 119 
> ++
>  2 files changed, 122 insertions(+), 14 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 8209f8b..e7b89fe 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1078,10 +1078,9 @@ static void prep_exclude(struct dir_struct *dir, const 
> char *base, int baselen)
> (!untracked || !untracked->valid ||
>  /*
>   * .. and .gitignore does not exist before
> - * (i.e. null exclude_sha1 and skip_worktree is
> - * not set). Then we can skip loading .gitignore,
> - * which would result in ENOENT anyway.
> - * skip_worktree is taken care in read_directory()
> + * (i.e. null exclude_sha1). Then we can skip
> + * loading .gitignore, which would result in
> + * ENOENT anyway.
>   */
>  !is_null_sha1(untracked->exclude_sha1))) {
> /*
> @@ -1880,7 +1879,6 @@ static struct untracked_cache_dir 
> *validate_untracked_cache(struct dir_struct *d
>   const struct pathspec 
> *pathspec)
>  {
> struct untracked_cache_dir *root;
> -   int i;
>
> if (!dir->untracked || getenv("GIT_DISABLE_UNTRACKED_CACHE"))
> return NULL;
> @@ -1932,15 +1930,6 @@ static struct untracked_cache_dir 
> *validate_untracked_cache(struct dir_struct *d
> if (dir->exclude_list_group[EXC_CMDL].nr)
> return NULL;
>
> -   /*
> -* An optimization in prep_exclude() does not play well with
> -* CE_SKIP_WORKTREE. It's a rare case anyway, if a single
> -* entry has that bit set, disable the whole untracked cache.
> -*/
> -   for (i = 0; i < active_nr; i++)
> -   if (ce_skip_worktree(active_cache[i]))
> -   return NULL;
> -
> if (!ident_in_untracked(dir->untracked)) {
> warning(_("Untracked cache is disabled on this system."));
> return NULL;
> diff --git a/t/t7063-status-untracked-cache.sh 
> b/t/t7063-status-untracked-cache.sh
> index bd4806c..ff23f4e 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -354,4 +354,123 @@ EOF
> test_cmp ../expect ../actual
>  '
>
> +test_expect_success 'set up for sparse checkout testing' '
> +   echo two >done/.gitignore &&
> +   echo three >>done/.gitignore &&
> +   echo two >done/two &&
> +   git add -f done/two done/.gitignore &&
> +   git commit -m "first commit"
> +'
> +
> +test_expect_success 'status after commit' '
> +   : >../trace &&
> +   GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
> +   git status --porcelain >../actual &&
> +   cat >../status.expect < +?? .gitignore
> +?? dtwo/
> +EOF
> +   test_cmp ../status.expect ../actual &&
> +   cat >../trace.expect < +node creation: 0
> +gitignore invalidation: 0
> +directory invalidation: 0
> +opendir: 1
> +EOF
> +   test_cmp ../trace.expect ../trace
> +'
> +
> +test_expect_success 'untracked cache correct after commit' '
> +   test-dump-untracked-cache >../actual &&
> +   cat >../expect < +info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
> +core.excludesfile 
> +exclude_per_dir .gitignore
> +flags 0006
> +/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid
> +.gitignore
> +dtwo/
> +/done/  recurse valid
> +/dthree/  recurse check_only valid
> +/dtwo/  recurse check_

Re: [PATCH v2 5/6] clone: fix hostname parsing when guessing dir

2015-08-03 Thread Patrick Steinhardt
On Thu, Jul 30, 2015 at 09:53:07AM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Well, but there is the above "or we may not" ;-)
> >
> >> But actually you are right, currently I still have the old logic
> >> in place that splits on colons in the path component. 
> >
> > Yes.  The reason why I suggested the simple route was exactly
> > because I noticed that you didn't seem to care about the above
> > "$site/foo:bar.git/" => "$site/foo:bar" => "bar" transform.
> >
> > And I think people might depend on that behaviour.  "Fixing" that
> > may even be seen as a regression.
> >
> > When was the last time you created a f...@bar.git repository?
> 
> Actually, this was an unrelated question and a wrong one to ask at
> that.
> 
> Even though I personally haven't created foo:bar.git repository,
> because it is no longer 2005, it is highly likely that somewhere
> there is such a person who depends on the current behaviour of
> turning that to "bar" on the cloned side.  Similarly, even if we the
> people who read the Git mailing list collectively do not know
> anybody who has f...@bar.git repository, it is highly likely that
> somewhere there is such a person who depends on the current
> behaviour of turning that to "foo@bar" on the cloned side.
> 
> So the ideal would be to keep turning $site/foo:bar.git to bar,
> $site/f...@bar.git to foo@bar, and $scheme//u@p:$host/ to $host.
> 
> And it would be ideal if we do so without much code churn.  "The
> whole site is dedicated to host a single repository at the root" is
> a highly unlikely set-up and it feels wasteful to spend too much
> effort on.

One more question for backwards compatibility remains then.
Currently when we clone something like 'http://example.com:/'
we'd create a git repository '' as we'd split on the first
occurrence of ':'. Should we remain backwards compatible here, as
well, or change the behavior to use 'example.com' as repository
name?


signature.asc
Description: Digital signature


Re: [PATCH v7 0/11] port tag.c to use ref-filter APIs

2015-08-03 Thread Karthik Nayak
On Sun, Aug 2, 2015 at 11:02 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> +test_expect_success 'no padding when `padright` length is smaller than atom 
>> length' '
>> +cat >expect <<-\EOF &&
>> +refs/heads/master|
>> +refs/heads/side|
>> +refs/odd/spot|
>> +refs/tags/double-tag|
>> +refs/tags/four|
>> +refs/tags/one|
>> +refs/tags/signed-tag|
>> +refs/tags/three|
>> +refs/tags/two|
>> +EOF
>> +git for-each-ref --format="%(padright:5)%(refname)|" >actual &&
>>  test_cmp expect actual
>>  '
>
> Nit: I think the test would be better with padright:15 for example, to
> show that some lines are aligned and some go beyond the 15 columns.
>

I've added something like this :)


-- 
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