Re: [PATCH 7/8] config: add core.untrackedCache

2015-12-27 Thread Junio C Hamano
Duy Nguyen  writes:

> Sounds good, except..
>
>> When write_index() writes out an index that wasn't initialized from
>> the filesystem, a new UNTR gets added only when the configuration is
>> set to 'true' and there is no in-core UNTR already.
>
> Do we really need this?

We don't; you're right.

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


[PATCH/RFC 2/2] describe: add --pcre-match option

2015-12-27 Thread Mostyn Bramley-Moore
Add a perl-compatible regular expression version of the --match option,
which allows more flexible pattern matching.

Signed-off-by: Mostyn Bramley-Moore 
---
 Documentation/git-describe.txt |  5 +++
 builtin/describe.c | 69 ++
 t/README   |  3 +-
 t/t6120-describe.sh| 29 +++---
 4 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index c8f28c8..f646560 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -85,6 +85,11 @@ OPTIONS
excluding the "refs/tags/" prefix.  This can be used to avoid
leaking private tags from the repository.
 
+--pcre-match ::
+   Only consider tags matching the given `PCRE` pattern,
+   excluding the "refs/tags/" prefix.  This can be used to avoid
+   leaking private tags from the repository.
+
 --always::
Show uniquely abbreviated commit object as fallback.
 
diff --git a/builtin/describe.c b/builtin/describe.c
index 2386c64..7ceecd7 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -10,6 +10,10 @@
 #include "hashmap.h"
 #include "argv-array.h"
 
+#ifdef USE_LIBPCRE
+#include 
+#endif
+
 #define SEEN   (1u << 0)
 #define MAX_TAGS   (FLAG_BITS - 1)
 
@@ -32,6 +36,12 @@ static const char *pattern;
 static int always;
 static const char *dirty;
 
+#ifdef USE_LIBPCRE
+static pcre *regex = NULL;
+static pcre_extra *extra = NULL;
+static const char *pcre_pattern = NULL;
+#endif
+
 /* diff-index command arguments to check if working tree is dirty. */
 static const char *diff_index_args[] = {
"diff-index", "--quiet", "HEAD", "--", NULL
@@ -119,6 +129,47 @@ static void add_to_known_names(const char *path,
}
 }
 
+#ifdef USE_LIBPCRE
+static void pcre_init()
+{
+   const char *error = NULL;
+   int erroffset = -1;
+   int opts = PCRE_MULTILINE;
+
+   regex = pcre_compile(pcre_pattern, opts, &error, &erroffset, NULL);
+   if (!regex)
+   die("invalid PCRE at position %d of \'%s\': %s",
+   erroffset, pcre_pattern, error);
+
+   extra = pcre_study(regex, 0, &error);
+   if (!extra && error)
+   die("%s", error);
+}
+
+static void pcre_cleanup()
+{
+   pcre_free(regex);
+   regex = NULL;
+   pcre_free(extra);
+   extra = NULL;
+}
+
+/* return 1 on match, 0 on no match. */
+static int pcre_match(const char *pattern, const char *text)
+{
+   int ovector[30], flags = 0; // FIXME: ovector size ... ?
+   int ret = pcre_exec(regex, extra,
+   text, strlen(text), 0,
+   flags, ovector, ARRAY_SIZE(ovector));
+   if (ret < 0 && ret != PCRE_ERROR_NOMATCH)
+   die("pcre_exec failed with error code %d", ret);
+   if (ret > 0)
+   return 0; /* no match */
+
+   return 1; /* match */
+}
+#endif
+
 static int get_name(const char *path, const struct object_id *oid, int flag, 
void *cb_data)
 {
int is_tag = starts_with(path, "refs/tags/");
@@ -132,6 +183,10 @@ static int get_name(const char *path, const struct 
object_id *oid, int flag, voi
/* Accept only tags that match the pattern, if given */
if (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))
return 0;
+#ifdef USE_LIBPCRE
+   if (pcre_pattern && (!is_tag || pcre_match(pcre_pattern, path + 10)))
+   return 0;
+#endif
 
/* Is it annotated? */
if (!peel_ref(path, peeled.hash)) {
@@ -406,6 +461,10 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
N_("consider  most recent tags (default: 10)")),
OPT_STRING(0, "match",   &pattern, N_("glob"),
   N_("only consider tags matching ")),
+#ifdef USE_LIBPCRE
+   OPT_STRING(0, "pcre-match",  &pcre_pattern, N_("regex"),
+  N_("only consider tags matching PCRE ")),
+#endif
OPT_BOOL(0, "always",&always,
N_("show abbreviated commit object as fallback")),
{OPTION_STRING, 0, "dirty",  &dirty, N_("mark"),
@@ -429,6 +488,11 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
if (longformat && abbrev == 0)
die(_("--long is incompatible with --abbrev=0"));
 
+#ifdef USE_LIBPCRE
+   if (pcre_pattern)
+   pcre_init();
+#endif
+
if (contains) {
struct argv_array args;
 
@@ -455,6 +519,11 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
if (!names.size && !always)
die(_("No names found, cannot describe anything."));
 
+#ifdef USE_LIBPCRE
+   if (pcre_pattern)
+   pcre_cleanup();
+#endif
+
if (argc == 0) {
if (dirty) {
static

[PATCH/RFC 0/2] add a perl compatible regex match flag to git describe

2015-12-27 Thread Mostyn Bramley-Moore
git describe currently only supports glob matching with the --matches flag.
It would be useful to support regular expressions.

PCRE is already an optional dependency from git grep, I wonder if posix or
extended regexes would be preferable?

Some old discussion of this as a candidate feature is here, though nobody put
together a patch as far as I can see:
http://comments.gmane.org/gmane.comp.version-control.git/173873

Mostyn Bramley-Moore (2):
  describe: mention glob in the --matches help text
  describe: add --pcre-match option

 Documentation/git-describe.txt |  5 +++
 builtin/describe.c | 73 --
 t/README   |  3 +-
 t/t6120-describe.sh| 29 ++---
 4 files changed, 103 insertions(+), 7 deletions(-)

-- 
2.5.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/RFC 1/2] describe: mention glob in the --matches help text

2015-12-27 Thread Mostyn Bramley-Moore
This saves the user from needing to consult to manpage to learn the
format of the --matches argument.

Signed-off-by: Mostyn Bramley-Moore 
---
 builtin/describe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 8a25abe..2386c64 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -404,8 +404,8 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
N_("only output exact matches"), 0),
OPT_INTEGER(0, "candidates", &max_candidates,
N_("consider  most recent tags (default: 10)")),
-   OPT_STRING(0, "match",   &pattern, N_("pattern"),
-  N_("only consider tags matching ")),
+   OPT_STRING(0, "match",   &pattern, N_("glob"),
+  N_("only consider tags matching ")),
OPT_BOOL(0, "always",&always,
N_("show abbreviated commit object as fallback")),
{OPTION_STRING, 0, "dirty",  &dirty, N_("mark"),
-- 
2.5.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] add a perl compatible regex match flag to git describe

2015-12-27 Thread brian m. carlson
On Sun, Dec 27, 2015 at 11:59:50PM +0100, Mostyn Bramley-Moore wrote:
> git describe currently only supports glob matching with the --matches flag.
> It would be useful to support regular expressions.
> 
> PCRE is already an optional dependency from git grep, I wonder if posix or
> extended regexes would be preferable?
> 
> Some old discussion of this as a candidate feature is here, though nobody put
> together a patch as far as I can see:
> http://comments.gmane.org/gmane.comp.version-control.git/173873
> 
> Mostyn Bramley-Moore (2):
>   describe: mention glob in the --matches help text
>   describe: add --pcre-match option

If you're going to implement this, I'd recommend an option that adjusts
--matches to accept a PCRE regexp instead of adding a new option
accepting its own pattern.  I'd also recommend calling it --perl-regexp
for compatibility with git grep.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature


Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

2015-12-27 Thread Junio C Hamano
Johannes Sixt  writes:

> Am 22.12.2015 um 19:35 schrieb Johannes Sixt:
>> Am 22.12.2015 um 16:27 schrieb Elia Pinto:
>>> -for mail in `echo 00*`
>>> +for mail in $(echo 00*)
>>
>>> -for mail in `echo rfc2047/00*`
>>> +for mail in $(echo rfc2047/00*)
>>
>> True, these are equvalence transformations. But a better way to get rid
>> of the back-quotes is to write these lines as
>>
>> for mail in echo 00*
>> for mail in echo rfc2047/00*
>
> Ahem... both of these lines without the 'echo', of course!

Very true.  Let's leave that kind of things as separate clean-ups
after these patches settle, as mixing manual and mechanical changes
in a single patch makes it harder to review.

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 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

2015-12-27 Thread Junio C Hamano
Junio C Hamano  writes:

> Very true.  Let's leave that kind of things as separate clean-ups
> after these patches settle, as mixing manual and mechanical changes
> in a single patch makes it harder to review.
>
> Thanks.

So that I won't forget (I'll need to amend with your sign-off even
if this one is satisfactory to you).

-- >8 --
From: Johannes Sixt 
Date: Tue, 22 Dec 2015 19:35:16 +0100
Subject: [PATCH] t5100: no need to use 'echo' command substitutions for globbing

Instead of making the shell expand 00* and invoke 'echo' with it,
and then capturing its output as command substitution, just use
the result of expanding 00* directly.

Signed-off-by: Junio C Hamano 
---
 t/t5100-mailinfo.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 4e52b3b..85b3df5 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -23,7 +23,7 @@ check_mailinfo () {
 }
 
 
-for mail in $(echo 00*)
+for mail in 00*
 do
test_expect_success "mailinfo $mail" '
check_mailinfo $mail "" &&
@@ -51,7 +51,7 @@ test_expect_success 'split box with rfc2047 samples' \
echo total is $last &&
test $(cat rfc2047/last) = 11'
 
-for mail in $(echo rfc2047/00*)
+for mail in rfc2047/00*
 do
test_expect_success "mailinfo $mail" '
git mailinfo -u $mail-msg $mail-patch <$mail >$mail-info &&
-- 
2.7.0-rc2-145-g5695473
--
To unsubscribe from this list: send the line "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: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

2015-12-27 Thread Junio C Hamano
Eric Sunshine  writes:

> So, it might be easier to think of NO_OPENSSL as really meaning NO_SSL
> (that is, "disable all SSL-related functionality"). Since the only SSL
> implementation Git knows how to use is OpenSSL, perhaps one can
> consider the name NO_OPENSSL a historic anomaly.

That is a good explanation of what is observed.  I am not sure if it
is a good justification, though.  If you tell somebody who needs to
link an implementation of SHA-1 in that you (1) do not want to use
OpenSSL (or do not want to have SSL at all), and (2) do not mind
using Apple's CommonCrypto, and if you _know_ that CommonCrypto is
a possible source of the SHA-1 implementation, then I would think it
is reasonable to expect that CommonCrypto SHA-1 to be used.

Note. To further explain the situation, the only reason we
added CommonCrypto knob in the build system was to allow
people to use OpenSSL as the SSL implementation.  Those who
added the knob weren't making a conscious decision on which
SHA-1 implementation to use in that scenario---they may not
even have been aware of the fact that SHA-1 was offered by
CommonCrypto for that matter.

A few questions we should be asking Apple users are:

 - Is there a strong-enough reason why those who do not want to use
   SSL should be able to choose the SHA-1 implementation available
   from CommonCrypto over block-sha1?

 - Is CommonCrypto SHA-1 a better implementation than block-sha1?

Depending on the answers to these questions, we might want to:

 - add a knob to allow choosing between two available
   implementations (i.e. when NO_APPLE_COMMON_CRYPTO is unset) of
   SHA-1, regardless of the setting of NO_OPENSSL.

 - decide which one between CommonCrypto and block-sha1 should be
   the default.

If we end up deciding that we use block-sha1 as the default, we
should do so even when both NO_OPENSSL and NO_APPLE_COMMON_CRYPTO
are left unset.  If we decide that block-sha1 should merely be a
fallback when no other SHA-1 implementation is availble, on the
other hand, we should be using CommonCrypto SHA-1 as long as the
user did not set NO_APPLE_COMMON_CRYPTO explicitly, even when we are
building with NO_OPENSSL.

If people do not care, we can leave things as they are.  It would
seem mysterious to use block-sha1 when we are not using CommonCrypto
for SSL (i.e. NO_OPENSSL), and otherwise CommonCrypto SHA-1, and
would invite a puzzlement we saw in this thread, though.



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


Re: [PATCH] user-manual: remove temporary branch entry from todo list

2015-12-27 Thread Junio C Hamano
"Stephen P. Smith"  writes:

> Remove the suggestion for using a detached HEAD instead of a
> temporary branch.

That is something we can read from the patch text.  Please explain
why it is a good idea to remove it.

I can think of two completely different reasons:

 (1) Maybe the task was done some time ago, and we are seeing a
 stale todo item?

 (2) The task the todo item hints at was not done, but maybe it is
 not a good thing to do after all?

You seem to be hinting the former, but I do not think "the task was
done" is the case here.

> Signed-off-by: Stephen P. Smith 
> ---
>
> Notes:
> A search of the user manual found only one location which refers to
> temporary branches.  This has to do with how Tony Luck uses them.
> 
> Even then there is a clarifying parenthetical noting that the
> temporary branches are topic branches.
> 
> A git blame showed that the last time that the entry was updated was
> in 2007.
>
>  Documentation/user-manual.txt | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index 1c790ac..18e2f1e 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -4636,9 +4636,6 @@ Scan email archives for other stuff left out
>  Scan man pages to see if any assume more background than this manual
>  provides.
>  
> -Simplify beginning by suggesting disconnected head instead of
> -temporary branch creation?
> -

What does "beginning" refer to in this sentence, though?

After a quick reading of the beginning part of the document, I am
getting the impression that it refers to the use of the 'new'
branch, which is initially created out of v2.6.13 and then later
reset to v2.6.17 while the user is in the sightseeing mode.  And
this way of working _is_ a remnant from the days back when detached
HEAD was not with us.

It is a completely separate matter if it is a good idea to teach
detached HEAD that early in the tutorial, though.  So "remove the
task because detached HEAD is a bit too weird thing to learn in that
early stage in the learning curve" (i.e. the latter reason) might
apply.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/6] apply: fix adding new files on i-t-a entries

2015-12-27 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Suppose that you came up with some contents to register at path F in
> your working tree, told Git about your intention with "add -N F", and
> then tried to apply a patch that wants to _create_ F:
>
> Without this patch, we would say "F already exists so a patch to
> create is incompatible with our current state".
>
> With this patch, i-t-a entries are ignored and we do not say that, but
> instead we'll hopefully trigger "does it exist in the working tree"
> check, unless you are running under "--cached".
>
> Which means that this change will not lead to data loss in the
> "untracked" file F in the working tree that was merely added to the
> index with i-t-a bit.
>
> (commit message mostly from Junio)

Hmm, I do not quite recall.  The above looks under-explained anyway;
we should stress the fact that it is a designed behaviour of this
change to allow only "apply --cached" and continue rejecting other
forms of "apply", but the above makes it sound like it is merely a
coincidence.

It might make sense, from purely mechanistic point of view, to allow
"git apply --cached" to create in the above scenario, but it does
not make any sense to allow "git apply" or "git apply --index", both
of which modifies the working tree (and I do not think the patch
allows the former; I think the latter would fail correctly, but it
may leave the index in a weird state--I didn't check).

"git add -N F" is like saying "I am telling you that F _exists_; I
am just not telling you what its exact contents are yet".  It's like
reserving a table in a restaurant.  The diner might not have arrived
yet, but that does not mean you can give the table to somebody else.

You need to wait for the reservation to be canceled (which you would
do by "git rm --cached F") before you give the table to somebody
else (i.e. creation by the patch).

So from that point of view, I am not convinced "git apply --cached"
should be allowed in such a case, though.

"I thought I told you to that I'll add to this path, but you chose
not to notice that the patch I tried to apply would create the path
with totally different contents and now I am getting from 'git diff'
nonsensical comparison" would be a plausible complaint if we took
this patch.  What is the practical benefit of automatically and
silently canceling the reservation made by an earlier "add -N"?
What workflow benefits from this change, and is this the best
solution to help that workflow?


> Reported-by: Patrick Higgins 
> Reported-by: Bjørnar Snoksrud 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  builtin/apply.c   |  9 +
>  t/t2203-add-intent.sh | 13 +
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 0769b09..315fce8 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -3550,10 +3550,11 @@ static int check_to_create(const char *new_name, int 
> ok_if_exists)
>  {
>   struct stat nst;
>  
> - if (check_index &&
> - cache_name_pos(new_name, strlen(new_name)) >= 0 &&
> - !ok_if_exists)
> - return EXISTS_IN_INDEX;
> + if (check_index && !ok_if_exists) {
> + int pos = cache_name_pos(new_name, strlen(new_name));
> + if (pos >= 0 && !ce_intent_to_add(active_cache[pos]))
> + return EXISTS_IN_INDEX;
> + }
>   if (cached)
>   return 0;
>  
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 2a4a749..bb5ef2b 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -82,5 +82,18 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success 'apply adds new file on i-t-a entry' '
> + git init apply &&
> + (
> + cd apply &&
> + echo newcontent >newfile &&
> + git add newfile &&
> + git diff --cached >patch &&
> + rm .git/index &&
> + git add -N newfile &&
> + git apply --cached patch
> + )
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] user-manual: remove temporary branch entry from todo list

2015-12-27 Thread Stephen & Linda Smith
On Sunday, December 27, 2015 06:41:09 PM Junio C Hamano wrote:
> "Stephen P. Smith"  writes:
> 
> > Remove the suggestion for using a detached HEAD instead of a
> > temporary branch.
> 
> That is something we can read from the patch text.  Please explain
> why it is a good idea to remove it.
> 
> I can think of two completely different reasons:
> 
>  (1) Maybe the task was done some time ago, and we are seeing a
>  stale todo item?
> 
>  (2) The task the todo item hints at was not done, but maybe it is
>  not a good thing to do after all?
> 
> You seem to be hinting the former, but I do not think "the task was
> done" is the case here.
 
I think that this is a stale todo.   
 
The only place there is a mention of temporary branches (which 
is then parenthetically called a topic branch) is in relation to how Tony Luck 
organizes his work.   
Additionally there is already a subsection on using a detatched head 
("Examining an 
old version without creating a new branch). 
 
If there is more that was wanted, then I would be glad to add something, but 
I don't see that there are references to remove.
 
> 
> > Signed-off-by: Stephen P. Smith 
> > ---
> >
> > Notes:
> > A search of the user manual found only one location which refers to
> > temporary branches.  This has to do with how Tony Luck uses them.
> > 
> > Even then there is a clarifying parenthetical noting that the
> > temporary branches are topic branches.
> > 
> > A git blame showed that the last time that the entry was updated was
> > in 2007.
> >
> >  Documentation/user-manual.txt | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> > index 1c790ac..18e2f1e 100644
> > --- a/Documentation/user-manual.txt
> > +++ b/Documentation/user-manual.txt
> > @@ -4636,9 +4636,6 @@ Scan email archives for other stuff left out
> >  Scan man pages to see if any assume more background than this manual
> >  provides.
> >  
> > -Simplify beginning by suggesting disconnected head instead of
> > -temporary branch creation?
> > -
> 
> What does "beginning" refer to in this sentence, though?

I had that question too even after looking at the 2007 version of the manual.

> 
> After a quick reading of the beginning part of the document, I am
> getting the impression that it refers to the use of the 'new'
> branch, which is initially created out of v2.6.13 and then later
> reset to v2.6.17 while the user is in the sightseeing mode.  And
> this way of working _is_ a remnant from the days back when detached
> HEAD was not with us.
> 
> It is a completely separate matter if it is a good idea to teach
> detached HEAD that early in the tutorial, though.  
 
So are you suggesting a move of the section further down?   
Or are you suggesting that that is excised from the manual?

> So "remove the task because detached HEAD is a bit too weird thing to learn 
> in that
> early stage in the learning curve" (i.e. the latter reason) might
> apply.
 
Could it be there are two reasons to remove the todo?

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

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


Re: [PATCH] worktree: stop supporting moving worktrees manually

2015-12-27 Thread Eric Sunshine
On Sun, Dec 27, 2015 at 10:43:16AM +0700, Nguyễn Thái Ngọc Duy wrote:
> The current update_linked_gitdir() has a bug that can create "gitdir"
> file in non-multi-worktree setup. Instead of fixing this, we step back a
> bit. The original design was probably not well thought out. For now, if
> the user manually moves a worktree, they have to fix up "gitdir" file
> manually or the worktree will get pruned. In future, we probably will
> add "git worktree mv" to support this use case.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -33,10 +33,8 @@ The working tree's administrative files in the repository 
> (see
>  If you move a linked working tree to another file system, or
> -within a file system that does not support hard links, you need to run
> -at least one git command inside the linked working tree
> -(e.g. `git status`) in order to update its administrative files in the
> -repository so that they do not get automatically pruned.
> +within a file system that does not support hard links, you need to update

Hmm, is this "hard links" feature implemented? If not, then this
documentation is a bit outdated.

> +$GIT_DIR/worktrees//gitdir so that they do not get automatically pruned.

Following the example of af189b4 (Documentation/git-worktree: split
technical info from general description, 2015-07-06), it might be a
good idea to keep this high-level overview free of such low-level
details and instead mention $GIT_DIR/worktrees//gitdir in the
"DETAILS" section.

Perhaps something like this, on top of your patch (assuming that the
"hard links" feature is not implemented):

--- 8< ---
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 4814f48..62c76c1 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -32,9 +32,9 @@ The working tree's administrative files in the repository (see
 `git worktree prune` in the main or any linked working tree to
 clean up any stale administrative files.
 
-If you move a linked working tree to another file system, or
-within a file system that does not support hard links, you need to update
-$GIT_DIR/worktrees//gitdir so that they do not get automatically pruned.
+If you move a linked working tree, you need to manually update the
+administrative files so that they do not get pruned automatically. See
+section "DETAILS" for more information.
 
 If a linked working tree is stored on a portable device or network share
 which is not always mounted, you can prevent its administrative files from
@@ -135,6 +135,13 @@ thumb is do not make any assumption about whether a path 
belongs to
 $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something
 inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path.
 
+If you move a linked working tree, you need to update the 'gitdir' file
+in the entry's directory. For example, if a linked working tree is moved
+to `/newpath/test-next` and its `.git` file points to
+`/path/main/.git/worktrees/test-next`, then update
+`/path/main/.git/worktrees/test-next/gitdir` to reference `/newpath/test-next`
+instead.
+
 To prevent a $GIT_DIR/worktrees entry from being pruned (which
 can be useful in some situations, such as when the
 entry's working tree is stored on a portable device), add a file named
--- 8< ---
--
To unsubscribe from this list: send the line "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 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

2015-12-27 Thread Johannes Sixt

Am 28.12.2015 um 02:59 schrieb Junio C Hamano:

Junio C Hamano  writes:


Very true.  Let's leave that kind of things as separate clean-ups
after these patches settle, as mixing manual and mechanical changes
in a single patch makes it harder to review.

Thanks.


So that I won't forget (I'll need to amend with your sign-off even
if this one is satisfactory to you).

-- >8 --
From: Johannes Sixt 
Date: Tue, 22 Dec 2015 19:35:16 +0100
Subject: [PATCH] t5100: no need to use 'echo' command substitutions for globbing

Instead of making the shell expand 00* and invoke 'echo' with it,
and then capturing its output as command substitution, just use
the result of expanding 00* directly.

Signed-off-by: Junio C Hamano 
---
  t/t5100-mailinfo.sh | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 4e52b3b..85b3df5 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -23,7 +23,7 @@ check_mailinfo () {
  }


-for mail in $(echo 00*)
+for mail in 00*
  do
test_expect_success "mailinfo $mail" '
check_mailinfo $mail "" &&
@@ -51,7 +51,7 @@ test_expect_success 'split box with rfc2047 samples' \
echo total is $last &&
test $(cat rfc2047/last) = 11'

-for mail in $(echo rfc2047/00*)
+for mail in rfc2047/00*
  do
test_expect_success "mailinfo $mail" '
git mailinfo -u $mail-msg $mail-patch <$mail >$mail-info &&



Signed-off-by: Johannes Sixt 

Thank you for making a proper patch!

-- Hannes

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