Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin

[David Aguilar]

[cc'd Roberto for submitGit q's]

On Thu, Jul 21, 2016 at 12:56:51AM +, Brett Cundal wrote:
> ---



The message on the pull request[1] has a better justification
for this change, which would have been nice in the commit
message itself:


Git 2.9 added a check against merging unrelated histories, which
is exactly what git subtree with --rejoin does. Adding the
--allow-unrelated-histories flag to merge will override this
check.


Is it possible that maybe submitGit can detect an empty commit
message for single-commit PRs and transplant that message onto it?

As-is, the commit itself should probably be amended to contain
that information.

>  contrib/subtree/git-subtree.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7a39b30..556cd92 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -661,7 +661,7 @@ cmd_split()
>   if [ -n "$rejoin" ]; then
>   debug "Merging split branch into HEAD..."
>   latest_old=$(cache_get latest_old)
> - git merge -s ours \
> + git merge -s ours --allow-unrelated-histories \
>   -m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
>   $latest_new >&2 || exit $?
>   fi
> 
> --

With the above description this change makes more sense,
but it seems that the existing tests do not detect the breakage
fixed by this patch.

Can you please add a test case in t/t7900-subtree.sh
demonstrating the breakage?

Looks good otherwise.

[1] https://github.com/git/git/pull/274
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 08/12] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

[Pranit Bauva]

Reimplement `is_expected_rev` & `check_expected_revs` shell function in
C and add a `--check-expected-revs` subcommand to `git bisect--helper` to
call it from git-bisect.sh .

Using `--check-expected-revs` subcommand is a temporary measure to port
shell functions to C so as to use the existing test suite. As more
functions are ported, this subcommand would be retired and will be
called by some other method.

Helped-by: Eric Sunshine 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 33 -
 git-bisect.sh| 20 ++--
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d125fd3..86bb334 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -162,13 +162,40 @@ static int bisect_reset(const char *commit)
return bisect_clean_state();
 }
 
+static int is_expected_rev(const char *expected_hex)
+{
+   struct strbuf actual_hex = STRBUF_INIT;
+   int res = 0;
+   if (strbuf_read_file(_hex, git_path_bisect_expected_rev(), 0) >= 
0) {
+   strbuf_trim(_hex);
+   res = !strcmp(actual_hex.buf, expected_hex);
+   }
+   strbuf_release(_hex);
+   return res;
+}
+
+static int check_expected_revs(const char **revs, int rev_nr)
+{
+   int i;
+
+   for (i = 0; i < rev_nr; i++) {
+   if (!is_expected_rev(revs[i])) {
+   remove_path(git_path_bisect_ancestors_ok());
+   remove_path(git_path_bisect_expected_rev());
+   return 0;
+   }
+   }
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
WRITE_TERMS,
BISECT_CLEAN_STATE,
-   BISECT_RESET
+   BISECT_RESET,
+   CHECK_EXPECTED_REVS
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -180,6 +207,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_CMDMODE(0, "bisect-reset", ,
 N_("reset the bisection state"), BISECT_RESET),
+   OPT_CMDMODE(0, "check-expected-revs", ,
+N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -206,6 +235,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
if (argc > 1)
die(_("--bisect-reset requires either zero or one 
arguments"));
return bisect_reset(argc ? argv[0] : NULL);
+   case CHECK_EXPECTED_REVS:
+   return check_expected_revs(argv, argc);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 18580b7..4f6545e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -238,22 +238,6 @@ bisect_write() {
test -n "$nolog" || echo "git bisect $state $rev" 
>>"$GIT_DIR/BISECT_LOG"
 }
 
-is_expected_rev() {
-   test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
-   test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
-}
-
-check_expected_revs() {
-   for _rev in "$@"; do
-   if ! is_expected_rev "$_rev"
-   then
-   rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
-   rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
-   return
-   fi
-   done
-}
-
 bisect_skip() {
all=''
for arg in "$@"
@@ -280,7 +264,7 @@ bisect_state() {
rev=$(git rev-parse --verify $(bisect_head)) ||
die "$(gettext "Bad rev input: $(bisect_head)")"
bisect_write "$state" "$rev"
-   check_expected_revs "$rev" ;;
+   git bisect--helper --check-expected-revs "$rev" ;;
2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip)
shift
hash_list=''
@@ -294,7 +278,7 @@ bisect_state() {
do
bisect_write "$state" "$rev"
done
-   check_expected_revs $hash_list ;;
+   git bisect--helper --check-expected-revs $hash_list ;;
*,"$TERM_BAD")
die "$(eval_gettext "'git bisect \$TERM_BAD' can take only one 
argument.")" ;;
*)

--
https://github.com/git/git/pull/273
--
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  

Re: [PATCH] submodule: do no re-read name in shell script

[Stefan Beller]

On Wed, Jul 20, 2016 at 6:00 PM, Jonathan Nieder  wrote:

>> - needs_cloning, ce->name);
>> + needs_cloning, sub->name, sub->path);
>
> Are there any restrictions on what characters a submodule name can
> contain?  Does this need e.g. to be quoted with sq_quote?

Oh good point, this ought to fail with weird names. So instead of quoting it,
let's just drop this patch.

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

[PATCH] submodule--helper: correct index computation

[Stefan Beller]

In 665b35eccd39 (2016-06-09, submodule--helper: initial clone learns
retry logic), the index computation for the second round is wrong; it
should make use of the index that was handed in via the idx_task_cb
pointer. When that commit was introduced, I wrote:

> I wonder how we can test this properly as the git binary we have here
> is too reliable, but I observed the correctness of this patch when
> cloning a repo with lots of submodules and one of them failing.

This was only partially true as I did not observe the second try failing,
only the first retry succeeding. Testing however is really easy, when the
url of one submodule is bogus, so add a test for that.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c |  4 +++-
 t/t7406-submodule-update.sh | 16 
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 494e088..5d54885 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -795,7 +795,9 @@ static int update_clone_task_finished(int result,
suc->failed_clones[suc->failed_clones_nr++] = ce;
return 0;
} else {
-   idx = suc->current - suc->list.nr;
+   idx -= suc->list.nr;
+   if (idx >= suc->failed_clones_nr)
+   die("BUG: idx too large???");
ce  = suc->failed_clones[idx];
strbuf_addf(err, _("Failed to clone '%s' a second time, 
aborting"),
ce->name);
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 88e9750..ffb329f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -889,4 +889,20 @@ test_expect_success 'git clone passes the parallel jobs 
config on to submodules'
rm -rf super4
 '
 
+cat << EOF >expect
+Submodule 'deeper/submodule' (bogus-url) registered for path 'deeper/submodule'
+fatal: clone of 'bogus-url' into submodule path '$pwd/super4/deeper/submodule' 
failed
+Failed to clone 'deeper/submodule'. Retry scheduled
+fatal: clone of 'bogus-url' into submodule path '$pwd/super4/deeper/submodule' 
failed
+Failed to clone 'deeper/submodule' a second time, aborting
+EOF
+
+test_expect_success 'correct message for retries' '
+   test_when_finished "rm -rf super4" &&
+   git -C super config -f .gitmodules submodule."deeper/submodule".url 
bogus-url &&
+   git -C super commit -a -m "bogus url for one submodule" &&
+   test_must_fail git clone --recurse-submodules -j 1 super super4 2>&1 | 
grep deeper >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.9.2.370.g4a59376.dirty

--
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] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin

[Brett Cundal]

---
 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7a39b30..556cd92 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -661,7 +661,7 @@ cmd_split()
if [ -n "$rejoin" ]; then
debug "Merging split branch into HEAD..."
latest_old=$(cache_get latest_old)
-   git merge -s ours \
+   git merge -s ours --allow-unrelated-histories \
-m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
$latest_new >&2 || exit $?
fi

--
https://github.com/git/git/pull/274
--
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] config.mak.uname: set PERL_PATH for FreeBSD 5.0+

[brian m. carlson]

On Wed, Jul 20, 2016 at 11:10:40AM -0700, Junio C Hamano wrote:
> On Wed, Jul 20, 2016 at 11:07 AM, Eric Wong  wrote:
> > Also, my use of a numeric comparison may be more future-proof
> > in case FreeBSD decides to have /usr/bin/perl again.
> >
> > I also wonder why we don't use `which` to search for somewhat
> > standard path components, instead.  Something like:
> 
> Because historically output from "which" was not meant to be
> machine parseable (some implementation said 'perl is /usr/bin/perl')

The POSIXy way to write which is "command -v", which may or may not be
more portable.  It does have the desired output, though.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] submodule: do no re-read name in shell script

[Jonathan Nieder]

Stefan Beller wrote:

> Instead of making another call to a submodule helper (name), just
> propagate the value when we know it (in the update-clone helper) already.
>
> Signed-off-by: Stefan Beller 
> ---
>  builtin/submodule--helper.c | 4 ++--
>  git-submodule.sh| 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b22352b..494e088 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -683,9 +683,9 @@ static int prepare_to_clone_next_submodule(const struct 
> cache_entry *ce,
>   needs_cloning = !file_exists(sb.buf);
> 
>   strbuf_reset();
> - strbuf_addf(, "%06o %s %d %d\t%s\n", ce->ce_mode,
> + strbuf_addf(, "%06o %s %d %d %s\t%s\n", ce->ce_mode,
>   sha1_to_hex(ce->sha1), ce_stage(ce),
> - needs_cloning, ce->name);
> + needs_cloning, sub->name, sub->path);

Are there any restrictions on what characters a submodule name can
contain?  Does this need e.g. to be quoted with sq_quote?

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

[PATCH] contrib/persistent-https: use Git version for build label

[Parker Moore]

From: Parker Moore 

The previous method simply used the UNIX timestamp of when the binary was
built as its build label.

$ make && ./git-remote-persistent-http -print_label
1469061546

This patch aims to align the label for this binary with the Git version
contained in the GIT-VERSION-FILE. This gives a better sense of the version
of the binary as it can be mapped to a particular revision or release of
Git itself. For example:

$ make && ./git-remote-persistent-http -print_label
2.9.1.275.g75676c8

Discussion of this patch is available on a related thread in the mailing
list surrounding this package called "contrib/persistent-https: update
ldflags syntax for Go 1.7+". The gmane.org link is:
http://article.gmane.org/gmane.comp.version-control.git/299653/

Signed-off-by: Parker Moore 
---
 contrib/persistent-https/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/persistent-https/Makefile
b/contrib/persistent-https/Makefile
index 92baa3be..8248269 100644
--- a/contrib/persistent-https/Makefile
+++ b/contrib/persistent-https/Makefile
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

-BUILD_LABEL=$(shell date +"%s")
+BUILD_LABEL=$(shell cat ../../GIT-VERSION-FILE | cut -d" " -f3)
 TAR_OUT=$(shell go env GOOS)_$(shell go env GOARCH).tar.gz

 all: git-remote-persistent-https git-remote-persistent-https--proxy \
--
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] contrib/persistent-https: update ldflags syntax for Go 1.7+

[Parker Moore]

From: Parker Moore 

This fixes contrib/persistent-https builds for Go v1.7+ and is
compatible with Go v1.0+.

Running `make all` in `contrib/persistent-https` results in a failure
on Go 1.7 and above.

Specifically, the error is:

go build -o git-remote-persistent-https \
   -ldflags "-X main._BUILD_EMBED_LABEL 1468613136"
# _/Users/parkr/github/git/contrib/persistent-https
/usr/local/Cellar/go/1.7rc1/libexec/pkg/tool/darwin_amd64/link: -X
flag requires argument of the form importpath.name=value
make: *** [git-remote-persistent-https] Error 2

This `name=value` syntax for the -X flag was introduced in Go v1.5
(released Aug 19, 2015):

- release notes: https://golang.org/doc/go1.5#link
- commit: 
https://github.com/golang/go/commit/12795c02f3d6fc54ece09a86e70aaa40a94d5131

In Go v1.7, support for the old syntax was removed:

- release notes: https://tip.golang.org/doc/go1.7#compiler
- commit: 
https://github.com/golang/go/commit/51b624e6a29b135ce0fadb22b678acf4998ff16f

This patch includes the `=` to fix builds with Go v1.7+.

Thanks-to: Junio C Hamano 
Signed-off-by: Parker Moore 
---
 contrib/persistent-https/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/persistent-https/Makefile
b/contrib/persistent-https/Makefile
index 92baa3be..2b4173d 100644
--- a/contrib/persistent-https/Makefile
+++ b/contrib/persistent-https/Makefile
@@ -25,8 +25,10 @@ git-remote-persistent-http: git-remote-persistent-https
ln -f -s git-remote-persistent-https git-remote-persistent-http

 git-remote-persistent-https:
+   case $$(go version) in \
+   "go version go"1.[0-5].*) EQ=" " ;; *) EQ="=" ;; esac && \
go build -o git-remote-persistent-https \
-   -ldflags "-X main._BUILD_EMBED_LABEL $(BUILD_LABEL)"
+   -ldflags "-X main._BUILD_EMBED_LABEL$${EQ}$(BUILD_LABEL)"

 clean:
rm -f git-remote-persistent-http* *.tar.gz
--
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: do no re-read name in shell script

[Stefan Beller]

Instead of making another call to a submodule helper (name), just
propagate the value when we know it (in the update-clone helper) already.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 4 ++--
 git-submodule.sh| 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b22352b..494e088 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -683,9 +683,9 @@ static int prepare_to_clone_next_submodule(const struct 
cache_entry *ce,
needs_cloning = !file_exists(sb.buf);
 
strbuf_reset();
-   strbuf_addf(, "%06o %s %d %d\t%s\n", ce->ce_mode,
+   strbuf_addf(, "%06o %s %d %d %s\t%s\n", ce->ce_mode,
sha1_to_hex(ce->sha1), ce_stage(ce),
-   needs_cloning, ce->name);
+   needs_cloning, sub->name, sub->path);
string_list_append(>projectlines, sb.buf);
 
if (!needs_cloning)
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..e23aada 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -584,11 +584,10 @@ cmd_update()
"$@" || echo "#unmatched"
} | {
err=
-   while read mode sha1 stage just_cloned sm_path
+   while read mode sha1 stage just_cloned name sm_path
do
die_if_unmatched "$mode"
 
-   name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
branch=$(get_submodule_config "$name" branch master)
if ! test -z "$update"
-- 
2.9.2.369.g0e67330

--
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: How to generate feature branch statistics?

[Junio C Hamano]

On Wed, Jul 20, 2016 at 4:10 PM, Jakub Narębski  wrote:
> W dniu 2016-07-20 o 20:49, Junio C Hamano pisze:
>
>> For our own history and workflow, the duration between the inception
>> of a topic branch and the time it gets merged to 'master' is not all
>> that interesting.
>
> Nb. if I haven't messed something up (the git history is not simple
> merging of topic branches into mainline), the shortest time from
> creating a branch to merging it in git.git is 7 seconds (probably
> it was a bugfix-type of a topic branch), the longest if I did it
> correctly is slightly less than 4 years (???): 641830c.

The former is quite understandable. The point of having such a topic
is so that it can be merged down to older maintenance releases.
--
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 3/4] submodule: support running in multiple worktree setup

[Stefan Beller]

On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
 wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  Documentation/git-worktree.txt | 8 
>  git-submodule.sh   | 8 
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 41350db..2a5661d 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -142,6 +142,14 @@ to share to all working directories:
> you are sure you always use sparse checkout for all working
> directories.
>
> + - `submodule.*` in current state should not be shared because the
> +   information is tied to a particular version of .gitmodules in a
> +   working directory.

While the submodule.* settings are copied from the .gitmodules file initially,
they can be changed in the config later. (That was actually the whole
point of it,
so you can change the submodule remotes URL without having to change history.)

And I would think that most submodule related settings (such as remote URL,
name, path, even depth recommendation) should be the same for all worktrees,
and a different value for one worktree is a carefully crafted
exception by the user.

So while the .gitmodules file can diverge in the work trees I do not
think that the
actual remotes for the submodules in the different worktrees differ
though. The change
of the .gitmodule files may be because you checked out an old commit, that
has outdated information on where to get the submodule from.

> +
> + - `remote.*` added by submodules may be per working directory as
> +   well, unless you are sure remotes from all possible submodules in
> +   history are consistent.
> +
>  DETAILS
>  ---
>  Each linked working tree has a private sub-directory in the repository's
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 4ec7546..7b576f5 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -261,7 +261,7 @@ or you are unsure what this means choose another name 
> with the '--name' option."
> esac
> ) || die "$(eval_gettext "Unable to checkout submodule 
> '\$sm_path'")"
> fi
> -   git config submodule."$sm_name".url "$realrepo"
> +   git config --worktree submodule."$sm_name".url "$realrepo"

This is in cmd_add. Actually I think this should be --not-worktree
(i.e. --local)
as when you add a submodule in one worktree, and then in another,
you may want to have the same URL. However if another worktree
already configured it you want to keep the option.
so rather:

  if git config  submodule."$sm_name".url then
  # it exists, do nothing
  else
# it does not exist
git config --local ...

>
> git add $force "$sm_path" ||
> die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
> @@ -461,7 +461,7 @@ Submodule work tree '\$displaypath' contains a .git 
> directory
> # Remove the whole section so we have a clean state 
> when
> # the user later decides to init this submodule again
> url=$(git config submodule."$name".url)
> -   git config --remove-section submodule."$name" 
> 2>/dev/null &&
> +   git config --worktree --remove-section 
> submodule."$name" 2>/dev/null &&
> say "$(eval_gettext "Submodule '\$name' (\$url) 
> unregistered for path '\$displaypath'")"

This is in cmd_deinit, which is documented as:
   Unregister the given submodules, i.e. remove the whole
   submodule.$name section from .git/config together with their work
   tree. Further calls to git submodule update, git submodule foreach
   and git submodule sync will skip any unregistered submodules until
   they are initialized again, so use this command if you don’t want
   to have a local checkout of the submodule in your work tree
   anymore. If you really want to remove a submodule from the
   repository and commit that use git-rm(1) instead.

So one might wonder if the unregister should be a global unregister
or a worktree specific unregister.

>From a users POV there are:
* non existent submodules (no gitlink recorded, no config set,
  no repo in place)
* not initialized submodules (gitlink is recorded, no config set,
  and an empty repo is put in the working tree as a place holder).
* initialized submodules (gitlink is recorded, the config
  submodule ..url is copied from the .gitmodules file to .git/config.
  an empty dir in the working tree as a place holder)
  A user may change the configuration before the next step as the url in
  the .gitmodules file may be wrong and the user doesn't want to
  rewrite history
* existing submodules (gitlink is recorded, the config option is set
  and instead of an empty placeholder dir, we actually have a git
  repo there.)
* matching 

Re: How to generate feature branch statistics?

[Jakub Narębski]

W dniu 2016-07-20 o 20:49, Junio C Hamano pisze:

> For our own history and workflow, the duration between the inception
> of a topic branch and the time it gets merged to 'master' is not all
> that interesting.

Nb. if I haven't messed something up (the git history is not simple
merging of topic branches into mainline), the shortest time from
creating a branch to merging it in git.git is 7 seconds (probably
it was a bugfix-type of a topic branch), the longest if I did it
correctly is slightly less than 4 years (???): 641830c.

-- 
Jakub Narębski


--
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 v10 09/12] bisect--helper: `bisect_write` shell function in C

[Pranit Bauva]

Reimplement the `bisect_write` shell function in C and add a
`bisect-write` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--bisect-write` subcommand is a temporary measure to port shell
function in C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.

Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD
from the global shell script thus we need to pass it to the subcommand
using the arguments. We then store them in a struct bisect_terms and
pass the memory address around functions.

This patch also introduces new methods namely bisect_state_init() and
bisect_terms_release() for easy memory management for the struct
bisect_terms.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 97 
 git-bisect.sh| 25 ++---
 2 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 86bb334..d1d12f2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -22,9 +22,27 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --write-terms  "),
N_("git bisect--helper --bisect-clean-state"),
N_("git bisect--helper --bisect-reset []"),
+   N_("git bisect--helper --bisect-write
 []"),
NULL
 };
 
+struct bisect_terms {
+   struct strbuf term_good;
+   struct strbuf term_bad;
+};
+
+static void bisect_terms_init(struct bisect_terms *terms)
+{
+   strbuf_init(>term_good, 0);
+   strbuf_init(>term_bad, 0);
+}
+
+static void bisect_terms_release(struct bisect_terms *terms)
+{
+   strbuf_release(>term_good);
+   strbuf_release(>term_bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -188,6 +206,52 @@ static int check_expected_revs(const char **revs, int 
rev_nr)
return 0;
 }
 
+static int bisect_write(const char *state, const char *rev,
+   const struct bisect_terms *terms, int nolog)
+{
+   struct strbuf tag = STRBUF_INIT;
+   struct strbuf commit_name = STRBUF_INIT;
+   struct object_id oid;
+   struct commit *commit;
+   struct pretty_print_context pp = {0};
+   FILE *fp;
+
+   if (!strcmp(state, terms->term_bad.buf))
+   strbuf_addf(, "refs/bisect/%s", state);
+   else if (one_of(state, terms->term_good.buf, "skip", NULL))
+   strbuf_addf(, "refs/bisect/%s-%s", state, rev);
+   else
+   return error(_("Bad bisect_write argument: %s"), state);
+
+   if (get_oid(rev, )) {
+   strbuf_release();
+   return error(_("couldn't get the oid of the rev '%s'"), rev);
+   }
+
+   if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
+  UPDATE_REFS_MSG_ON_ERR)) {
+   strbuf_release();
+   return -1;
+   }
+   strbuf_release();
+
+   fp = fopen(git_path_bisect_log(), "a");
+   if (!fp)
+   return error_errno(_("couldn't open the file '%s'"), 
git_path_bisect_log());
+
+   commit = lookup_commit_reference(oid.hash);
+   format_commit_message(commit, "%s", _name, );
+   fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
+   commit_name.buf);
+   strbuf_release(_name);
+
+   if (!nolog)
+   fprintf(fp, "git bisect %s %s\n", state, rev);
+
+   fclose(fp);
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -195,9 +259,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
WRITE_TERMS,
BISECT_CLEAN_STATE,
BISECT_RESET,
-   CHECK_EXPECTED_REVS
+   CHECK_EXPECTED_REVS,
+   BISECT_WRITE
} cmdmode = 0;
-   int no_checkout = 0;
+   int no_checkout = 0, res = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -209,10 +274,14 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("reset the bisection state"), BISECT_RESET),
OPT_CMDMODE(0, "check-expected-revs", ,
 N_("check for expected revs"), CHECK_EXPECTED_REVS),
+   OPT_CMDMODE(0, "bisect-write", ,
+N_("write out the bisection state in BISECT_LOG"), 
BISECT_WRITE),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
};
+  

[PATCH v10 07/12] bisect--helper: `bisect_reset` shell function in C

[Pranit Bauva]

Reimplement `bisect_reset` shell function in C and add a `--bisect-reset`
subcommand to `git bisect--helper` to call it from git-bisect.sh .

Using `bisect_reset` subcommand is a temporary measure to port shell
functions to C so as to use the existing test suite. As more functions
are ported, this subcommand would be retired and will be called by some
other method.

Note: --bisect-clean-state subcommand has not been retired as there are
still a function namely `bisect_start()` which still uses this
subcommand.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 47 ++-
 git-bisect.sh| 28 ++--
 2 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index ad67a97..d125fd3 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,6 +4,8 @@
 #include "bisect.h"
 #include "refs.h"
 #include "dir.h"
+#include "argv-array.h"
+#include "run-command.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -13,11 +15,13 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 static GIT_PATH_FUNC(git_path_head_name, "head-name")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
+static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
N_("git bisect--helper --write-terms  "),
N_("git bisect--helper --bisect-clean-state"),
+   N_("git bisect--helper --bisect-reset []"),
NULL
 };
 
@@ -124,12 +128,47 @@ static int bisect_clean_state(void)
return result;
 }
 
+static int bisect_reset(const char *commit)
+{
+   struct strbuf branch = STRBUF_INIT;
+
+   if (!commit) {
+   if (strbuf_read_file(, git_path_bisect_start(), 0) < 1) {
+   printf("We are not bisecting.\n");
+   return 0;
+   }
+   strbuf_rtrim();
+   } else {
+   struct object_id oid;
+   if (get_oid(commit, ))
+   return error(_("'%s' is not a valid commit"), commit);
+   strbuf_addstr(, commit);
+   }
+
+   if (!file_exists(git_path_bisect_head())) {
+   struct argv_array argv = ARGV_ARRAY_INIT;
+   argv_array_pushl(, "checkout", branch.buf, "--", NULL);
+   if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+   error(_("Could not check out original HEAD '%s'. Try"
+   "'git bisect reset '."), branch.buf);
+   strbuf_release();
+   argv_array_clear();
+   return -1;
+   }
+   argv_array_clear();
+   }
+
+   strbuf_release();
+   return bisect_clean_state();
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
WRITE_TERMS,
-   BISECT_CLEAN_STATE
+   BISECT_CLEAN_STATE,
+   BISECT_RESET
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -139,6 +178,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("write the terms to .git/BISECT_TERMS"), 
WRITE_TERMS),
OPT_CMDMODE(0, "bisect-clean-state", ,
 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
+   OPT_CMDMODE(0, "bisect-reset", ,
+N_("reset the bisection state"), BISECT_RESET),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -161,6 +202,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
if (argc != 0)
die(_("--bisect-clean-state requires no arguments"));
return bisect_clean_state();
+   case BISECT_RESET:
+   if (argc > 1)
+   die(_("--bisect-reset requires either zero or one 
arguments"));
+   return bisect_reset(argc ? argv[0] : NULL);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index bbc57d2..18580b7 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -409,35 +409,11 @@ bisect_visualize() {
eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
 }
 
-bisect_reset() {
-   test -s "$GIT_DIR/BISECT_START" || {
-   

Re: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE

[David Aguilar]

On Tue, Jul 19, 2016 at 02:06:35PM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > It is not wrong per-se, but as you are in a subshell, you do not
> > have to unset these, I would think.  Not worth a reroll, but unless
> > I am overlooking something (in which case please holler) I'm
> > inclined to remove these two lines myself while queuing the series.
> 
> I propose to squashing the following to 2/3 (and adjusting 3/3 as
> needed).  No need to resend if you agree it is a good idea, as it is
> part of what I've queued on 'pu'.
> 
> Thanks.


I had originally meant to squash that in but it slipped through.
It looks great.

Thank you!


>  git-difftool.perl   | 2 +-
>  t/t7800-difftool.sh | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/git-difftool.perl b/git-difftool.perl
> index bc2267f..c81cbe4 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -88,11 +88,11 @@ sub changed_files
>   my @refreshargs = (
>   @gitargs, 'update-index',
>   '--really-refresh', '-q', '--unmerged');
> - my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
>   try {
>   Git::command_oneline(@refreshargs);
>   } catch Git::Error::Command with {};
>  
> + my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
>   my $line = Git::command_oneline(@diffargs);
>   my @files;
>   if (defined $line) {
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index afdf370..cb25480 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -421,8 +421,6 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory 
> with GIT_DIR set' '
>   cd sub &&
>   git difftool --dir-diff $symlinks --extcmd ls \
>   branch -- sub >output &&
> - sane_unset GIT_WORK_TREE &&
> - sane_unset GIT_DIR &&
>   grep sub output &&
>   ! grep file output
>   )
> -- 
> 2.9.2-581-g77f0ffb
> 

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

Re: [PATCH v4 0/8] Name for A..B ranges?

[Junio C Hamano]

Philip Oakley  writes:

> No change in the number of patches. Interdiff below.
>
> The patches carefully tease out the clarification of
> reachability. Reachability is defined relative the ancestry chain thus
> (hopefully) avoiding misunderstandings.
>
> The final patch updates the summary examples, and the tricky (for the
> untutored reader) two dots case of a linear development where r1..r2
> excludes r1 itself.

All looked sensible and each focused on a single issue and fixing it
well.  Done very nicely.

Thanks.  Will (re)queue, wait for a few days for further comments
and let's merge it to 'next'.

--
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 v10 03/12] bisect--helper: `write_terms` shell function in C

[Pranit Bauva]

Reimplement the `write_terms` shell function in C and add a `write-terms`
subcommand to `git bisect--helper` to call it from git-bisect.sh . Also
remove the subcommand `--check-term-format` as it can now be called from
inside the function write_terms() C implementation.

Also `|| exit` is added when calling write-terms subcommand from
git-bisect.sh so as to exit whenever there is an error.

Using `--write-terms` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other method.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 36 +---
 git-bisect.sh| 22 +++---
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 48285d4..ef87c82 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,9 +4,11 @@
 #include "bisect.h"
 #include "refs.h"
 
+static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
-   N_("git bisect--helper --check-term-format  "),
+   N_("git bisect--helper --write-terms  "),
NULL
 };
 
@@ -56,18 +58,38 @@ static int check_term_format(const char *term, const char 
*orig_term)
return 0;
 }
 
+static int write_terms(const char *bad, const char *good)
+{
+   FILE *fp;
+   int res;
+
+   if (!strcmp(bad, good))
+   return error(_("please use two different terms"));
+
+   if (check_term_format(bad, "bad") || check_term_format(good, "good"))
+   return -1;
+
+   fp = fopen(git_path_bisect_terms(), "w");
+   if (!fp)
+   return error_errno(_("could not open the file BISECT_TERMS"));
+
+   res = fprintf(fp, "%s\n%s\n", bad, good);
+   fclose(fp);
+   return (res < 0) ? -1 : 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
-   CHECK_TERM_FMT
+   WRITE_TERMS
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
-   OPT_CMDMODE(0, "check-term-format", ,
-N_("check format of the term"), CHECK_TERM_FMT),
+   OPT_CMDMODE(0, "write-terms", ,
+N_("write the terms to .git/BISECT_TERMS"), 
WRITE_TERMS),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -82,10 +104,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
-   case CHECK_TERM_FMT:
+   case WRITE_TERMS:
if (argc != 2)
-   die(_("--check-term-format requires two arguments"));
-   return check_term_format(argv[0], argv[1]);
+   die(_("--write-terms requires two arguments"));
+   return write_terms(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 7d7965d..cd39bd0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -210,7 +210,7 @@ bisect_start() {
eval "$eval true" &&
if test $must_write_terms -eq 1
then
-   write_terms "$TERM_BAD" "$TERM_GOOD"
+   git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD"
fi &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
@@ -557,18 +557,6 @@ get_terms () {
fi
 }
 
-write_terms () {
-   TERM_BAD=$1
-   TERM_GOOD=$2
-   if test "$TERM_BAD" = "$TERM_GOOD"
-   then
-   die "$(gettext "please use two different terms")"
-   fi
-   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
-   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
-   printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
-}
-
 check_and_set_terms () {
cmd="$1"
case "$cmd" in
@@ -582,13 +570,17 @@ check_and_set_terms () {
bad|good)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
-   write_terms bad good
+   TERM_BAD=bad
+   TERM_GOOD=good
+   git bisect--helper --write-terms 

Re: [PATCH v4] config: add conditional include

[Junio C Hamano]

Jeff King  writes:

> On Sun, Jul 17, 2016 at 10:15:55AM +0200, Johannes Schindelin wrote:
>
>> FWIW I am slightly less worried about the conditional includes (it is
>> already a horrible mess to figure out too-long include chains now, before
>> having conditional includes, for example). I am slightly more worried
>> about eventually needing to introduce support for something like
>> 
>>  [if-gitdir(...):section]
>>  thisSettingIsConditional = ...
>> 
>> or even
>> 
>>  [if (worktree==...):section]
>>  anotherConditional = ...
>> 
>> and then having two incompatible conditional constructs, one generic, the
>> other one specific to [include].
>> 
>> In other words, if we already introduce a conditional construct, I'd
>> rather have one that could easily be used for other conditions/sections
>> when (and if) needed.
>
> I had assumed we would resist introducing anything like that, simply
> because of backwards compatibility issues with the syntax. But I admit
> that was just an assumption in my head; future compatibility with
> reality is not guaranteed. :)

I actually read that assumption between lines and almost wrote the
same response that begins with "I think the untold assumption ever
since the inclusion mechanism was introduced is..." ;-)

A config file with "[include] path=..." in it would not include from
the named path by ancient verison of Git, but at least it won't
cause its parser to barf, and the assumption has been that it is a
good property to keep when we introduce new and incompatible
features.

I can however understand it if somebody thinks it actually is better
to actively break older Git implementations by forcing them to stop
parsing when we introduce constructs that will lead them to do wrong
things (e.g. missing some configuration defintions by not reading
from the file that the user wanted to be read from), rather than
making them silently ignore.
--
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 v10 02/12] bisect: rewrite `check_term_format` shell function in C

[Pranit Bauva]

Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh

Using `--check-term-format` subcommand is a temporary measure to port
shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and will
be called by some other method/subcommand. For eg. In conversion of
write_terms() of git-bisect.sh, the subcommand will be removed and
instead check_term_format() will be called in its C implementation while
a new subcommand will be introduced for write_terms().

Helped-by: Johannes Schindelein 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 59 +++-
 git-bisect.sh| 31 ++---
 2 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8111c91..48285d4 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,19 +2,72 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
+   N_("git bisect--helper --check-term-format  "),
NULL
 };
 
+/*
+ * Check whether the string `term` belongs to the set of strings
+ * included in the variable arguments.
+ */
+static int one_of(const char *term, ...)
+{
+   int res = 0;
+   va_list matches;
+   const char *match;
+
+   va_start(matches, term);
+   while (!res && (match = va_arg(matches, const char *)))
+   res = !strcmp(term, match);
+   va_end(matches);
+
+   return res;
+}
+
+static int check_term_format(const char *term, const char *orig_term)
+{
+   int res;
+   char *new_term = xstrfmt("refs/bisect/%s", term);
+
+   res = check_refname_format(new_term, 0);
+   free(new_term);
+
+   if (res)
+   return error(_("'%s' is not a valid term"), term);
+
+   if (one_of(term, "help", "start", "skip", "next", "reset",
+   "visualize", "replay", "log", "run", NULL))
+   return error(_("can't use the builtin command '%s' as a term"), 
term);
+
+   /*
+* In theory, nothing prevents swapping completely good and bad,
+* but this situation could be confusing and hasn't been tested
+* enough. Forbid it for now.
+*/
+
+   if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
+(strcmp(orig_term, "good") && one_of(term, "good", "old", 
NULL)))
+   return error(_("can't change the meaning of the term '%s'"), 
term);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-   enum { NEXT_ALL = 1 } cmdmode = 0;
+   enum {
+   NEXT_ALL = 1,
+   CHECK_TERM_FMT
+   } cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
+   OPT_CMDMODE(0, "check-term-format", ,
+N_("check format of the term"), CHECK_TERM_FMT),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -29,6 +82,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
+   case CHECK_TERM_FMT:
+   if (argc != 2)
+   die(_("--check-term-format requires two arguments"));
+   return check_term_format(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..7d7965d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,38 +564,11 @@ write_terms () {
then
die "$(gettext "please use two different terms")"
fi
-   check_term_format "$TERM_BAD" bad
-   check_term_format "$TERM_GOOD" good
+   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
+   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 
-check_term_format () {
-   term=$1
-   git check-ref-format refs/bisect/"$term" ||
-   die "$(eval_gettext "'\$term' is not a valid term")"
-   case "$term" in
-   help|start|terms|skip|next|reset|visualize|replay|log|run)
-   die "$(eval_gettext "can't use the builtin command 

Re: [PATCH v4 2/4] submodule: update core.worktree using git-config

[Stefan Beller]

On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
 wrote:
> To access a separate repository, the first step should be read its
> config file to determine if this repository layout is supported or
> not, or if we understand all repo extensions, of it is a linked
> worktree. Only then should know where to update the config file.
>
> Unfortunately, our C code base is not ready for doing all that in the
> same process. The repo detection is not meant to be used for peeking
> in other repository, and config code would read config.worktree that
> is in _current_ $GIT_DIR.
>
> For now, let's spawn a new process and let all that done separately.
>
> PS. submodule-helper also updates core.worktree. But in there, we
> create a new clone, we know what is the initial repository layout, so
> we know we can simply update "config" file without risks.

Right, the submodule--helper update_clone should not be required to convert.

>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  submodule.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index abc2ac2..b912871 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1128,7 +1128,9 @@ void connect_work_tree_and_git_dir(const char 
> *work_tree, const char *git_dir)
>  {
> struct strbuf file_name = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> +   struct strbuf path = STRBUF_INIT;
> const char *real_work_tree = xstrdup(real_path(work_tree));
> +   struct child_process cp = CHILD_PROCESS_INIT;
>
> /* Update gitfile */
> strbuf_addf(_name, "%s/.git", work_tree);
> @@ -1136,13 +1138,17 @@ void connect_work_tree_and_git_dir(const char 
> *work_tree, const char *git_dir)
>relative_path(git_dir, real_work_tree, _path));
>
> /* Update core.worktree setting */
> -   strbuf_reset(_name);
> -   strbuf_addf(_name, "%s/config", git_dir);
> -   git_config_set_in_file(file_name.buf, "core.worktree",
> -  relative_path(real_work_tree, git_dir,
> -_path));
> +   strbuf_addstr(, relative_path(real_work_tree, git_dir,
> +  _path));
> +   cp.git_cmd = 1;
> +   argv_array_pushl(, "-C", work_tree, NULL);
> +   argv_array_pushl(, "--work-tree", ".", NULL);
> +   argv_array_pushl(, "config", "core.worktree", path.buf, NULL);
> +   if (run_command() < 0)
> +   die(_("failed to update core.worktree for %s"), git_dir);

Do we need to make this conditional on the extensions.worktreeConfig
variable, though? When I just run

git config --worktree . foo bar
fatal: Per-worktree configuration requires extensions.worktreeConfig
Please read section CONFIGURATION in `git help worktree` before
enabling it.

which would trigger the failure here?

>
> strbuf_release(_name);
> +   strbuf_release();
> strbuf_release(_path);
> free((void *)real_work_tree);
>  }
> --
> 2.9.1.566.gbd532d4
>
--
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 v10 05/12] t6030: explicitly test for bisection cleanup

[Pranit Bauva]

Add test to explicitly check that 'git bisect reset' is working as
expected. This is already covered implicitly by the test suite.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 

---
I faced this problem while converting `bisect_clean_state` and the tests
where showing breakages but it wasn't clear as to where exactly are they
breaking. This will patch  will help in that. Also I tested the test
coverage of the test suite before this patch and it covers this (I did
this by purposely changing names of files in git-bisect.sh and running
the test suite).
---
 t/t6030-bisect-porcelain.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index e74662b..a17f7a6 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -894,4 +894,21 @@ test_expect_success 'bisect start takes options and revs 
in any order' '
test_cmp expected actual
 '
 
+test_expect_success 'git bisect reset cleans bisection state properly' '
+   git bisect reset &&
+   git bisect start &&
+   git bisect good $HASH1 &&
+   git bisect bad $HASH4 &&
+   git bisect reset &&
+   test -z "$(git for-each-ref "refs/bisect/*")" &&
+   test_path_is_missing "$GIT_DIR/BISECT_EXPECTED_REV" &&
+   test_path_is_missing "$GIT_DIR/BISECT_ANCESTORS_OK" &&
+   test_path_is_missing "$GIT_DIR/BISECT_LOG" &&
+   test_path_is_missing "$GIT_DIR/BISECT_RUN" &&
+   test_path_is_missing "$GIT_DIR/BISECT_TERMS" &&
+   test_path_is_missing "$GIT_DIR/head-name" &&
+   test_path_is_missing "$GIT_DIR/BISECT_HEAD" &&
+   test_path_is_missing "$GIT_DIR/BISECT_START"
+'
+
 test_done

--
https://github.com/git/git/pull/273
--
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 v10 04/12] bisect--helper: `bisect_clean_state` shell function in C

[Pranit Bauva]

Reimplement `bisect_clean_state` shell function in C and add a
`bisect-clean-state` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

Using `--bisect-clean-state` subcommand is a measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by
bisect_reset() and bisect_start().

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 55 +++-
 git-bisect.sh| 26 +++
 2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index ef87c82..ad67a97 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -3,12 +3,21 @@
 #include "parse-options.h"
 #include "bisect.h"
 #include "refs.h"
+#include "dir.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
+static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
+static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
+static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
N_("git bisect--helper --write-terms  "),
+   N_("git bisect--helper --bisect-clean-state"),
NULL
 };
 
@@ -78,11 +87,49 @@ static int write_terms(const char *bad, const char *good)
return (res < 0) ? -1 : 0;
 }
 
+static int mark_for_removal(const char *refname, const struct object_id *oid,
+   int flag, void *cb_data)
+{
+   struct string_list *refs = cb_data;
+   char *ref = xstrfmt("refs/bisect/%s", refname);
+   string_list_append(refs, ref);
+   return 0;
+}
+
+static int bisect_clean_state(void)
+{
+   int result = 0;
+
+   /* There may be some refs packed during bisection */
+   struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
+   for_each_ref_in("refs/bisect/", mark_for_removal, (void *) 
_for_removal);
+   string_list_append(_for_removal, xstrdup("BISECT_HEAD"));
+   result = delete_refs(_for_removal);
+   refs_for_removal.strdup_strings = 1;
+   string_list_clear(_for_removal, 0);
+   remove_path(git_path_bisect_expected_rev());
+   remove_path(git_path_bisect_ancestors_ok());
+   remove_path(git_path_bisect_log());
+   remove_path(git_path_bisect_names());
+   remove_path(git_path_bisect_run());
+   remove_path(git_path_bisect_terms());
+   /* Cleanup head-name if it got left by an old version of git-bisect */
+   remove_path(git_path_head_name());
+   /*
+* Cleanup BISECT_START last to support the --no-checkout option
+* introduced in the commit 4796e823a.
+*/
+   remove_path(git_path_bisect_start());
+
+   return result;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
-   WRITE_TERMS
+   WRITE_TERMS,
+   BISECT_CLEAN_STATE
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -90,6 +137,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("perform 'git bisect next'"), NEXT_ALL),
OPT_CMDMODE(0, "write-terms", ,
 N_("write the terms to .git/BISECT_TERMS"), 
WRITE_TERMS),
+   OPT_CMDMODE(0, "bisect-clean-state", ,
+N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -108,6 +157,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
if (argc != 2)
die(_("--write-terms requires two arguments"));
return write_terms(argv[0], argv[1]);
+   case BISECT_CLEAN_STATE:
+   if (argc != 0)
+   die(_("--bisect-clean-state requires no arguments"));
+   return bisect_clean_state();
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index cd39bd0..bbc57d2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -187,7 +187,7 @@ bisect_start() {
#
# Get rid of any old bisect state.
#
-   bisect_clean_state || exit
+   git bisect--helper 

Re: Complex gitweb URL

[Jakub Narębski]

On 20 July 2016 at 23:35, CLOSE Dave  wrote:
> Thanks, Jakub, for the quick response.
>
> On 07/20/16 02:20 PM, Jakub Narębski wrote:
>
>>> If I replace the hb=SHA argument with hb=HEAD, the URL still works. But
>>> I have no idea what I can use to replace the h=SHA argument.
>>
>> You can remove it.  'hb' (hash_base) and 'f' (filename) identify target
>> file in a repository unambiguously.
>
> If I do, leaving everything else present, I get "404 cannot find file".

Which gitweb version are you using? For 'a=blob' (i.e. displaying
contents of the file), if 'h' (hash) parameter is not set, then gitweb
finds it by using `git ls-tree  -- ` in
git_get_hash_by_path() subroutine. No 'hb' is equivalent to 'hb=HEAD'

What does `git ls-tree  -- ` results in your
repository?

>>> A complication is that the target file is not in the master branch.
>>> Somehow I need to be able to specify the branch. I've tried putting it
>>> as the h= argument but that results in "Reading blob failed". If I leave
>>> out the h= argument entirely, gitweb responds, "404 cannot find file".
>>
>> Did you forgot to set 'hb' parameter?  Is said file present in revision
>> given by the 'hb' parameter?
>
> The file is present in both master and develop branches. If I include
> hb=HEAD and leave out h=, gitweb doesn't find either of them.

It works for repo.or.cz (which uses gitweb as web interface). So we
have WORKSFORME situation. Now we need to find out what is
different in your case...

>   
>  Using
> hb=HEAD and h=develop or h=master results in "Reading blob failed".

'h' must be hash of the blob as an abject (of its contents at given revision).

-- 
Jakub Narebski
--
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 5/6] Add porcelain V2 documentation to status manpage

[Junio C Hamano]

Jeff Hostetler  writes:

> +A series of lines are then displayed for the tracked entries.
> +
> + R 
> [\t]
> +
> +Field   Meaning
> +
> +The staged and unstaged values described earlier, with
> +unchanged indicated by a "." rather than a space.

Ahh, this is where these mysterious xy came from.  You just needed
two random consecutive letters, and they could have been ab, ij, or
jk.  I don't like any of them ;-)

Also I have trouble with the "staged and unstaged" here, especially
the latter, as the word implies the user did "git rm --cached" on
the path earlier, which is not the case.  You are saying what is in
the index and what is in the working tree.

Perhaps using iw instead of xy to make them in sync with the way
"git diff --mnemonic-prefix" denotes the contents in the index and
in the working tree?  Together with s/staged/in the index/ and
s/unstaged/in the working tree/, the result would become more
consistent with the rest of the system, I suspect.

> +The file modes for the entry.
> +For unmerged entries, these are the stage 1, 2, and 3,
> +and the worktree modes.
> +For regular entries, these are the head, index, and
> +worktree modes; the fourth is zero.
> +  The SHA1 values for the entry.
> +For unmerged entries, these are the stage 1,2, and 3 values.
> +For regular entries, these are the head and index values;
> +the third entry is zero.

To future-proof, we should use "object name" for what you call "SHA1
value" here, I would think.

> +R   The rename percentage score.

s/percentage score/score/.  Do you differentiate between renames and
copies?

> +  The current pathname. It is C-Quoted if necessary.
> +   The original path. This is only present for staged renames.
> +It is C-Quoted if necessary.

Seeing "if necessary" makes me wonder if we want to define when it
is "necessary".
--
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 v10 11/12] bisect--helper: `bisect_next_check` shell function in C

[Pranit Bauva]

Reimplement `bisect_next_check` shell function in C and add
`bisect-next-check` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

Using `--bisect-next-check` is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.

bisect_voc() is removed as it is redundant and does not serve any useful
purpose. We are better off specifying "bad|new" "good|old" as and when
we require in bisect_next_check().

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 79 +++-
 git-bisect.sh| 60 +++-
 2 files changed, 82 insertions(+), 57 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b9119e3..001096a 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,6 +6,7 @@
 #include "dir.h"
 #include "argv-array.h"
 #include "run-command.h"
+#include "prompt.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-reset []"),
N_("git bisect--helper --bisect-write
 []"),
N_("git bisect--helper --bisect-check-and-set-terms  
 "),
+   N_("git bisect--helper --bisect-next-check []  
term_bad.buf);
+   char *good_glob = xstrfmt("%s*", terms->term_good.buf);
+
+   if (ref_exists(bad_ref))
+   missing_bad = 0;
+   free(bad_ref);
+
+   for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+(void *) _good);
+   free(good_glob);
+
+   if (!missing_good && !missing_bad)
+   return 0;
+
+   if (!current_term)
+   return -1;
+
+   if (missing_good && !missing_bad && current_term &&
+   !strcmp(current_term, terms->term_good.buf)) {
+   char *yesno;
+   /*
+* have bad (or new) but not good (or old). We could bisect
+* although this is less optimum.
+*/
+   fprintf(stderr, "Warning: bisecting only with a %s commit\n",
+   terms->term_bad.buf);
+   if (!isatty(0))
+   return 0;
+   /*
+* TRANSLATORS: Make sure to include [Y] and [n] in your
+* translation. The program will only accept English input
+* at this point.
+*/
+   yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+   if (starts_with(yesno, "N") || starts_with(yesno, "n"))
+   return -1;
+   return 0;
+   }
+   if (!is_empty_or_missing_file(git_path_bisect_start()))
+   return error(_("You need to give me at least one good|old and "
+   "bad|new revision. You can use \"git bisect "
+   "bad|new\" and \"git bisect good|old\" for "
+   "that. \n"));
+   else
+   return error(_("You need to start by \"git bisect start\". "
+   "You then need to give me at least one good|"
+   "old and bad|new revision. You can use \"git "
+   "bisect bad|new\" and \"git bisect good|old\" "
+   " for that.\n"));
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -301,7 +368,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
BISECT_RESET,
CHECK_EXPECTED_REVS,
BISECT_WRITE,
-   CHECK_AND_SET_TERMS
+   CHECK_AND_SET_TERMS,
+   BISECT_NEXT_CHECK
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -319,6 +387,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("write out the bisection state in BISECT_LOG"), 

[PATCH v10 10/12] bisect--helper: `check_and_set_terms` shell function in C

[Pranit Bauva]

Reimplement the `check_and_set_terms` shell function in C and add
`check-and-set-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--check-and-set-terms` subcommand is a temporary measure to port
shell function in C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and will be called
by some other methods.

check_and_set_terms() sets and receives two global variables namely
TERM_GOOD and TERM_BAD in the shell script. Luckily the file BISECT_TERMS
also contains the value of those variables so its appropriate to evoke the
method get_terms() after calling the subcommand so that it retrieves the
value of TERM_GOOD and TERM_BAD from the file BISECT_TERMS. The two
global variables are passed as arguments to the subcommand.

Also introduce bisect_terms_reset() to empty the contents of `term_good`
and `term_bad` of `struct bisect_terms`.

Also introduce set_terms() to copy the `term_good` and `term_bad` into
`struct bisect_terms` and write it out to the file BISECT_TERMS.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 52 +++-
 git-bisect.sh| 36 -
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d1d12f2..b9119e3 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -23,6 +23,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-clean-state"),
N_("git bisect--helper --bisect-reset []"),
N_("git bisect--helper --bisect-write
 []"),
+   N_("git bisect--helper --bisect-check-and-set-terms  
 "),
NULL
 };
 
@@ -43,6 +44,12 @@ static void bisect_terms_release(struct bisect_terms *terms)
strbuf_release(>term_bad);
 }
 
+static void bisect_terms_reset(struct bisect_terms *term)
+{
+   strbuf_reset(>term_good);
+   strbuf_reset(>term_bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -252,6 +259,39 @@ static int bisect_write(const char *state, const char *rev,
return 0;
 }
 
+static int set_terms(struct bisect_terms *terms, const char *bad,
+const char *good)
+{
+   bisect_terms_reset(terms);
+   strbuf_addstr(>term_good, good);
+   strbuf_addstr(>term_bad, bad);
+   return write_terms(terms->term_bad.buf, terms->term_good.buf);
+}
+
+static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
+{
+   int no_term_file = is_empty_or_missing_file(git_path_bisect_terms());
+
+   if (one_of(cmd, "skip", "start", "terms", NULL))
+   return 0;
+
+   if (!no_term_file &&
+   strcmp(cmd, terms->term_bad.buf) &&
+   strcmp(cmd, terms->term_good.buf))
+   return error(_("Invalid command: you're currently in a "
+   "'%s' '%s' bisect"), terms->term_bad.buf,
+   terms->term_good.buf);
+
+   if (no_term_file) {
+   if (one_of(cmd, "bad", "good", NULL))
+   return set_terms(terms, "bad", "good");
+   if (one_of(cmd, "new", "old", NULL))
+   return set_terms(terms, "new", "old");
+   }
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -260,7 +300,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
BISECT_CLEAN_STATE,
BISECT_RESET,
CHECK_EXPECTED_REVS,
-   BISECT_WRITE
+   BISECT_WRITE,
+   CHECK_AND_SET_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -276,6 +317,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_CMDMODE(0, "bisect-write", ,
 N_("write out the bisection state in BISECT_LOG"), 
BISECT_WRITE),
+   OPT_CMDMODE(0, "check-and-set-terms", ,
+N_("check and set terms in a bisection state"), 
CHECK_AND_SET_TERMS),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -319,6 +362,13 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
strbuf_addstr(_bad, argv[3]);
res = bisect_write(argv[0], argv[1], , nolog);
break;
+   case CHECK_AND_SET_TERMS:
+   if (argc != 3)
+   

[PATCH v10 12/12] bisect--helper: `get_terms` & `bisect_terms` shell function in C

[Pranit Bauva]

Reimplement the `get_terms` and `bisect_terms` shell function in C and
add `bisect-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

In the shell version, the terms were identified by strings but in C
version its done by bit manipulation and passing the integer value to
the function.

Using `--bisect-terms` subcommand is a temporary measure to port shell
function in C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 74 +++-
 git-bisect.sh| 35 ++-
 2 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 001096a..185a8ad 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -8,6 +8,13 @@
 #include "run-command.h"
 #include "prompt.h"
 
+enum terms_defined {
+   TERM_BAD = 1,
+   TERM_GOOD = 2,
+   TERM_NEW = 4,
+   TERM_OLD = 8
+};
+
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
@@ -26,6 +33,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-write
 []"),
N_("git bisect--helper --bisect-check-and-set-terms  
 "),
N_("git bisect--helper --bisect-next-check []  
term_bad, fp) == EOF ||
+ strbuf_getline(>term_good, fp) == EOF;
+
+   fclose(fp);
+   return res ? -1 : 0;
+}
+
+static int bisect_terms(struct bisect_terms *terms, int term_defined)
+{
+   if (get_terms(terms)) {
+   fprintf(stderr, "no terms defined\n");
+   return -1;
+   }
+   if (!term_defined) {
+   printf("Your current terms are %s for the old state\nand "
+  "%s for the new state.\n", terms->term_good.buf,
+  terms->term_bad.buf);
+   return 0;
+   }
+
+   if (term_defined == TERM_GOOD || term_defined == TERM_OLD)
+   printf("%s\n", terms->term_good.buf);
+   if (term_defined == TERM_BAD || term_defined == TERM_NEW)
+   printf("%s\n", terms->term_bad.buf);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -369,9 +414,11 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
CHECK_EXPECTED_REVS,
BISECT_WRITE,
CHECK_AND_SET_TERMS,
-   BISECT_NEXT_CHECK
+   BISECT_NEXT_CHECK,
+   BISECT_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0;
+   enum terms_defined term_defined = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -389,6 +436,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("check and set terms in a bisection state"), 
CHECK_AND_SET_TERMS),
OPT_CMDMODE(0, "bisect-next-check", ,
 N_("check whether bad or good terms exist"), 
BISECT_NEXT_CHECK),
+   OPT_CMDMODE(0, "bisect-terms", ,
+N_("print out the bisect terms"), BISECT_TERMS),
+   OPT_BIT(0, "term-bad", _defined,
+N_("show the bad term"), TERM_BAD),
+   OPT_BIT(0, "term-good", _defined,
+N_("show the good term"), TERM_GOOD),
+   OPT_BIT(0, "term-new", _defined,
+N_("show the new term"), TERM_NEW),
+   OPT_BIT(0, "term-old", _defined,
+N_("show the old term"), TERM_OLD),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -399,6 +456,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
argc = parse_options(argc, argv, prefix, options,
 git_bisect_helper_usage, 0);
 
+   if (cmdmode 

[PATCH v10 01/12] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Pranit Bauva]

`--next-all` is meant to be used as a subcommand to support multiple
"operation mode" though the current implementation does not contain any
other subcommand along side with `--next-all` but further commits will
include some more subcommands.

Helped-by: Johannes Schindelin 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3324229..8111c91 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -10,11 +10,11 @@ static const char * const git_bisect_helper_usage[] = {
 
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-   int next_all = 0;
+   enum { NEXT_ALL = 1 } cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
-   OPT_BOOL(0, "next-all", _all,
-N_("perform 'git bisect next'")),
+   OPT_CMDMODE(0, "next-all", ,
+N_("perform 'git bisect next'"), NEXT_ALL),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -23,9 +23,14 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
argc = parse_options(argc, argv, prefix, options,
 git_bisect_helper_usage, 0);
 
-   if (!next_all)
+   if (!cmdmode)
usage_with_options(git_bisect_helper_usage, options);
 
-   /* next-all */
-   return bisect_next_all(prefix, no_checkout);
+   switch (cmdmode) {
+   case NEXT_ALL:
+   return bisect_next_all(prefix, no_checkout);
+   default:
+   die("BUG: unknown subcommand '%d'", cmdmode);
+   }
+   return 0;
 }

--
https://github.com/git/git/pull/273
--
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 v10 06/12] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()

[Pranit Bauva]

is_empty_file() can help to refactor a lot of code. This will be very
helpful in porting "git bisect" to C.

Suggested-by: Torsten Bögershausen 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/am.c | 20 ++--
 cache.h  |  3 +++
 wrapper.c| 13 +
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..6ee158f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -30,22 +30,6 @@
 #include "mailinfo.h"
 
 /**
- * Returns 1 if the file is empty or does not exist, 0 otherwise.
- */
-static int is_empty_file(const char *filename)
-{
-   struct stat st;
-
-   if (stat(filename, ) < 0) {
-   if (errno == ENOENT)
-   return 1;
-   die_errno(_("could not stat %s"), filename);
-   }
-
-   return !st.st_size;
-}
-
-/**
  * Returns the length of the first line of msg.
  */
 static int linelen(const char *msg)
@@ -1323,7 +1307,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
goto finish;
}
 
-   if (is_empty_file(am_path(state, "patch"))) {
+   if (is_empty_or_missing_file(am_path(state, "patch"))) {
printf_ln(_("Patch is empty. Was it split wrong?"));
die_user_resolve(state);
}
@@ -1911,7 +1895,7 @@ static void am_run(struct am_state *state, int resume)
resume = 0;
}
 
-   if (!is_empty_file(am_path(state, "rewritten"))) {
+   if (!is_empty_or_missing_file(am_path(state, "rewritten"))) {
assert(state->rebasing);
copy_notes_for_rebase(state);
run_post_rewrite_hook(state);
diff --git a/cache.h b/cache.h
index 6049f86..91e2f81 100644
--- a/cache.h
+++ b/cache.h
@@ -1870,4 +1870,7 @@ void sleep_millisec(int millisec);
  */
 void safe_create_dir(const char *dir, int share);
 
+/* Return 1 if the file is empty or does not exists, 0 otherwise. */
+extern int is_empty_or_missing_file(const char *filename);
+
 #endif /* CACHE_H */
diff --git a/wrapper.c b/wrapper.c
index 5dc4e15..e70e4d1 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -696,3 +696,16 @@ void sleep_millisec(int millisec)
 {
poll(NULL, 0, millisec);
 }
+
+int is_empty_or_missing_file(const char *filename)
+{
+   struct stat st;
+
+   if (stat(filename, ) < 0) {
+   if (errno == ENOENT)
+   return 1;
+   die_errno(_("could not stat %s"), filename);
+   }
+
+   return !st.st_size;
+}

--
https://github.com/git/git/pull/273
--
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: Complex gitweb URL

[CLOSE Dave]

On 07/20/16 02:20 PM, Jakub Narębski wrote:

> See e.g. http://repo.or.cz/?p=git/zerocommit.git;a=tree;hb=master;f=t

Ah! You have the branch in the hb= parameter whereas I had HEAD in that 
one. Since HEAD is the default, I suppose I don't really need it.

Indeed, if I use a URL like 
"http://site/gitweb/?p=repo;a=blob;f=file;hb=branch;, I get my file.

Thanks for the clue.
-- 
Dave Close

Re: Complex gitweb URL

[CLOSE Dave]

Thanks, Jakub, for the quick response.

On 07/20/16 02:20 PM, Jakub Narębski wrote:

>> If I replace the hb=SHA argument with hb=HEAD, the URL still works. But
>> I have no idea what I can use to replace the h=SHA argument.
>
> You can remove it.  'hb' (hash_base) and 'f' (filename) identify target
> file in a repository unambiguously.

If I do, leaving everything else present, I get "404 cannot find file".

>> A complication is that the target file is not in the master branch.
>> Somehow I need to be able to specify the branch. I've tried putting it
>> as the h= argument but that results in "Reading blob failed". If I leave
>> out the h= argument entirely, gitweb responds, "404 cannot find file".
>
> Did you forgot to set 'hb' parameter?  Is said file present in revision
> given by the 'hb' parameter?

The file is present in both master and develop branches. If I include 
hb=HEAD and leave out h=, gitweb doesn't find either of them. Using 
hb=HEAD and h=develop or h=master results in "Reading blob failed".
-- 
Dave Close

Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2

[Junio C Hamano]

Jeff Hostetler  writes:

> diff --git a/wt-status.c b/wt-status.c
> index c19b52c..2d4829e 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -406,6 +406,89 @@ static void wt_status_print_change_data(struct wt_status 
> *s,
>   strbuf_release();
>  }
>  
> +
> +/* Copy info for both sides of a head-vs-index change
> + * into the Porcelain V2 data.
> + */

/* 
 * Please don't use unbalanced/asymmetric comment.
 * Our multi-line comment blocks look more like
 * this.
 */

> +static void porcelain_v2_updated_entry(
> + struct wt_status_change_data *d,
> + struct diff_filepair *p)
> +{
> + switch (p->status) {
> + case DIFF_STATUS_ADDED:
> + d->porcelain_v2.mode_index = p->two->mode;
> + hashcpy(d->porcelain_v2.sha1_index, p->two->sha1);
> + break;
> +
> + case DIFF_STATUS_DELETED:
> + d->porcelain_v2.mode_head = p->one->mode;
> + hashcpy(d->porcelain_v2.sha1_head, p->one->sha1);
> + break;
> +
> + case DIFF_STATUS_RENAMED:
> + d->porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;
> + case DIFF_STATUS_COPIED:
> + case DIFF_STATUS_MODIFIED:
> + case DIFF_STATUS_TYPE_CHANGED:
> + case DIFF_STATUS_UNMERGED:
> + d->porcelain_v2.mode_head = p->one->mode;
> + d->porcelain_v2.mode_index = p->two->mode;
> + hashcpy(d->porcelain_v2.sha1_head, p->one->sha1);
> + hashcpy(d->porcelain_v2.sha1_index, p->two->sha1);
> + break;
> +
> + case DIFF_STATUS_UNKNOWN:
> + /* This should never happen. */
> + break;
> + }

You may want to have

die("BUG: status unknown???");

if you know "This should never happen.".

The code seems to assume that d->porcelain_v2.* fields are
initialized earlier in the callchain to reasonable values
(e.g. STATUS_ADDED case does not clear .mode_head to "missing"); I
am not sure if that is easier to read or fill in all the values
explicitly in this function.

> +}
> +
> +/* Copy info for both sides of an index-vs-worktree change
> + * into the very verbose porcelain data.
> + */
> +static void porcelain_v2_changed_entry(
> + struct wt_status_change_data *d,
> + const struct diff_filepair *p)
> +{
> + switch (p->status) {
> + case DIFF_STATUS_ADDED:
> + d->porcelain_v2.mode_worktree = p->two->mode;
> + /* don't bother with worktree sha, since it is almost always 
> zero. */
> + break;
> +
> + case DIFF_STATUS_DELETED:
> + d->porcelain_v2.mode_index = p->one->mode;
> + hashcpy(d->porcelain_v2.sha1_index, p->one->sha1);
> + break;
> +
> + case DIFF_STATUS_COPIED:
> + case DIFF_STATUS_MODIFIED:
> + case DIFF_STATUS_RENAMED:

Can RENAMED or COPIED happen here?

> + case DIFF_STATUS_TYPE_CHANGED:
> + d->porcelain_v2.mode_index = p->one->mode;
> + d->porcelain_v2.mode_worktree = p->two->mode;
> + hashcpy(d->porcelain_v2.sha1_index, p->one->sha1);
> + /* don't bother with worktree sha, since it is almost always 
> zero. */
> + break;
> +
> + case DIFF_STATUS_UNKNOWN:
> + /* This should never happen. */
> + break;
> +
> + case DIFF_STATUS_UNMERGED:
> + /* This should never happen. */
> + break;
> + }
> +}

Other than that, the same comments as the ones for _updated_ apply
to this _changed_ function.

> +static void porcelain_v2_added_initial_entry(
> + struct wt_status_change_data *d,
> + const struct cache_entry *ce)
> +{
> + d->porcelain_v2.mode_index = ce->ce_mode;
> + hashcpy(d->porcelain_v2.sha1_index, ce->sha1);
> +}
> +
>  static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
>struct diff_options *options,
>void *data)
> @@ -433,6 +516,9 @@ static void wt_status_collect_changed_cb(struct 
> diff_queue_struct *q,
>   d->dirty_submodule = p->two->dirty_submodule;
>   if (S_ISGITLINK(p->two->mode))
>   d->new_submodule_commits = !!hashcmp(p->one->sha1, 
> p->two->sha1);
> +
> + if (s->status_format == STATUS_FORMAT_PORCELAIN_V2)
> + porcelain_v2_changed_entry(d, p);
>   }
>  }
>  
> @@ -486,6 +572,9 @@ static void wt_status_collect_updated_cb(struct 
> diff_queue_struct *q,
>   d->stagemask = unmerged_mask(p->two->path);
>   break;
>   }
> +
> + if (s->status_format == STATUS_FORMAT_PORCELAIN_V2)
> + porcelain_v2_updated_entry(d, p);
>   }
>  }
> @@ -565,8 +654,12 @@ static void wt_status_collect_changes_initial(struct 
> wt_status *s)
>   d->index_status = DIFF_STATUS_UNMERGED;
>  

Re: Complex gitweb URL

[Jakub Narębski]

W dniu 2016-07-20 o 21:24, CLOSE Dave pisze:
> I'm trying to create a URL that will always refer to the latest version 
> of a file stored under Gerrit. gitweb access is available. The man page 
> specification doesn't seem to work for me. Instead, I seem to need to 
> put most of the information into arguments (after the '?').
> 
> For example, the repo name includes several directories, so it doesn't 
> work to put it into the 
> ".../gitweb.cgi///:/?" format. 
> Or, at least, I don't see how.

The fact that gitweb path_info-based URL uses /... instead
of / is in my opinion a bit of an unfortunate design error,
kept because of backward compatibility.

That said, gitweb can detect where the  part ends and 
part begins, even for hierarchical multi-part repository name. See e.g.
http://repo.or.cz/git/zerocommit.git/tree/HEAD:/Documentation with
repository part being 'git/zerocommit.git' (a "fork" of git.git). So it
should work.

There are some cases however where the URL cannot be represented in
path_info form, and some parameters must be put as query arguments,
i.e. after '?'.

> 
> Instead I'm trying to use a URL in the format, 
> "http://site/gitweb/?args;. If I use gitweb itself to navigate to my 
> target file, I see a URL in this format that contains several arguments, 
> "p=repo;a=blob;f=file;h=SHA;hb=SHA". If I use that URL directly, I get 
> my file. But those SHA values are not something I know how to determine 
> in advance. And I suspect they are unique to the specific version of the 
> file accessed, not always the latest as I want.
> 
> If I replace the hb=SHA argument with hb=HEAD, the URL still works. But 
> I have no idea what I can use to replace the h=SHA argument.

You can remove it.  'hb' (hash_base) and 'f' (filename) identify target
file in a repository unambiguously.

> 
> A complication is that the target file is not in the master branch. 
> Somehow I need to be able to specify the branch. I've tried putting it 
> as the h= argument but that results in "Reading blob failed". If I leave 
> out the h= argument entirely, gitweb responds, "404 cannot find file".

Did you forgot to set 'hb' parameter?  Is said file present in revision
given by the 'hb' parameter?

See e.g. http://repo.or.cz/?p=git/zerocommit.git;a=tree;hb=master;f=t
 
> Are these arguments documented somewhere? What is the recommended way to 
> construct a URL like I need?

Well, they are not described in the documentation, but you can check
the code and its comments.

Best,
-- 
Jakub Narębski


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

[PATCH v4 1/8] doc: use 'symmetric difference' consistently

[Philip Oakley]

Signed-off-by: Philip Oakley 
---
 Documentation/gitk.txt | 2 +-
 Documentation/rev-list-options.txt | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 6ade002..6c3eb15 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -70,7 +70,7 @@ linkgit:git-rev-list[1] for a complete list.
 
 --left-right::
 
-   Mark which side of a symmetric diff a commit is reachable
+   Mark which side of a symmetric difference a commit is reachable
from.  Commits from the left side are prefixed with a `<`
symbol and those from the right with a `>` symbol.
 
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4f009d4..6dc0bb0 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -225,7 +225,7 @@ excluded from the output.
 
 --left-only::
 --right-only::
-   List only commits on the respective side of a symmetric range,
+   List only commits on the respective side of a symmetric difference,
i.e. only those which would be marked `<` resp. `>` by
`--left-right`.
 +
@@ -766,7 +766,7 @@ ifdef::git-rev-list[]
 endif::git-rev-list[]
 
 --left-right::
-   Mark which side of a symmetric diff a commit is reachable from.
+   Mark which side of a symmetric difference a commit is reachable from.
Commits from the left side are prefixed with `<` and those from
the right with `>`.  If combined with `--boundary`, those
commits are prefixed with `-`.
-- 
2.9.0.windows.1

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

[PATCH v4 5/8] doc: gitrevisions - use 'reachable' in page description

[Philip Oakley]

Signed-off-by: Philip Oakley 
---
 Documentation/gitrevisions.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
index e903eb7..33039c6 100644
--- a/Documentation/gitrevisions.txt
+++ b/Documentation/gitrevisions.txt
@@ -15,8 +15,8 @@ DESCRIPTION
 
 Many Git commands take revision parameters as arguments. Depending on
 the command, they denote a specific commit or, for commands which
-walk the revision graph (such as linkgit:git-log[1]), all commits which can
-be reached from that commit. In the latter case one can also specify a
+walk the revision graph (such as linkgit:git-log[1]), all commits which are
+reachable from that commit. In the latter case one can also specify a
 range of revisions explicitly.
 
 In addition, some Git commands (such as linkgit:git-show[1]) also take
-- 
2.9.0.windows.1

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

[PATCH v4 4/8] doc: give headings for the two and three dot notations

[Philip Oakley]

While there, also break out the other shorthand notations and
add a title for the revision range summary (which also appears
in git-rev-parse, so keep it mixed case).

Signed-off-by: Philip Oakley 
---
 Documentation/revisions.txt | 58 -
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 6e9cd41..5b37283 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,49 @@ specifying a single revision with the notation described 
in the
 previous section means the set of commits reachable from that
 commit, following the commit ancestry chain.
 
-To exclude commits reachable from a commit, a prefix '{caret}'
-notation is used.  E.g. '{caret}r1 r2' means commits reachable
-from 'r2' but exclude the ones reachable from 'r1'.
-
-This set operation appears so often that there is a shorthand
-for it.  When you have two commits 'r1' and 'r2' (named according
-to the syntax explained in SPECIFYING REVISIONS above), you can ask
-for commits that are reachable from r2 excluding those that are reachable
-from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
-
-A similar notation 'r1\...r2' is called symmetric difference
-of 'r1' and 'r2' and is defined as
-'r1 r2 --not $(git merge-base --all r1 r2)'.
-It is the set of commits that are reachable from either one of
-'r1' (left side) or 'r2' (right side) but not from both.
-
-In these two shorthands, you can omit one end and let it default to HEAD.
+Commit Exclusions
+~
+
+'{caret}' (caret) Notation::
+ To exclude commits reachable from a commit, a prefix '{caret}'
+ notation is used.  E.g. '{caret}r1 r2' means commits reachable
+ from 'r2' but exclude the ones reachable from 'r1'.
+
+Dotted Range Notations
+~~
+
+The '..' (two-dot) Range Notation::
+ The '{caret}r1 r2' set operation appears so often that there is a shorthand
+ for it.  When you have two commits 'r1' and 'r2' (named according
+ to the syntax explained in SPECIFYING REVISIONS above), you can ask
+ for commits that are reachable from r2 excluding those that are reachable
+ from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
+
+The '...' (three dot) Symmetric Difference Notation::
+ A similar notation 'r1\...r2' is called symmetric difference
+ of 'r1' and 'r2' and is defined as
+ 'r1 r2 --not $(git merge-base --all r1 r2)'.
+ It is the set of commits that are reachable from either one of
+ 'r1' (left side) or 'r2' (right side) but not from both.
+
+In these two shorthand notations, you can omit one end and let it default to 
HEAD.
 For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
 did I do since I forked from the origin branch?"  Similarly, '..origin'
 is a shorthand for 'HEAD..origin' and asks "What did the origin do since
 I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
 empty range that is both reachable and unreachable from HEAD.
 
-Two other shorthands for naming a set that is formed by a commit
-and its parent commits exist.  The 'r1{caret}@' notation means all
-parents of 'r1'.  'r1{caret}!' includes commit 'r1' but excludes
-all of its parents.
+Special '{caret}' Shorthand Notations
+~~
+Two other shorthands exist, particularly useful for merge commits, is
+for naming a set that is formed by a commit and its parent commits.
 
-To summarize:
+The 'r1{caret}@' notation means all parents of 'r1'.
+
+'r1{caret}!' includes commit 'r1' but excludes all of its parents.
+
+Revision Range Summary
+--
 
 ''::
Include commits that are reachable from (i.e. ancestors of)
-- 
2.9.0.windows.1

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

[PATCH v4 7/8] doc: revisions - define `reachable`

[Philip Oakley]

Do not self-define `reachable`, which can lead to misunderstanding.
Instead define `reachability` explictly.

Signed-off-by: Philip Oakley 
---
 Documentation/revisions.txt | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 5b37283..d3b7cee 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -237,10 +237,16 @@ SPECIFYING RANGES
 -
 
 History traversing commands such as `git log` operate on a set
-of commits, not just a single commit.  To these commands,
-specifying a single revision with the notation described in the
-previous section means the set of commits reachable from that
-commit, following the commit ancestry chain.
+of commits, not just a single commit.
+
+For these commands,
+specifying a single revision, using the notation described in the
+previous section, means the set of commits `reachable` from the given
+commit.
+
+A commit's reachable set is the commit itself and the commits in
+its ancestry chain.
+
 
 Commit Exclusions
 ~
-- 
2.9.0.windows.1

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

[PATCH v4 6/8] doc: gitrevisions - clarify 'latter case' is revision walk

[Philip Oakley]

The prior sentence has too many clauses for easy parsing.
Replace 'the latter case' with a direct quote.

Signed-off-by: Philip Oakley 
---
 Documentation/gitrevisions.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
index 33039c6..27dec5b 100644
--- a/Documentation/gitrevisions.txt
+++ b/Documentation/gitrevisions.txt
@@ -16,8 +16,8 @@ DESCRIPTION
 Many Git commands take revision parameters as arguments. Depending on
 the command, they denote a specific commit or, for commands which
 walk the revision graph (such as linkgit:git-log[1]), all commits which are
-reachable from that commit. In the latter case one can also specify a
-range of revisions explicitly.
+reachable from that commit. For commands that walk the revision graph one can
+also specify a range of revisions explicitly.
 
 In addition, some Git commands (such as linkgit:git-show[1]) also take
 revision parameters which denote other objects than commits, e.g. blobs
-- 
2.9.0.windows.1

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

[PATCH v4 8/8] doc: revisions - clarify reachability examples

[Philip Oakley]

 For the r1..r2 case, the exclusion of r1, rather than inclusion of r2,
 would be the unexpected case in natural language for a simple linear
 development, i.e. start..end excludes start.

Signed-off-by: Philip Oakley 
---
 Documentation/revisions.txt | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d3b7cee..14622cc 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -254,7 +254,8 @@ Commit Exclusions
 '{caret}' (caret) Notation::
  To exclude commits reachable from a commit, a prefix '{caret}'
  notation is used.  E.g. '{caret}r1 r2' means commits reachable
- from 'r2' but exclude the ones reachable from 'r1'.
+ from 'r2' but exclude the ones reachable from 'r1' (i.e. 'r1' and
+ its ancestors).
 
 Dotted Range Notations
 ~~
@@ -293,12 +294,12 @@ Revision Range Summary
 --
 
 ''::
-   Include commits that are reachable from (i.e. ancestors of)
-   .
+   Include commits that are reachable from  (i.e.  and its
+   ancestors).
 
 '{caret}'::
-   Exclude commits that are reachable from (i.e. ancestors of)
-   .
+   Exclude commits that are reachable from  (i.e.  and its
+   ancestors).
 
 '..'::
Include commits that are reachable from  but exclude
-- 
2.9.0.windows.1

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

[PATCH v4 3/8] doc: show the actual left, right, and boundary marks

[Philip Oakley]

Signed-off-by: Philip Oakley 
---
Found while checking the 'symmetric difference' documentation
---
 Documentation/pretty-formats.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 29b19b9..10719e1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -166,7 +166,7 @@ endif::git-rev-list[]
   respecting the `auto` settings of the former if we are going to a
   terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
   on the next placeholders until the color is switched again.
-- '%m': left, right or boundary mark
+- '%m': left (`<`), right (`>`) or boundary (`-`) mark
 - '%n': newline
 - '%%': a raw '%'
 - '%x00': print a byte from a hex code
-- 
2.9.0.windows.1

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

[PATCH v4 2/8] doc: revisions - name the left and right sides

[Philip Oakley]

The terms left and right side originate from the symmetric
difference. Name them there.

Signed-off-by: Philip Oakley 
---
 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 19314e3..6e9cd41 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -256,7 +256,7 @@ A similar notation 'r1\...r2' is called symmetric difference
 of 'r1' and 'r2' and is defined as
 'r1 r2 --not $(git merge-base --all r1 r2)'.
 It is the set of commits that are reachable from either one of
-'r1' or 'r2' but not from both.
+'r1' (left side) or 'r2' (right side) but not from both.
 
 In these two shorthands, you can omit one end and let it default to HEAD.
 For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
-- 
2.9.0.windows.1

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

[PATCH v4 0/8] Name for A..B ranges?

[Philip Oakley]

This is the V4 re-roll of the po/range-doc (2016-07-01) 3 commits and its
follow on patch.

Capitalisation has been fixed.
Heading levels for man pages has been fixed.
Quoting of the caret has been fixed.
The extra width wise 'caret' shorthands now mention their applicability to 
merge commits.

No change in the number of patches. Interdiff below.

The patches carefully tease out the clarification of
reachability. Reachability is defined relative the ancestry chain thus
(hopefully) avoiding misunderstandings.

The final patch updates the summary examples, and the tricky (for the
untutored reader) two dots case of a linear development where r1..r2
excludes r1 itself.

The patches can be squashed together if required.

Original discussion starts at: $gmane/297908
V1 patch series $gmane/298223
V2 patch series $gmane/298689
V3 patch series $gmane/299293


Philip Oakley (8):
  doc: use 'symmetric difference' consistently
  doc: revisions - name the left and right sides
  doc: show the actual left, right, and boundary marks
  doc: give headings for the two and three dot notations
  doc: gitrevisions - use 'reachable' in page description
  doc: gitrevisions - clarify 'latter case' is revision walk
  doc: revisions  - define `reachable`
  doc: revisions - clarify reachability examples

 Documentation/gitk.txt |  2 +-
 Documentation/gitrevisions.txt |  6 +--
 Documentation/pretty-formats.txt   |  2 +-
 Documentation/rev-list-options.txt |  4 +-
 Documentation/revisions.txt| 83 --
 5 files changed, 59 insertions(+), 38 deletions(-)

 interdiff:
 
 diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index dba4fc6..14622cc 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -241,35 +241,38 @@ of commits, not just a single commit.
 
 For these commands,
 specifying a single revision, using the notation described in the
-previous section, means the `reachable` set of commits of the given
+previous section, means the set of commits `reachable` from the given
 commit.
 
-A commit's reachable set is the commit itself and the commits of
+A commit's reachable set is the commit itself and the commits in
 its ancestry chain.
 
 
-The '{caret}' (caret) notation
-~~~
-To exclude commits reachable from a commit, a prefix '{caret}'
-notation is used.  E.g. '{caret}r1 r2' means commits reachable
-from 'r2' but exclude those reachable from 'r1' (i.e. 'r1' and its
-ancestors).
-
-The '..' (two-dot) range notation
-~
-The '{caret}r1 r2' set operation appears so often that there is a shorthand
-for it.  When you have two commits 'r1' and 'r2' (named according
-to the syntax explained in SPECIFYING REVISIONS above), you can ask
-for commits that are reachable from r2 excluding those that are reachable
-from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
-
-The '...' (three dot) Symmetric Difference notation
-~~~
-A similar notation 'r1\...r2' is called symmetric difference
-of 'r1' and 'r2' and is defined as
-'r1 r2 --not $(git merge-base --all r1 r2)'.
-It is the set of commits that are reachable from either one of
-'r1' (Left side) or 'r2' (Right side) but not from both.
+Commit Exclusions
+~
+
+'{caret}' (caret) Notation::
+ To exclude commits reachable from a commit, a prefix '{caret}'
+ notation is used.  E.g. '{caret}r1 r2' means commits reachable
+ from 'r2' but exclude the ones reachable from 'r1' (i.e. 'r1' and
+ its ancestors).
+
+Dotted Range Notations
+~~
+
+The '..' (two-dot) Range Notation::
+ The '{caret}r1 r2' set operation appears so often that there is a shorthand
+ for it.  When you have two commits 'r1' and 'r2' (named according
+ to the syntax explained in SPECIFYING REVISIONS above), you can ask
+ for commits that are reachable from r2 excluding those that are reachable
+ from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
+
+The '...' (three dot) Symmetric Difference Notation::
+ A similar notation 'r1\...r2' is called symmetric difference
+ of 'r1' and 'r2' and is defined as
+ 'r1 r2 --not $(git merge-base --all r1 r2)'.
+ It is the set of commits that are reachable from either one of
+ 'r1' (left side) or 'r2' (right side) but not from both.
 
 In these two shorthand notations, you can omit one end and let it default to 
HEAD.
 For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
@@ -278,10 +281,10 @@ is a shorthand for 'HEAD..origin' and asks "What did the 
origin do since
 I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
 empty range that is both reachable and unreachable from HEAD.
 
-Additional '{caret}' Shorthand notations
-
-Two other shorthands for naming a set that is formed by a commit
-and its parent commits exist.
+Special '{caret}' Shorthand Notations

Re: Fast-forward able commit, otherwise fail

[Philip Oakley]

From: "Stefan Haller" 

Junio C Hamano  wrote:


Another thing to consider is that the proposed workflow would not
scale if your team becomes larger.  Requiring each and every commit
on the trunk to be a merge commit, whose second parent (i.e. the tip
of the feature branch) fast-forwards to the first parent of the
merge (i.e. you require the feature to be up-to-date), would mean
that Alice and Bob collaborating on this project would end up
working like this:

 A:git pull --ff-only origin ;# starts working
 A:git checkout -b topic-a
 A:git commit; git commit; git commit
 B:git pull --ff-only origin ;# starts working
 B:git checkout -b topic-b
 B:git commit; git commit

 A:git checkout master && git merge --ff-only --no-ff topic-a
 A:git push origin ;# happy

 B:git checkout master && git merge --ff-only --no-ff topic-b
 B:git push origin ;# fails!

 B:git fetch origin ;# starts recovering
 B:git reset --hard origin/master
 B:git merge --ff-only --no-ff topic-b ;# fails!
 B:git rebase origin/master topic-b
 B:git checkout master && git merge --ff-only --no-ff topic-b
 B:git push origin ;# hopefully nobody pushed in the meantime

The first push by Bob fails because his 'master', even though it is
a merge between the once-at-the-tip-of-public-master and topic-b
which was forked from that once-at-the-tip, it no longer fast-forwards
because Alice pushed her changes to the upstream.

And it is not sufficient to redo the previous merge after fetching
the updated upstream, because your additional "feature branch must
be up-to-date" requirement is now broken for topic-b.  Bob needs to
rebuild it on top of the latest, which includes what Alice pushed,
using rebase, do that merge again, and hope that nobody else pushed
to update the upstream in the meantime.  As you have more people
working simultanously on more features, Bob will have to spend more
time doing steps between "starts recovering" and "hopefully nobody
pushed in the meantime", because the probability is higher that
somebody other than Alice has pushed while Bob is trying to recover.

The time spend on recovery is not particularly productive, and this
workflow gives him a (wrong) incentive to do that recovery procedure
quickly to beat the other participants to become the first to push.


I have read and re-read this thread a hundred times now, but I still
don't understand why it makes much of a difference whether or not Bob
rebases his branch onto master before pushing his merge. In both cases,
Alice and Bob have this race as to whose push succeeds, and in both
cases you end up with merge commits on master that are not well tested.


I'd like to put in comment from a different perspective.

You didn't say how often you expected these merge commits to be made 
(daily/hourly/2-minutes), and also didn't mention how long it's taking to do 
the tests - 5-mins/one hour/3 hours/all day, and in particular the values 
for the difficult cases.


For example it maybe: team size means typical small feature merge commits 
are ~twice/day, needing only 30 mins test time, however there are difficult 
(big feature) commits are every 1-3 weeks, and take all day (or more) to 
test, leaving an open window for small feature merges to sneak in and break 
the 'clean-test' process.


In the above case you have a management issue, where someone needs to 
(actively) manage the process so that there are no commits to master during 
that window, and rather they are put to 'next' (using Git' branch 
convention) and once the big feature has landed the 'next-features' can be 
rebased & tested and added on top of master.


Should the case be more that there is just some occasional overlaps from the 
30mins test, during the two per day window, then that should be more 
manageable by social and procedural convention.


The thing to notice is that the upstream is always good and well formed 
(because it rejects, or should reject, commits that 
aren't --ff-only --no-ff). So before any merge (either way) [i.e. just after 
rebase & test] master should be pulled to be locally up to date.


If there were no extra commits on top, then a quick --ff-only --no-ff merge 
and push, taking only a few seconds, will complete the feature. Otherwise, 
if there were extra commits on top, then it's a moderately quick rebase & 
re-test (an annoyance but handleable because its infrequent) should mean the 
updated commit will now pass the 'pull master' check cleanly - remember the 
commit/push rate means it's unlikely that a second clash happens, otherwise 
its back to the 'management' approach.





First of all, let me say that at my company we do use the workflow that
David suggests; we rebase topic branches onto master before merging them
(with --no-ff), and we like the resulting shape of the history. Even the
more experienced git users like it for its simplicity; it simply saves
us time and mental 

Re: [PATCH v1 0/6] Porcelain Status V2

[Jeff King]

On Wed, Jul 20, 2016 at 03:27:45PM -0400, Jeff Hostetler wrote:

> > A totally reasonable response is "haha no. Please stop moving the
> > goalposts". I just wanted to throw it out there as an option (and in
> > case you are interested, to let you think about it before any more work
> > goes into this direction).
> 
> haha no :-)
> 
> Short term, I'd rather nail down what I have now (both content-wise
> and format-wise) and see how we like it.  And have a follow-up task
> to look at the --state header we spoke of earlier.  And save the JSON
> version as an independent task for later.
> 
> I understand the motivation for a JSON option (and have thought
> about it before) but I think it ought to be kept separate.
> At a higher-level, it seems like a JSON option would be an
> opportunity to start a project-wide conversation about formats,
> consistency, plumbing, and etc.  A top-down conversation if you
> will about which commands will/won't get enhanced, legacy cruft
> that would not need to be converted, JSON style and naming and
> consistency issues, current best practices in the node/whatever
> community, and etc.  I could be wrong, but this feels like a
> top-down feature conversation in a wider audience.

I agree with everything you've said here.

If we add JSON, we'd want to do it everywhere: lists of commits, lists
of refs, status output, etc. I mentioned that somebody had asked me
about it recently; they are working on a git client and finding that
libgit2 is not serving their needs well, so they'd like to shell out to
git more, and wanted to have a standard way to get the data back in.

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

Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2

[Jeff King]

On Wed, Jul 20, 2016 at 02:20:24PM -0400, Jeff Hostetler wrote:

> > IIRC, it happens when HEAD points to a broken ref. So something like:
> > 
> >git init
> >echo broken >.git/refs/heads/master
> > 
> > would cause resolving HEAD to return NULL.
> 
> That worked and I see "(unknown)".
> 
> This is a bit of a nit, but is there a value we'd like
> to see there, such as "(unknown)" or "(broken)" or "(missing)"
> in that case?  (And make it clear that this is a different
> case from "(detached)".)
> 
> I'm thinking it would be nicer to always have a field
> there for parsing.

My gut feeling is to err on the side of being vague, like "unknown".
This is something that _shouldn't_ ever happen, and if it does, it could
be a broken on-disk ref, a transient syscall error, or some other
weirdness. I don't think we need to get too specific in this context
(we'll likely have said something else useful on stderr already, I
think).

-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: [ANNOUNCE] GitRev News edition 17

[Christian Couder]

On Wed, Jul 20, 2016 at 10:40 PM, Junio C Hamano  wrote:
> On Wed, Jul 20, 2016 at 1:38 PM, Christian Couder
>  wrote:
>>> Micronit. 2.9.1 (or 2.9.2) would not be a "major release". 2.9.0 was,
>>> and x.y.z (z > 0) are "maintenance releases".
>>
>> Yeah, I wondered about that when I saw :
>>
>> https://github.com/git/git.github.io/commit/d2eb8fbbb30594d19fcda731c309ad03229dc5d5
>>
>> but forgot to ask or do something about it...
>
> As long as it gets fixed quickly, no harm is done.

Fixed with:

https://github.com/git/git.github.io/commit/5d50fd0d807d0f8046b712aac1492c30aea7a652

While at it, a maintainance/maintenance mistake is also fixed in:

https://github.com/git/git.github.io/commit/ae08ca7689b4bc0c55b87edd27641fbe721e3220

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

Re: git-prompt.sh incompatible with non-basic global grep.patternType

[Jeff King]

On Wed, Jul 20, 2016 at 01:10:42PM -0700, Junio C Hamano wrote:

> This may fix it.  I think the root cause is that logic to smear
> "pattern type" into various broken-down fields in grep_opt for the
> short-hands like --basic-regexp option needs to leave "I am setting
> this short-hand" mark to allow the grep_commit_pattern_type() that
> is done as the final step of the set-up sequence before we call
> compile_grep_patterns() can take notice.  The calls currently made
> to grep_set_pattern_type_option() when we parse "--basic-regexp" and
> friends forgets to override the "source of truth" field and only
> updates the broken-down fields.
> 
> An alternative may be to update places that parse "--basic-regexp"
> and friends to just write to .pattern_type_option without calling
> grep_set_pattern_type_option(); that might be a cleaner, but I am
> not feeling well today so I won't be able to do a deeper analysis
> right now.

I gave a very cursory look when I wrote the other email, and your
alternative solution is what looked like the most elegant fix to me.

I suspect this bug has been there quite a while, so no rush. :)

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

Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2

[Junio C Hamano]

Jeff Hostetler  writes:

Just to avoid later headaches...  please look at your commit titles
and imagine how they will look when listed among 400+ other changes
when they are included in a future release in "git shortlog" output.

> Subject: Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2

Subject: status: per-file output for --porcelain=v2

or something like that, perhaps?

> This commit sets up version 2 porcelain status and
> defines the format of detail lines.  This includes
> the usual XY and pathname fields.  It adds the various
> file modes and SHAs and the rename score.  For regular
> entries these values reflect the head, index and
> worktree. For unmerged entries these values reflect
> the stage 1, 2, and 3 values.

Also, we usually do not say "This commit does this and that".

See Documentation/SubmittingPatches for more details regarding the
above two points (and more).

--
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 1/6] Allow --porcelain[=] in status and commit commands

[Junio C Hamano]

Jeff Hostetler  writes:

> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index b0a294d..0791573 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -104,7 +104,7 @@ OPTIONS
>  --branch::
>   Show the branch and tracking info even in short-format.
>  
> ---porcelain::
> +--porcelain[=]::
>   When doing a dry-run, give the output in a porcelain-ready
>   format. See linkgit:git-status[1] for details. Implies
>   `--dry-run`.

I suspect that it would be easier to implement to allow "commit --porcelain",
but I am not sure if it truly makes sense.  Not a new problem, though.

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index e1e8f57..de97729 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -32,11 +32,14 @@ OPTIONS
>  --branch::
>   Show the branch and tracking info even in short-format.
>  
> ---porcelain::
> +--porcelain[=]::
>   Give the output in an easy-to-parse format for scripts.
>   This is similar to the short output, but will remain stable
>   across Git versions and regardless of user configuration. See
>   below for details.
> ++
> +The version parameter is used to specify the format version.
> +This is optional and defaults to the original version '1' format.

I agree with Peff that "v1" would be easier to read, and also his
comment on parsing using strcmp() to require exact matches and
resetting to NONE, not UNSPECIFIED.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] GitRev News edition 17

[Junio C Hamano]

On Wed, Jul 20, 2016 at 1:38 PM, Christian Couder
 wrote:
>> Micronit. 2.9.1 (or 2.9.2) would not be a "major release". 2.9.0 was,
>> and x.y.z (z > 0) are "maintenance releases".
>
> Yeah, I wondered about that when I saw :
>
> https://github.com/git/git.github.io/commit/d2eb8fbbb30594d19fcda731c309ad03229dc5d5
>
> but forgot to ask or do something about it...

As long as it gets fixed quickly, no harm is 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: [ANNOUNCE] GitRev News edition 17

[Christian Couder]

On Wed, Jul 20, 2016 at 8:05 PM, Junio C Hamano  wrote:
> On Wed, Jul 20, 2016 at 5:58 AM, Christian Couder
>  wrote:
>> Hi everyone,
>>
>> I'm happy announce that the 17th edition of Git Rev News is now published:
>>
>> https://git.github.io/rev_news/2016/07/20/edition-17/
>
> Micronit. 2.9.1 (or 2.9.2) would not be a "major release". 2.9.0 was,
> and x.y.z (z > 0) are "maintenance releases".

Yeah, I wondered about that when I saw :

https://github.com/git/git.github.io/commit/d2eb8fbbb30594d19fcda731c309ad03229dc5d5

but forgot to ask or do something about it...

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

Re: git-prompt.sh incompatible with non-basic global grep.patternType

[Junio C Hamano]

Jeff King  writes:

> On Mon, Jul 18, 2016 at 03:56:09PM -0700, Richard Soderberg wrote:
>
>> ps. git log --basic-regexp does not fix the issue, as for unknown
>> reasons (I'll start another thread) the command-line option doesn't
>> override grep.patternType.
>
> Dscho gave a fix for your immediate issue, but this "ps" definitely
> seems like a bug to me. Command-line options should always take
> precedence over config.

This may fix it.  I think the root cause is that logic to smear
"pattern type" into various broken-down fields in grep_opt for the
short-hands like --basic-regexp option needs to leave "I am setting
this short-hand" mark to allow the grep_commit_pattern_type() that
is done as the final step of the set-up sequence before we call
compile_grep_patterns() can take notice.  The calls currently made
to grep_set_pattern_type_option() when we parse "--basic-regexp" and
friends forgets to override the "source of truth" field and only
updates the broken-down fields.

An alternative may be to update places that parse "--basic-regexp"
and friends to just write to .pattern_type_option without calling
grep_set_pattern_type_option(); that might be a cleaner, but I am
not feeling well today so I won't be able to do a deeper analysis
right now.

 grep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grep.c b/grep.c
index 394c856..908ed3d 100644
--- a/grep.c
+++ b/grep.c
@@ -203,6 +203,7 @@ void grep_set_pattern_type_option(enum grep_pattern_type 
pattern_type, struct gr
opt->regflags &= ~REG_EXTENDED;
break;
}
+   opt->pattern_type_option = pattern_type;
 }
 
 static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,

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

Complex gitweb URL

[CLOSE Dave]

I'm trying to create a URL that will always refer to the latest version 
of a file stored under Gerrit. gitweb access is available. The man page 
specification doesn't seem to work for me. Instead, I seem to need to 
put most of the information into arguments (after the '?').

For example, the repo name includes several directories, so it doesn't 
work to put it into the 
".../gitweb.cgi///:/?" format. 
Or, at least, I don't see how.

Instead I'm trying to use a URL in the format, 
"http://site/gitweb/?args;. If I use gitweb itself to navigate to my 
target file, I see a URL in this format that contains several arguments, 
"p=repo;a=blob;f=file;h=SHA;hb=SHA". If I use that URL directly, I get 
my file. But those SHA values are not something I know how to determine 
in advance. And I suspect they are unique to the specific version of the 
file accessed, not always the latest as I want.

If I replace the hb=SHA argument with hb=HEAD, the URL still works. But 
I have no idea what I can use to replace the h=SHA argument.

A complication is that the target file is not in the master branch. 
Somehow I need to be able to specify the branch. I've tried putting it 
as the h= argument but that results in "Reading blob failed". If I leave 
out the h= argument entirely, gitweb responds, "404 cannot find file".

Are these arguments documented somewhere? What is the recommended way to 
construct a URL like I need?
-- 
Dave 
CloseN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v1 0/6] Porcelain Status V2

[Jeff Hostetler]

On 07/20/2016 12:15 PM, Jeff King wrote:

One final bit of food for thought.

Just yesterday somebody asked me about renewing the old idea of using a
more standardized format for machine-readable output, like --json.
That's obviously something that would exist alongside the existing
formats for compatibility, and it doesn't fundamentally change anything
about adding a new format as your patches do (it just becomes yet
another format).

However I wanted to mention it in case you are intrigued by the idea,
and would be interested in skipping porcelain-v2 entirely in favor of
moving to something like json.

A totally reasonable response is "haha no. Please stop moving the
goalposts". I just wanted to throw it out there as an option (and in
case you are interested, to let you think about it before any more work
goes into this direction).


haha no :-)

Short term, I'd rather nail down what I have now (both content-wise
and format-wise) and see how we like it.  And have a follow-up task
to look at the --state header we spoke of earlier.  And save the JSON
version as an independent task for later.

I understand the motivation for a JSON option (and have thought
about it before) but I think it ought to be kept separate.
At a higher-level, it seems like a JSON option would be an
opportunity to start a project-wide conversation about formats,
consistency, plumbing, and etc.  A top-down conversation if you
will about which commands will/won't get enhanced, legacy cruft
that would not need to be converted, JSON style and naming and
consistency issues, current best practices in the node/whatever
community, and etc.  I could be wrong, but this feels like a
top-down feature conversation in a wider audience.

Jeff


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

Re: Git and SHA-1 security (again)

[Junio C Hamano]

Stefan Beller  writes:

>> to follow the above, in my view, interaction with sha1 repos go
>> through some conversion bridges like what we have with hg and svn. I
>> don't know if we are going this route. It's certainly simpler and
>> people already have experiences (from previous migration) to prepare
>> for it.
>
> When treating the SHA1 version as a foreign dvcs and the SHA256
> as the real deal, we could introduce "pointer objects", and during the
> conversion
> create a 4e55ed3 pointer that points to the SHA256 commit of (add:
> add --chmod=+x / --chmod=-x options, 2016-05-31).

Hmmm.  If you are designing this "pointer objects" to be extensible
enough to cover other foreign vcs (i.e.e.g. you make it to be
capable of mapping Subversion's r24323 to a matching commit in the
converted result), I would think it may be a very useful thing to
have, but I think it is pretty much orthogonal to the discussion in
this topic.  IOW, that can happen with or without change of the hash
function.

And looking at it that way, I am not sure if such a mapping feature
should require adding a new type of "object".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] contrib/persistent-https: update ldflags syntax for Go 1.7+

[Junio C Hamano]

Parker Moore  writes:

>> the logical place to pull that information from would be 
>> ../../GIT-VERSION-FILE,
>
> I agree. It would make more sense to build this to a specific version
> or git revision rather
> than a time. Perhaps that would be a different patch?

It would definitely be a separate (and optional) patch and must come
after "do we use = in between or send var/val as two separate
arguments?" patch.  That was what I meant to say with "... would be
version file, but the mechanism to embed it would be the same as
today".

>> So unless the "dynamic lookup in the Makefile" turns out to be too
>> gross, we would want to keep the mechanism and just make it usable
>> for versions before 1.5 and also after 1.7, I would guess.
>
> A dynamic lookup of the go version would look for go 1.0, 1.1, 1.2,
> 1.3, 1.4 and 1.5.0.
> These versions would be incompatible with the `X var=val` syntax. I am
> not too familiar
> with Makefile syntax for numerical comparison, but I believe this
> would be fairly simple.

$ go version
go version go1.3.3 linux/amd64

is what I seem to be locally getting, so I'd imagine it would be
something like this perhaps?

git-remote-persistent-https:
case $$(go version) in \
"go version go"1.[0-5].*) EQ=" " ;; *) EQ="=" ;; esac && \
go build -o git-remote-persistent-https \
-ldflags "-X main._BUILD_EMBED_LABEL$${EQ}$(BUILD_LABEL)"

> Would you like me to whip up a patch for it?

Surely, and 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: How to generate feature branch statistics?

[Junio C Hamano]

Jeff King  writes:

> In a workflow that merges feature branches to master, you can generally
> recognize them by looking for merges along the first-parent chain of
> commits:
>
>   git log --first-parent --merges master
>
> (Depending on your workflow, some feature branches may be fast-forwards
> with no merge commit, so this is just a sampling. Some workflows use
> "git merge --no-ff" to merge in feature branches, so this would see all
> of them).
> And then for each merge, you can get the set of commits that were merged
> in (it is the commits in the second parent that are not in the first).
> The bottom-most one is the "start" of the branch (or close to it; of
> course the author started writing code before they made a commit), and
> the "end" is the merge itself.

A few things to keep in mind are

 * A feature branch may be merged to the master multiple times,
   when the feature branch is properly managed.

   E.g. It may have been once thought to be complete with 3 commits,
   get merged to 'master', then a bug is discovered and gain its
   fourth commits to fix the bug and merged to 'master' again,
   resulting in a topology like this:

 A---B---C---D (feature)
/ \   \
---o---o---o---1---o---o---2---o (master)

   "git log --first-parent --merges master" will first find commit
   '2' that merged the feature for the second time, bringing in
   commit 'D', and then it will find commit '1' that merged the
   feature previously, bringing in commit 'A', 'B' and 'C'.

 * A feature branch that depends on other feature may have merges on
   their own.  You may start a feature X that depends on another
   features Y and Z that are not yet in 'master', in addition to
   depending on things in 'master' that have been added since Y and
   Z forked from it.  In such a case, your feature X may look like
   this:

 .---1--2x---x (feature X)
/   /  /
   y---y---y (feature Y)   /  /
  /   /  /
  ---o---o---o---o---o---o---o---0 (master) /
  \/
   z---z (feature Z)  /
\/
 .--.

   where '1' and '2' are merges of feature Y and then Z into the tip
   of 'master' when you start working on feature X.

   And then feature Y and feature Z may graduate to 'master' before
   your feature X is ready to do so, resulting in something like:

 .---1--2x---x (feature X)
/   /  /
   y---y---y (feature Y)   / ---  /  --.
  /   /  /  \
  ---o---o---o---o---o---o---o---o---o---o---o---o---Y---Z (master)
  \//
   z---z (feature Z) --   /  --.
\/
 .--.

   where 'Y' and 'Z' are merges of features Y and Z to 'master'.
   After that, feature X may become ready to be merged, resulting in:

 .---1--2x---x (feature X)
/   /  /  \
   y---y---y (feature Y)   / ---  /  --.   \
  /   /  /  \   \
  ---o---o---o---o---o---o---o---o---o---o---o---o---Y---Z---o---X (master)
  \//
   z---z (feature Z) --   /  --.
\/
 .--.

  When "git log --first-parent --merges master" finds X, it would
  notice that it pulled in commits '1', '2' and two 'x'.  The "tool"
  to inspect the history needs to be careful deciding if '1' and '2'
  are the part of feature X.  There are variants that make it tricky
  (e.g. 'Y' may not have yet been merged to 'master' when 'X' is
  merged, in which case you may end up pulling both 'x' and 'y' into
  'master' with a single merge), which should be avoided if feature
  branches are managed carefully, but not everybody is careful when
  managing their history.

Coming back to the introduction of the original message:

>> I assume that feature branches are not frequently enough merged into
>> master. Because of that we discover bugs later than we could with a more
>> continuous code integration. I don't want to discuss here whether feature
>> branches are good or bad.

For our own history and workflow, the duration between the inception
of a topic branch and the time it gets merged to 'master' is not all
that interesting.  More interesting numbers are:

 * The duration between the time a topic hits 'next' and the time it
   gets merged to 'master'.  This is the time the developers and
   testers 

Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2

[Jeff Hostetler]

On 07/20/2016 12:06 PM, Jeff King wrote:

On Tue, Jul 19, 2016 at 06:10:56PM -0400, Jeff Hostetler wrote:


+   } else {
+   /*
+* TODO All of various print routines allow for s->branch to be 
null.
+* TODO When can this happen and what should we report here?
+*/
+   fprintf(s->fp, " %s", "(unknown)");
+   }


IIRC, it happens when HEAD points to a broken ref. So something like:

   git init
   echo broken >.git/refs/heads/master

would cause resolving HEAD to return NULL.


That worked and I see "(unknown)".

This is a bit of a nit, but is there a value we'd like
to see there, such as "(unknown)" or "(broken)" or "(missing)"
in that case?  (And make it clear that this is a different
case from "(detached)".)

I'm thinking it would be nicer to always have a field
there for parsing.

Jeff




--
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: How to generate feature branch statistics?

[Jakub Narębski]

W dniu 2016-07-20 o 15:56, Jakub Narębski pisze:
> W dniu 2016-07-20 o 10:05, Ernesto Maserati pisze:
> 
>> I assume that feature branches are not frequently enough merged into
>> master. Because of that we discover bugs later than we could with a more
>> continuous code integration. I don't want to discuss here whether feature
>> branches are good or bad.
>>
>> I want just to ask is there a way how to generate a statistic for the
>> average duration of feature branches until they are merged to the master? I
>> would like to know if it is 1 day, 2 days or lets say 8 or 17 days. Also it
>> would be interesting to see the statistical outliers.
>>
>> I hope my motivation became clear and what kind of git repository data I
>> would like to produce.
>>
>> Any ideas?
> 
> There are at least two tools to generate statistics about git repository,
> namely Gitstat (https://sourceforge.net/projects/gitstat) and GitStats
> (https://github.com/hoxu/gitstats), both generating repo statistics as
> a web page. You can probably find more... but I don't know if any includes
> the statistics you need.
> 
> I assume that you have some way of determining if the merge in 'master'
> branch is a merge of a topic branch, or of long-lived graduation branch
> (e.g. 'maint' or equivalent). To simplify the situation, I assume that
> the only merges in master are merges of topic branches:
> 
>   git rev-list --min-parents=2 master | 

Self correction: Here you need to use --first-parent, as in Peff answer
(which also uses less git invocations, and less of git porcelain).

I wonder if it is something that libgit2 would be helpful...
-- 
Jakub Narębski

--
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] config.mak.uname: set PERL_PATH for FreeBSD 5.0+

[Junio C Hamano]

On Wed, Jul 20, 2016 at 11:07 AM, Eric Wong  wrote:
> Also, my use of a numeric comparison may be more future-proof
> in case FreeBSD decides to have /usr/bin/perl again.
>
> I also wonder why we don't use `which` to search for somewhat
> standard path components, instead.  Something like:

Because historically output from "which" was not meant to be
machine parseable (some implementation said 'perl is /usr/bin/perl')
--
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] config.mak.uname: set PERL_PATH for FreeBSD 5.0+

[Eric Wong]

Johannes Schindelin  wrote:
> Hi Eric,
> 
> On Wed, 20 Jul 2016, Eric Wong wrote:
> 
> > diff --git a/config.mak.uname b/config.mak.uname
> > index a88f139..6c29545 100644
> > --- a/config.mak.uname
> > +++ b/config.mak.uname
> > @@ -202,6 +202,11 @@ ifeq ($(uname_S),FreeBSD)
> > NO_UINTMAX_T = YesPlease
> > NO_STRTOUMAX = YesPlease
> > endif
> > +   R_MAJOR := $(shell expr "$(uname_R)" : '\([0-9]*\)\.')
> > +
> > +   ifeq ($(shell test "$(R_MAJOR)" -ge 5 && echo 1),1)
> > +   PERL_PATH = /usr/local/bin/perl
> > +   endif
> 
> In keeping with other uname_R usage, should this not read
> 
>   # Since FreeBSD 5.0, Perl is part of the core
>   ifneq ($(shell expr "$(uname_R)" : '[1-4]\.'),2)
>   PERL_PATH = /usr/local/bin/perl
>   endif
> 
> instead?

That's fine; however I don't use `expr` often, so it required
a little more time to realize the '2' means 2 characters were
matched.

Also, my use of a numeric comparison may be more future-proof
in case FreeBSD decides to have /usr/bin/perl again.

I also wonder why we don't use `which` to search for somewhat
standard path components, instead.  Something like:

  PERL_PATH = $(shell PATH=/bin:/usr/bin:/usr/local/bin which perl)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] GitRev News edition 17

[Junio C Hamano]

On Wed, Jul 20, 2016 at 5:58 AM, Christian Couder
 wrote:
> Hi everyone,
>
> I'm happy announce that the 17th edition of Git Rev News is now published:
>
> https://git.github.io/rev_news/2016/07/20/edition-17/

Micronit. 2.9.1 (or 2.9.2) would not be a "major release". 2.9.0 was,
and x.y.z (z > 0) are "maintenance releases".
--
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 2/6] Status and checkout unit tests for --porcelain[=]

[Jeff Hostetler]

On 07/20/2016 12:03 PM, Jeff King wrote:

On Wed, Jul 20, 2016 at 10:00:07AM -0600, Jeff King wrote:


On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:


+test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '
+   echo bongo bongo bongo >>file &&
+   git commit -m next -a --porcelain=bogus
+'


Hrm. That seems unexpected to me. Shouldn't it complain about
--porcelain=bogus?


Pondering more, did you mean:

   test_expect_success '--porcelain=bogus complains about format' '
echo bongo bongo bongo >>file &&
test_must_fail git commit -m next -a --porcelain=bogus
   '

?


Yes.  That is what I meant.  And yes, some of the names are confusing.
Thanks.

Jeff
--
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 2/6] Status and checkout unit tests for --porcelain[=]

[Jeff Hostetler]

On 07/20/2016 12:00 PM, Jeff King wrote:

On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:


+test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '
+   echo bongo bongo bongo >>file &&
+   git commit -m next -a --porcelain=bogus
+'


Hrm. That seems unexpected to me. Shouldn't it complain about
--porcelain=bogus?


Looks like a cut-n-paste operator error.

Jeff
--
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 1/6] Allow --porcelain[=] in status and commit commands

[Jeff Hostetler]

On 07/20/2016 11:58 AM, Jeff King wrote:

On Tue, Jul 19, 2016 at 06:10:53PM -0400, Jeff Hostetler wrote:


+static int opt_parse_porcelain(const struct option *opt, const char *arg, int 
unset)
+{
+   enum wt_status_format *value = (enum wt_status_format *)opt->value;
+   if (unset) {
+   *value = STATUS_FORMAT_UNSPECIFIED;


Nice attention to detail here and below in handling "unset" and "!arg"
cases.  I think should be STATUS_FORMAT_NONE, though, which is what the
old code used to do (since "0" is the usual special value for --no-*
options). It only matters if you do:

   git status --no-porcelain

Right now that will switch to the long format, regardless of your
config. With your path it defaults to any configured value. It's
probably a case that nobody hits ever, but in the absence of a good
reason to do otherwise, I'd stick with the current behavior.


Good catch. I'll make it _NONE.




+   } else if (arg) {
+   int n = strtol(arg, NULL, 10);
+   if (n == 1)
+   *value = STATUS_FORMAT_PORCELAIN;
+   else
+   die("unsupported porcelain version");


This silently allows:

   git status --porcelain="1 for the money"

and later:

   git status --porcelain="2 for the show"

Probably not a big deal in practice, but since the list of formats is
constrained, we don't really care about parsing arbitrary numbers.
So:

   if (!strcmp(arg, "1"))
*value = STATUS_FORMAT_PORCELAIN;

is actually simpler, and more robust.

I also wondered if:

   git status --porcelain=v1

is more self-documenting about the meaning of "1". It's purely
aesthetics, but it somehow looks better to me. Matching that is also
much easier with pure strcmps.


I wondered about making it =v1 rather than just =1. It seemed
more aesthetically pleasing, even if it was an extra character
to type.  In a later email in this thread you mention a JSON
option.  If I switched this here to be "=v1" and "=v2", it would
be easy later to have a "=j2" or "=v2j" to do that.





@@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)

s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
s.ignore_submodule_arg = ignore_submodule_arg;
+   s.status_format = status_format;
+


I wonder what happens if you pass a "wt_status" with a format of "SHORT"
to the long-formatting code.

I think it is ignored completely, as you are just now introducing the
s.status_format field. But I wonder if there is room for further cleanup
in pushing the big switch statements from run_status() and cmd_status()
into wt-status.c.


I'll look into that.

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

[PATCH v4 4/4] t2029: some really basic tests for submodules in multi worktree

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 t/t2029-worktree-submodule.sh (new +x) | 166 +
 1 file changed, 166 insertions(+)
 create mode 100755 t/t2029-worktree-submodule.sh

diff --git a/t/t2029-worktree-submodule.sh b/t/t2029-worktree-submodule.sh
new file mode 100755
index 000..f96fa50
--- /dev/null
+++ b/t/t2029-worktree-submodule.sh
@@ -0,0 +1,166 @@
+#!/bin/sh
+
+test_description='submodule with multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   git config extensions.worktreeConfig true &&
+   >t &&
+   git add t &&
+   git commit -m initial &&
+   git branch initial
+'
+
+test_expect_success 'setup - repository in init subdirectory' '
+   mkdir init &&
+   (
+   cd init &&
+   git init &&
+   git config extensions.worktreeConfig true &&
+   echo a >a &&
+   git add a &&
+   git commit -m "submodule commit 1" &&
+   git tag -a -m "rev-1" rev-1
+   )
+'
+
+test_expect_success 'setup - commit with gitlink' '
+   echo a >a &&
+   echo z >z &&
+   git add a init z &&
+   git commit -m "super commit 1"
+'
+
+test_expect_success 'setup - hide init subdirectory' '
+   mv init .subrepo
+'
+
+test_expect_success 'setup - repository to add submodules to' '
+   git init addtest &&
+   git -C addtest config extensions.worktreeConfig true &&
+   git init addtest-ignore &&
+   git -C addtest-ignore config extensions.worktreeConfig true
+'
+
+# The 'submodule add' tests need some repository to add as a submodule.
+# The trash directory is a good one as any. We need to canonicalize
+# the name, though, as some tests compare it to the absolute path git
+# generates, which will expand symbolic links.
+submodurl=$(pwd -P)
+
+listbranches() {
+   git for-each-ref --format='%(refname)' 'refs/heads/*'
+}
+
+inspect() {
+   dir=$1 &&
+   dotdot="${2:-..}" &&
+
+   (
+   cd "$dir" &&
+   listbranches >"$dotdot/heads" &&
+   { git symbolic-ref HEAD || :; } >"$dotdot/head" &&
+   git rev-parse HEAD >"$dotdot/head-sha1" &&
+   git update-index --refresh &&
+   git diff-files --exit-code &&
+   git clean -n -d -x >"$dotdot/untracked"
+   )
+}
+
+test_expect_success 'submodule add' '
+   echo "refs/heads/master" >expect &&
+   >empty &&
+
+   (
+   cd addtest &&
+   git submodule add -q "$submodurl" submod >actual &&
+   test_must_be_empty actual &&
+   echo "gitdir: ../.git/modules/submod" >expect &&
+   test_cmp expect submod/.git &&
+   (
+   cd submod &&
+   git config core.worktree >actual &&
+   echo "../../../submod" >expect &&
+   test_cmp expect actual &&
+   rm -f actual expect
+   ) &&
+   git submodule init
+   ) &&
+
+   rm -f heads head untracked &&
+   inspect addtest/submod ../.. &&
+   test_cmp expect heads &&
+   test_cmp expect head &&
+   test_cmp empty untracked
+'
+
+test_expect_success 'submodule.* in supermodule is per-worktree' '
+   (
+   cd addtest &&
+   git config -f .git/config.worktree submodule.submod.url >actual 
&&
+   echo "$submodurl" >expect &&
+   test_cmp expect actual
+   )
+'
+
+test_expect_success 'turn submodule to multiworktree' '
+   (
+   cd addtest/.git/modules/submod &&
+   CORE_WT="$(git config core.worktree)" &&
+   git config -f config.worktree core.worktree "$CORE_WT" &&
+   git config --unset core.worktree &&
+   git config extensions.worktreeConfig true &&
+   git config core.worktree >actual &&
+   echo "$CORE_WT" >expect &&
+   test_cmp expect actual
+   )
+'
+
+test_expect_success 'new worktree in submodule' '
+   (
+   cd addtest/submod &&
+   git worktree add submod-elsewhere &&
+   cd submod-elsewhere &&
+   test_must_fail git config core.worktree
+   )
+'
+
+test_expect_success 'new worktree in supermodule' '
+   (
+   cd addtest &&
+   git commit -m initial &&
+   git worktree add super-elsewhere &&
+   cd super-elsewhere &&
+   test_must_fail git config submodule.submode
+   )
+'
+
+test_expect_success 'submodule add in the second worktree' '
+   (
+   cd addtest/super-elsewhere &&
+   git submodule add -q "$submodurl" submod2 >actual &&
+   test_must_be_empty actual &&
+   echo "gitdir: 
../../.git/worktrees/super-elsewhere/modules/submod2" >expect &&
+   test_cmp 

[PATCH v4 2/4] submodule: update core.worktree using git-config

[Nguyễn Thái Ngọc Duy]

To access a separate repository, the first step should be read its
config file to determine if this repository layout is supported or
not, or if we understand all repo extensions, of it is a linked
worktree. Only then should know where to update the config file.

Unfortunately, our C code base is not ready for doing all that in the
same process. The repo detection is not meant to be used for peeking
in other repository, and config code would read config.worktree that
is in _current_ $GIT_DIR.

For now, let's spawn a new process and let all that done separately.

PS. submodule-helper also updates core.worktree. But in there, we
create a new clone, we know what is the initial repository layout, so
we know we can simply update "config" file without risks.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 submodule.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index abc2ac2..b912871 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1128,7 +1128,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, 
const char *git_dir)
 {
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
+   struct strbuf path = STRBUF_INIT;
const char *real_work_tree = xstrdup(real_path(work_tree));
+   struct child_process cp = CHILD_PROCESS_INIT;
 
/* Update gitfile */
strbuf_addf(_name, "%s/.git", work_tree);
@@ -1136,13 +1138,17 @@ void connect_work_tree_and_git_dir(const char 
*work_tree, const char *git_dir)
   relative_path(git_dir, real_work_tree, _path));
 
/* Update core.worktree setting */
-   strbuf_reset(_name);
-   strbuf_addf(_name, "%s/config", git_dir);
-   git_config_set_in_file(file_name.buf, "core.worktree",
-  relative_path(real_work_tree, git_dir,
-_path));
+   strbuf_addstr(, relative_path(real_work_tree, git_dir,
+  _path));
+   cp.git_cmd = 1;
+   argv_array_pushl(, "-C", work_tree, NULL);
+   argv_array_pushl(, "--work-tree", ".", NULL);
+   argv_array_pushl(, "config", "core.worktree", path.buf, NULL);
+   if (run_command() < 0)
+   die(_("failed to update core.worktree for %s"), git_dir);
 
strbuf_release(_name);
+   strbuf_release();
strbuf_release(_path);
free((void *)real_work_tree);
 }
-- 
2.9.1.566.gbd532d4

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

[PATCH v4 0/4] Split .git/config in multiple worktree setup

[Nguyễn Thái Ngọc Duy]

On Wed, Jul 20, 2016 at 6:14 AM, Duy Nguyen  wrote:
>> If yes, do you know if someone is working on this? If nobody is working on 
>> this, do
>> you have some pointers for me what the main problems are?
>
> The blocker is config file being shared (you do not want to share
> core.worktree and submodule.*). I made some progress last weekend,
> just needed to add some tests to see if submodule works as expected
> and will post the series soon. Then you can take over if you want ;)

Here it is. I'm going to describe some more for new people. Let's
start with the problem, then the high level solution and finally
what's done in submodule. These are separated by ^$

Multipl worktrees in its current form share the config file. The only
exceptions are core.bare and core.worktree which will be applied for
the main worktree only, if present in the config file.

This does not make submodules happy because submodules use
core.worktree to link back to the real repos stored inside .git dir of
the super modules. This is not so bad right now because the
submodule's worktree would be "main" worktree and core.worktree sticks
to it. If one day "git submodule add" initialize the worktree as a
linked worktree, problem arises.

The second problem is more real. When you initialize submodules,
submodule info is stored in the supermodule's config file, which is
shared. So the secondary worktree will not be able to initialize its
own submodules (and may be confused by the existing submodule.*
section).



So we need to split the config file into two logical parts: a shared
part and a worktree-specific one. This makes everybody happy even
though it's not easy.

What this series does is adding "git config --worktree" which writes
to the worktree-specific part, while "git config" writes to the shared
part. "git config" as a read operation will read the shared part
first, then the worktree specific part.

Now. In current multiple worktrees setup, both the shared and private
parts are in the same file, "config". And "git config --worktree" will
refuse to work if you have more than one worktree. For it to work with
multiple worktree, you need to enable extensions.worktreeConfig (in
config file).

This extension designates the file "config.worktree" as storage for
the private part. It can be .git/config.worktree for main worktree, or
.git/worktrees/xxx/config.worktree for linked ones.

Before enabling extensions.worktreeConfig (or soon after it), you need
to move core.bare and core.worktree to .git/config.worktree because
the exceptions above are gone. If they are present in "config" file,
they are _shared_ (and hell follows)

If you have followed through the first four iterations, v4 [1] has
very close design. The main difference is: in v4, "config" is
per-worktree and the shared part is split away, hidden in
.git/common/config. This leads to the migration and backward
compatibility problems.

The new design is free of that because "config" remains shared while
the private is hidden away. The risk is "git config" by default writes
to the shared part. Accidentally sharing something may be more
dangerous than accidentally _not_ sharing something like v3 [1] (which
defaults to per-worktree). I've thought about this and I'm willing to
take the new direction, bettting that 90% of the time people want to
share, so it's a rare problem.



With all that in place, what does a command have to do to take
advantage of it?

- Whenever you need to write a per-worktree config (you decide it!),
  use "git config --worktree". That's it. You don't really need to
  care where it ends up to. This is what 3/4 is, for submodule.

- Avoid "git config /path/to/.git/config" because that may or may not
  be the right place (there's also config.worktree now). Builtin
  commands have this worse because if you look at _another_ repo, then
  you may need to go through repo detection and stuff before you can
  read its config. I just fall back to running "git config" in 2/4.

So that's it. It seems to be running ok. But I know very little about
submodules to test it properly.

The only problem left that I have to work out is config deletion. I
suppose we could follow the chain backward again: try to delete in
per-worktree config file first. If not found, try again in the shared
config file...

[1] http://thread.gmane.org/gmane.comp.version-control.git/281906/focus=284803
-- 
2.9.1.566.gbd532d4

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

[PATCH v4 3/4] submodule: support running in multiple worktree setup

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-worktree.txt | 8 
 git-submodule.sh   | 8 
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 41350db..2a5661d 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -142,6 +142,14 @@ to share to all working directories:
you are sure you always use sparse checkout for all working
directories.
 
+ - `submodule.*` in current state should not be shared because the
+   information is tied to a particular version of .gitmodules in a
+   working directory.
+
+ - `remote.*` added by submodules may be per working directory as
+   well, unless you are sure remotes from all possible submodules in
+   history are consistent.
+
 DETAILS
 ---
 Each linked working tree has a private sub-directory in the repository's
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..7b576f5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -261,7 +261,7 @@ or you are unsure what this means choose another name with 
the '--name' option."
esac
) || die "$(eval_gettext "Unable to checkout submodule 
'\$sm_path'")"
fi
-   git config submodule."$sm_name".url "$realrepo"
+   git config --worktree submodule."$sm_name".url "$realrepo"
 
git add $force "$sm_path" ||
die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
@@ -461,7 +461,7 @@ Submodule work tree '\$displaypath' contains a .git 
directory
# Remove the whole section so we have a clean state when
# the user later decides to init this submodule again
url=$(git config submodule."$name".url)
-   git config --remove-section submodule."$name" 
2>/dev/null &&
+   git config --worktree --remove-section 
submodule."$name" 2>/dev/null &&
say "$(eval_gettext "Submodule '\$name' (\$url) 
unregistered for path '\$displaypath'")"
fi
done
@@ -1106,7 +1106,7 @@ cmd_sync()
then
displaypath=$(git submodule--helper relative-path 
"$prefix$sm_path" "$wt_prefix")
say "$(eval_gettext "Synchronizing submodule url for 
'\$displaypath'")"
-   git config submodule."$name".url "$super_config_url"
+   git config --worktree submodule."$name".url 
"$super_config_url"
 
if test -e "$sm_path"/.git
then
@@ -1114,7 +1114,7 @@ cmd_sync()
sanitize_submodule_env
cd "$sm_path"
remote=$(get_default_remote)
-   git config remote."$remote".url 
"$sub_origin_url"
+   git config --worktree remote."$remote".url 
"$sub_origin_url"
 
if test -n "$recursive"
then
-- 
2.9.1.566.gbd532d4

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

[PATCH v4 1/4] worktree: add per-worktree config files

[Nguyễn Thái Ngọc Duy]

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config files are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file ".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
extension is not present so that it works in any setup.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt   | 11 -
 Documentation/git-config.txt   | 26 
 Documentation/git-worktree.txt | 31 ++
 Documentation/gitrepository-layout.txt |  8 
 builtin/config.c   | 18 +++-
 cache.h|  2 +
 config.c   |  7 
 environment.c  |  1 +
 setup.c|  5 ++-
 t/t2028-worktree-config.sh (new +x)| 77 ++
 10 files changed, 175 insertions(+), 11 deletions(-)
 create mode 100755 t/t2028-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 16dc22d..7d64da0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 --
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) are each
+repository is used to store the configuration for that repository, and
 `$HOME/.gitconfig` is used to store a per-user configuration as
 fallback values for the `.git/config` file. The file `/etc/gitconfig`
 can be used to store a system-wide default configuration.
@@ -264,6 +265,12 @@ advice.*::
show directions on how to proceed from the current state.
 --
 
+extensions.worktreeConfig::
+   If set, by default "git config" reads from both "config" and
+   "config.worktree" file in that order. In multiple working
+   directory mode, "config" file is shared while
+   "config.worktree" is per-working directory.
+
 core.fileMode::
Tells Git if the executable bit of files in the working tree
is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index f163113..9dfdb6a 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -47,13 +47,15 @@ checks or transformations are performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file ` can be
-used to tell the command to read from only that location (see <>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file ` can be used to tell the command to read from only
+that location (see <>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file ` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file ` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -133,6 +135,11 @@ from all available files.
 +
 See also <>.
 
+--worktree::
+   Similar to `--local` except that 

Re: Git and SHA-1 security (again)

[Stefan Beller]

On Wed, Jul 20, 2016 at 7:44 AM, Duy Nguyen  wrote:
> On Wed, Jul 20, 2016 at 2:28 PM, Johannes Schindelin
>  wrote:
>> But that strategy *still* ignores the distributed nature of Git. Just
>> because *you* make that merge at a certain point does not necessarily mean
>> that I make it at that point, too.
>>
>> Any approach that tries to have one single point of conversion will most
>> likely fall short of a solution.
>
> OK I see the difference in our views now. To me an sha256 repo would
> see an sha1 repo as a _foreign_ DVCS, pretty much like git sees
> mercurial now. So a transition from sha1 to sha256 is not that
> different from cvs -> svn -> a dvcs bubble -> git.
>
>> To be honest, I am less concerned about the GPG-signed commits (after all,
>> after switching to a more secure hash algorithm, a maintainer could
>> cross-sign all signed commits, or only the branch tips or tags, as new
>> tags, to reinstitute trust).
>>
>> I am much more concerned about references to commits, both inside and
>> outside the repository. That is, if I read anywhere on the internet about
>> Git having added support for `git add --chmod=+x ` in 4e55ed3 (add:
>> add --chmod=+x / --chmod=-x options, 2016-05-31), I want to find that
>> commit by that reference.
>>
>> And I am of course concerned what should happen if a user wants to fetch
>> from, or push to, a SHA-1-hashed remote repository into, or from, a
>> SHA-256-hashed local one.
>
> to follow the above, in my view, interaction with sha1 repos go
> through some conversion bridges like what we have with hg and svn. I
> don't know if we are going this route. It's certainly simpler and
> people already have experiences (from previous migration) to prepare
> for it.

When treating the SHA1 version as a foreign dvcs and the SHA256
as the real deal, we could introduce "pointer objects", and during the
conversion
create a 4e55ed3 pointer that points to the SHA256 commit of (add:
add --chmod=+x / --chmod=-x options, 2016-05-31). Ideally we would
not even expose this sort of object a lot, e.g. git show  would just
redirect automatically. Instead of a new class of "pointer objects" we could
also solve this via a lot of refs. (refs/old-sha1/4e55ed3 pointing to
the converted
commit; Though we would need to accept partial refs names then :/)

> --
> 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
--
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] config: add conditional include

[Jakub Narębski]

W dniu 2016-07-16 o 18:47, Jeff King pisze:

> Heh. Don't get me wrong, I do think there's room for digging ourselves a
> deep hole with conditional includes, because anything dynamic opens up a
> question of _when_ it is evaluated, and in what context. So any
> condition should be something that we would consider static through the
> whole run of the program. Looking at the "gitdir" is right on the
> borderline of that, but I think it's OK, because we already have to
> invalidate any cached config when we setup the gitdir, because
> "$GIT_DIR/config" will have become a new source.
> 
> So I agree it's something we need to be thoughtful about, but I think
> this particular instance is useful and probably not going to bite us.

A question: we have a way to track where the value came from (tracking
includes). Do we have a way to check where the value didn't came from,
because for example error in the include condition?

-- 
Jakub Narębski

--
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 0/6] Porcelain Status V2

[Jeff King]

On Tue, Jul 19, 2016 at 06:10:52PM -0400, Jeff Hostetler wrote:

> This patch series adds porcelain V2 format to status.
> This provides detailed information about file changes
> and about the current branch.
> 
> The new output is accessed via:
> git status --porcelain=2 [--branch]
> 
> An earlier draft of this work was submitted under the
> "Add very verbose porcelain output to status" title.
> This new version addresses the concerns about using
> (abusing) "-vv" and simplifies the some of the formatting.

I reviewed the first two, which look good except for a few minor
comments. I don't have time at the moment to dig carefully into the v2
format itself from the later patches (but from a cursory view they look
OK). I'm flying all day today, so probably won't get to a more thorough
review for a day or two (but if there are other reviewers, please don't
feel you have to wait for my input).

One final bit of food for thought.

Just yesterday somebody asked me about renewing the old idea of using a
more standardized format for machine-readable output, like --json.
That's obviously something that would exist alongside the existing
formats for compatibility, and it doesn't fundamentally change anything
about adding a new format as your patches do (it just becomes yet
another format).

However I wanted to mention it in case you are intrigued by the idea,
and would be interested in skipping porcelain-v2 entirely in favor of
moving to something like json.

A totally reasonable response is "haha no. Please stop moving the
goalposts". I just wanted to throw it out there as an option (and in
case you are interested, to let you think about it before any more work
goes into this direction).

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

Re: [PATCH] worktree: add: introduce the --name option

[Antoine Tenart]

On Wed, Jul 20, 2016 at 04:54:40PM +0200, Duy Nguyen wrote:
> On Tue, Jul 19, 2016 at 9:35 PM, Antoine Tenart  wrote:
> >
> > Hmm, so if I understand correctly my use case still won't be supported,
> > as adding a new worktree with the same basename will fail. Or did I miss
> > something?
> 
> Hm... _what_ fails? If you create two worktrees project0/foo and
> project1/foo, you'll get .git/worktrees/foo and .git/worktrees/foo1
> but worktree creation should succeed both times. As long as you don't
> have to look into .git/worktrees/ everything should be fine, you won't
> see foo vs foo1.

You're right, I don't know why I was sure this wasn't working... Sorry
for the noise :-)

-- 
Antoine


signature.asc
Description: PGP signature

Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2

[Jeff King]

On Tue, Jul 19, 2016 at 06:10:56PM -0400, Jeff Hostetler wrote:

> + } else {
> + /*
> +  * TODO All of various print routines allow for s->branch to be 
> null.
> +  * TODO When can this happen and what should we report here?
> +  */
> + fprintf(s->fp, " %s", "(unknown)");
> + }

IIRC, it happens when HEAD points to a broken ref. So something like:

  git init
  echo broken >.git/refs/heads/master

would cause resolving HEAD to return NULL.

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

Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]

[Jeff King]

On Wed, Jul 20, 2016 at 10:00:07AM -0600, Jeff King wrote:

> On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:
> 
> > +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '
> > +   echo bongo bongo bongo >>file &&
> > +   git commit -m next -a --porcelain=bogus
> > +'
> 
> Hrm. That seems unexpected to me. Shouldn't it complain about
> --porcelain=bogus?

Pondering more, did you mean:

  test_expect_success '--porcelain=bogus complains about format' '
echo bongo bongo bongo >>file &&
test_must_fail git commit -m next -a --porcelain=bogus
  '

?

expect_failure is for tests which we _want_ to succeed, but do not yet
(so they get annotated in test results appropriately). expect_success is
for an outcome we expect to happen, but which may involve specific steps
returning failure.

The names are kind of confusing in that regard.

I wonder if just "test_expect" would be a better name for
test_expect_success, and an argument or environment variable to trigger
"we know this is currently broken" rather than having a separate
test_expect_failure function. That's clearly outside the scope of your
series, of course.

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

Re: [PATCH v1 6/6] Unit tests for V2 porcelain status

[Jakub Narębski]

On 20 July 2016 at 17:47, Jeff Hostetler  wrote:
> On 07/20/2016 11:30 AM, Jakub Narębski wrote:
>> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:
>>>
>>> +test_expect_success pre_initial_commit_0 '
>>> +   printf "## branch: (initial) master\n" >expected &&
>>> +   printf "?? actual\n" >>expected &&
>>> +   printf "?? dir1/\n" >>expected &&
>>> +   printf "?? expected\n" >>expected &&
>>> +   printf "?? file_x\n" >>expected &&
>>> +   printf "?? file_y\n" >>expected &&
>>> +   printf "?? file_z\n" >>expected &&
>>
>> Why not use heredoc syntax (cat <<\EOF), or prepare a file
>> with expected output in the testsuite?
>
> The tests involving renames needed to embed a tab character
> in the output and hiding a tab character in a heredoc seemed
> error prone.  So to be consistent I made them all printf-style.

Ah, so that's the case for series of printf. I think in some other
cases the Git testsuite simply uses HT variable for the TAB
character. I guess that "\t" for TAB is available in POSIX and
all shells that Git is run on?

See t3300-funny-names.sh, t3902-quoted.sh, t4213-log-tabexpand.sh

> Also, some of the tests include SHAs for the commit and for
> file content, so having pre-computed expected output is awkward.
> Granted we could hard code the file SHAs, but not the commits.

Right... but heredoc can include variable expansion (or rather
it includes variable expansion by default, and you can prevent
it by quoting end-of-heredoc marker).

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

Re: [PATCH 0/9] Resend of gitster/pb/bisect

[Pranit Bauva]

On Wed, Jul 13, 2016 at 4:05 AM, Pranit Bauva  wrote:
> Hey Junio,
>
> A small mistake got unnoticed by me which Lars recently pointed out.
> The naming convention is "git_path_" and underscore
> instead of spaces.
>
> Thanks!
>
> The interdiff is:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index c2f3cee..88a1df8 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -7,7 +7,7 @@
>  #include "argv-array.h"
>  #include "run-command.h"
>
> -static GIT_PATH_FUNC(git_path_bisect_write_terms, "BISECT_TERMS")
> +static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
>  static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
>  static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
>  static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
> @@ -100,7 +100,7 @@ static int write_terms(const char *bad, const char *good)
> if (check_term_format(bad, "bad") || check_term_format(good, "good"))
> return -1;
>
> -   fp = fopen(git_path_bisect_write_terms(), "w");
> +   fp = fopen(git_path_bisect_terms(), "w");
> if (!fp)
> return error_errno(_("could not open the file BISECT_TERMS"));
>
> @@ -134,7 +134,7 @@ static int bisect_clean_state(void)
> remove_path(git_path_bisect_log());
> remove_path(git_path_bisect_names());
> remove_path(git_path_bisect_run());
> -   remove_path(git_path_bisect_write_terms());
> +   remove_path(git_path_bisect_terms());
> /* Cleanup head-name if it got left by an old version of git-bisect */
> remove_path(git_path_head_name());
> /*
>
>
> Pranit Bauva (9):
>   bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
>   bisect: rewrite `check_term_format` shell function in C
>   bisect--helper: `write_terms` shell function in C
>   bisect--helper: `bisect_clean_state` shell function in C
>   t6030: explicitly test for bisection cleanup
>   wrapper: move is_empty_file() and rename it as
> is_empty_or_missing_file()
>   bisect--helper: `bisect_reset` shell function in C
>   bisect--helper: `is_expected_rev` & `check_expected_revs` shell
> function in C
>   bisect--helper: `bisect_write` shell function in C
>
>  builtin/am.c|  20 +--
>  builtin/bisect--helper.c| 310 
> +++-
>  cache.h |   3 +
>  git-bisect.sh   | 146 +++--
>  t/t6030-bisect-porcelain.sh |  17 +++
>  wrapper.c   |  13 ++
>  6 files changed, 355 insertions(+), 154 deletions(-)

Could someone please look into this series and review so that Junio
can merge this into next which is a vital part of my GSoC project?

Regards,
Pranit Bauva
--
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 2/6] Status and checkout unit tests for --porcelain[=]

[Jeff King]

On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:

> +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '
> + echo bongo bongo bongo >>file &&
> + git commit -m next -a --porcelain=bogus
> +'

Hrm. That seems unexpected to me. Shouldn't it complain about
--porcelain=bogus?

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

Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands

[Jeff King]

On Tue, Jul 19, 2016 at 06:10:53PM -0400, Jeff Hostetler wrote:

> +static int opt_parse_porcelain(const struct option *opt, const char *arg, 
> int unset)
> +{
> + enum wt_status_format *value = (enum wt_status_format *)opt->value;
> + if (unset) {
> + *value = STATUS_FORMAT_UNSPECIFIED;

Nice attention to detail here and below in handling "unset" and "!arg"
cases.  I think should be STATUS_FORMAT_NONE, though, which is what the
old code used to do (since "0" is the usual special value for --no-*
options). It only matters if you do:

  git status --no-porcelain

Right now that will switch to the long format, regardless of your
config. With your path it defaults to any configured value. It's
probably a case that nobody hits ever, but in the absence of a good
reason to do otherwise, I'd stick with the current behavior.

> + } else if (arg) {
> + int n = strtol(arg, NULL, 10);
> + if (n == 1)
> + *value = STATUS_FORMAT_PORCELAIN;
> + else
> + die("unsupported porcelain version");

This silently allows:

  git status --porcelain="1 for the money"

and later:

  git status --porcelain="2 for the show"

Probably not a big deal in practice, but since the list of formats is
constrained, we don't really care about parsing arbitrary numbers.
So:

  if (!strcmp(arg, "1"))
*value = STATUS_FORMAT_PORCELAIN;

is actually simpler, and more robust.

I also wondered if:

  git status --porcelain=v1

is more self-documenting about the meaning of "1". It's purely
aesthetics, but it somehow looks better to me. Matching that is also
much easier with pure strcmps.

> @@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char 
> *prefix)
>  
>   s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
>   s.ignore_submodule_arg = ignore_submodule_arg;
> + s.status_format = status_format;
> +

I wonder what happens if you pass a "wt_status" with a format of "SHORT"
to the long-formatting code.

I think it is ignored completely, as you are just now introducing the
s.status_format field. But I wonder if there is room for further cleanup
in pushing the big switch statements from run_status() and cmd_status()
into wt-status.c.

-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: What's cooking in git.git (Jul 2016, #06; Tue, 19)

[Pranit Bauva]

Hey Junio,

On Wed, Jul 20, 2016 at 4:24 AM, Junio C Hamano  wrote:

> * pb/bisect (2016-07-12) 9 commits
>  - bisect--helper: `bisect_write` shell function in C
>  - bisect--helper: `is_expected_rev` & `check_expected_revs` shell function 
> in C
>  - bisect--helper: `bisect_reset` shell function in C
>  - wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
>  - t6030: explicitly test for bisection cleanup
>  - bisect--helper: `bisect_clean_state` shell function in C
>  - bisect--helper: `write_terms` shell function in C
>  - bisect: rewrite `check_term_format` shell function in C
>  - bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
>
>  GSoC "bisect" topic.
>
>  Are people happy with this version?  How much of the planned
>  "porting" is finished by this part of the work?  How much more to
>  go?

The current branch contains 30-40 % of the work. I am going to send 3
more functions check_and_set_terms(), bisect_next_check() and
bisect_terms() positively by today. After that the work will be around
50-60% complete. After that bisect_start() (I have ported this but it
has a bug right now) then bisect_next(), bisect_replay(),
bisect_state(), and a few small more.

Regards,
Pranit Bauva
--
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 5/6] Add porcelain V2 documentation to status manpage

[Jakub Narębski]

On 20 July 2016 at 17:42, Jeff Hostetler  wrote:
> On 07/20/2016 11:29 AM, Jakub Narębski wrote:
>> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:
>>
>>> +Porcelain Format Version 2
>>> +~~
>>> +
>>> +
>>> +If `--branch` is given, a header line showing branch tracking
>>> information
>>> +is printed.  This line begins with "### branch: ".  Fields are separated
>>> +by a single space.
>>> +
>>> +FieldMeaning
>>> +
>>> + | (initial)Current commit
>>> + | (detached)Current branch
>>
>> I was wondering if all possible combinations are allowed.  It turns out
>> that for technical implementation reasons it is not possible to have
>> "(initial) (detached)".
>>
>> Just something I was wondering about, no need for any change...
>
> Right. I don't think that combination is possible.  Not sure how
> to document that succinctly.

I don't think it is something we need to document, at least not here.

-- 
Jakub Narębski
--
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 2/6] Status and checkout unit tests for --porcelain[=]

[Jeff Hostetler]

On 07/20/2016 11:19 AM, Johannes Schindelin wrote:

Hi Jeff,

On Tue, 19 Jul 2016, Jeff Hostetler wrote:


Simple unit tests to validate the argument parsing.

Signed-off-by: Jeff Hostetler 


They are simple alright, but do we really need so many of them? I would
like to keep the ones in t7060, but I do not think that we necessarily
have to add the t7501 ones.

I know I am a bit at odds here with Junio, who frequently prefers more
tests. It's just that I have to run the complete test suite so often and
it does take 30-45 minutes to run here (due to the fact that the test
suite exercises quite a lot of the POSIX emulation layer via shell
scripting).

So do not take my suggestions as the sole basis for deciding how to go
from here.


I'm open to suggestion here.  I mainly wanted to be able to
prove that adding "=1" didn't affect the output and that an invalid
parameter throws.  We could eliminate several of the "more trivial"
ones if that would help.

Jeff

--
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 6/6] Unit tests for V2 porcelain status

[Jeff Hostetler]

On 07/20/2016 11:30 AM, Jakub Narębski wrote:

W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:

+test_expect_success pre_initial_commit_0 '
+   printf "## branch: (initial) master\n" >expected &&
+   printf "?? actual\n" >>expected &&
+   printf "?? dir1/\n" >>expected &&
+   printf "?? expected\n" >>expected &&
+   printf "?? file_x\n" >>expected &&
+   printf "?? file_y\n" >>expected &&
+   printf "?? file_z\n" >>expected &&


Why not use heredoc syntax (cat <<\EOF), or prepare a file
with expected output in the testsuite?



The tests involving renames needed to embed a tab character
in the output and hiding a tab character in a heredoc seemed
error prone.  So to be consistent I made them all printf-style.

Also, some of the tests include SHAs for the commit and for
file content, so having pre-computed expected output is awkward.
Granted we could hard code the file SHAs, but not the commits.

Jeff
--
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 5/6] Add porcelain V2 documentation to status manpage

[Jeff Hostetler]

On 07/20/2016 11:29 AM, Jakub Narębski wrote:

W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:


+Porcelain Format Version 2
+~~
+
+
+If `--branch` is given, a header line showing branch tracking information
+is printed.  This line begins with "### branch: ".  Fields are separated
+by a single space.
+
+FieldMeaning
+
+ | (initial)Current commit
+ | (detached)Current branch


I was wondering if all possible combinations are allowed.  It turns out
that for technical implementation reasons it is not possible to have
"(initial) (detached)".

Just something I was wondering about, no need for any change...



Right. I don't think that combination is possible.  Not sure how
to document that succinctly.

Jeff

--
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 1/6] Allow --porcelain[=] in status and commit commands

[Jeff Hostetler]

On 07/20/2016 11:08 AM, Johannes Schindelin wrote:

On Tue, 19 Jul 2016, Jeff Hostetler wrote:

diff --git a/builtin/commit.c b/builtin/commit.c
+   } else if (arg) {
+   int n = strtol(arg, NULL, 10);
+   if (n == 1)
+   *value = STATUS_FORMAT_PORCELAIN;
+   else
+   die("unsupported porcelain version");
+   } else {
+   *value = STATUS_FORMAT_PORCELAIN;


This could be folded into the previous conditional:

}
else {
int n = arg ? strtol(arg, NULL, 10) : 1;
...



I did it this way because I didn't want to make any assumptions
here on the numeric value of the enum values.  Or rather, I didn't
want to add specific assignments to the enum type.

This also helps make it easier to see my later commit:
else if (n == 2) *value = STATUS_FORMAT_PORCELAIN_V2;

Also, I didn't want to alter the order of assignments to the global
status_format variable.  That is, if I capture the value of  in
a porcelain_version variable and assign that to status_format
afterwards, there is opportunity for mistakes if they type:
git status --short --porcelain=1 --long
where the status_format variable is assigned 3 times.


Having said this, ...


@@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
N_("show status concisely"), STATUS_FORMAT_SHORT),
OPT_BOOL('b', "branch", _branch,
 N_("show branch information")),
-   OPT_SET_INT(0, "porcelain", _format,
-   N_("machine-readable output"),
-   STATUS_FORMAT_PORCELAIN),
+   { OPTION_CALLBACK, 0, "porcelain", _format,
+ N_("version"), N_("machine-readable output"),
+ PARSE_OPT_OPTARG, opt_parse_porcelain },


How about using a COUNTUP here instead? We could then set the status
format afterwards, like this:

if (porcelain == 0)
status_format = STATUS_FORMAT_UNSPECIFIED;
else {
status_format = STATUS_FORMAT_PORCELAIN;
if (porcelain > 1)
warning("No porcelain v%d; falling back to v1",
porcelain);
}



Maybe I misread the COUNTUP docs, but it looked like it would
allow "--porcelain --porcelain", but not "--porcelain=2".

Jeff
--
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 6/6] Unit tests for V2 porcelain status

[Jakub Narębski]

W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:
> +test_expect_success pre_initial_commit_0 '
> + printf "## branch: (initial) master\n" >expected &&
> + printf "?? actual\n" >>expected &&
> + printf "?? dir1/\n" >>expected &&
> + printf "?? expected\n" >>expected &&
> + printf "?? file_x\n" >>expected &&
> + printf "?? file_y\n" >>expected &&
> + printf "?? file_z\n" >>expected &&

Why not use heredoc syntax (cat <<\EOF), or prepare a file
with expected output in the testsuite?

-- 
Jakub Narębski
--
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 5/6] Add porcelain V2 documentation to status manpage

[Jakub Narębski]

W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:

> +Porcelain Format Version 2
> +~~
> +
> +
> +If `--branch` is given, a header line showing branch tracking information
> +is printed.  This line begins with "### branch: ".  Fields are separated
> +by a single space.
> +
> +FieldMeaning
> +
> + | (initial)Current commit
> + | (detached)Current branch

I was wondering if all possible combinations are allowed.  It turns out
that for technical implementation reasons it is not possible to have
"(initial) (detached)".

Just something I was wondering about, no need for any change...
-- 
Jakub Narębski
--
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 2/6] Status and checkout unit tests for --porcelain[=]

[Johannes Schindelin]

Hi Jeff,

On Tue, 19 Jul 2016, Jeff Hostetler wrote:

> Simple unit tests to validate the argument parsing.
> 
> Signed-off-by: Jeff Hostetler 

They are simple alright, but do we really need so many of them? I would
like to keep the ones in t7060, but I do not think that we necessarily
have to add the t7501 ones.

I know I am a bit at odds here with Junio, who frequently prefers more
tests. It's just that I have to run the complete test suite so often and
it does take 30-45 minutes to run here (due to the fact that the test
suite exercises quite a lot of the POSIX emulation layer via shell
scripting).

So do not take my suggestions as the sole basis for deciding how to go
from here.

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

Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands

[Johannes Schindelin]

Hi Jeff,

On Tue, 19 Jul 2016, Jeff Hostetler wrote:

> Update the --porcelain argument to take an optional
> version number.  This will allow us to define new
> porcelain formats in the future.
> 
> This default to 1 and represents the existing porcelain
> format.
> 
> Signed-off-by: Jeff Hostetler 

Very nice introductory patch.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 1f6dbcd..892d7f7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -142,14 +142,24 @@ static int show_ignored_in_status, have_option_m;
>  static const char *only_include_assumed;
>  static struct strbuf message = STRBUF_INIT;
>  
> -static enum status_format {
> - STATUS_FORMAT_NONE = 0,
> - STATUS_FORMAT_LONG,
> - STATUS_FORMAT_SHORT,
> - STATUS_FORMAT_PORCELAIN,
> +static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
>  
> - STATUS_FORMAT_UNSPECIFIED
> -} status_format = STATUS_FORMAT_UNSPECIFIED;
> +static int opt_parse_porcelain(const struct option *opt, const char *arg, 
> int unset)
> +{
> + enum wt_status_format *value = (enum wt_status_format *)opt->value;
> + if (unset) {
> + *value = STATUS_FORMAT_UNSPECIFIED;

In Git's source code, we skip the curly braces for one-liners.

> + } else if (arg) {
> + int n = strtol(arg, NULL, 10);
> + if (n == 1)
> + *value = STATUS_FORMAT_PORCELAIN;
> + else
> + die("unsupported porcelain version");
> + } else {
> + *value = STATUS_FORMAT_PORCELAIN;

This could be folded into the previous conditional:

}
else {
int n = arg ? strtol(arg, NULL, 10) : 1;
...

Having said this, ...

> @@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char 
> *prefix)
>   N_("show status concisely"), STATUS_FORMAT_SHORT),
>   OPT_BOOL('b', "branch", _branch,
>N_("show branch information")),
> - OPT_SET_INT(0, "porcelain", _format,
> - N_("machine-readable output"),
> - STATUS_FORMAT_PORCELAIN),
> + { OPTION_CALLBACK, 0, "porcelain", _format,
> +   N_("version"), N_("machine-readable output"),
> +   PARSE_OPT_OPTARG, opt_parse_porcelain },

How about using a COUNTUP here instead? We could then set the status
format afterwards, like this:

if (porcelain == 0)
status_format = STATUS_FORMAT_UNSPECIFIED;
else {
status_format = STATUS_FORMAT_PORCELAIN;
if (porcelain > 1)
warning("No porcelain v%d; falling back to v1",
porcelain);
}

The rest of the patch looks good to me!

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

Re: t7063 failure on FreeBSD 10.3 i386/amd64

[Duy Nguyen]

On Wed, Jul 20, 2016 at 5:02 AM, Eric Wong  wrote:
> Duy Nguyen  wrote:
>> On Tue, Jul 19, 2016 at 12:54 AM, Eric Wong  wrote:
>> > Oops, forgot to Cc some folks who worked on this :x
>> >
>> > Filesystem is ufs and it fails regardless of whether
>> > soft-updates is enabled or not.
>>
>> Nothing stands out to my eyes, so I'm going to install freebsd this
>> weekend. I hope ufs does not have any nasty surprise for me. Stay
>> tuned.
>
> Thanks, this problem might be ufs-specific, tmpfs is fine.
> Tested tmpfs with:
>
> kldload tmpfs
> mkdir /tmp/tmpfs
> mount -t tmpfs tmpfs /tmp/tmpfs
>
> (Documenting all this since much of this is new to me)

If you can, try with some other "real" filesystems, not virtual ones
like tmpfs, e.g. zfs or ext2 (not sure if it's supported on fbsd).
This is just to make sure I didn't hit a bug specific to fbsd. Don't
worry if you don't have time to do it. I'll get to it eventually.

> I noticed FreeBSD now provides ready-to-run VM images along with
> normal installation stuff, including qcow2 ones for QEMU users,
> so that saves some time.
>
> http://ftp.freebsd.org/pub/FreeBSD/releases/VM-IMAGES/10.3-RELEASE/amd64/Latest/FreeBSD-10.3-RELEASE-amd64.qcow2.xz

Yeah that's what I'm going to do, but not right 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] worktree: add: introduce the --name option

[Duy Nguyen]

On Tue, Jul 19, 2016 at 9:35 PM, Antoine Tenart  wrote:
> On Tue, Jul 19, 2016 at 09:04:11PM +0200, Duy Nguyen wrote:
>> On Tue, Jul 19, 2016 at 8:54 PM, Antoine Tenart  
>> wrote:
>> > On Tue, Jul 19, 2016 at 08:23:58PM +0200, Duy Nguyen wrote:
>> >> On Tue, Jul 19, 2016 at 8:04 PM, Junio C Hamano  wrote:
>> >>
>> >> 080739b (worktree.c: find_worktree() search by path suffix -
>> >> 2016-06-13) from 'next' should help identify worktrees in this case by
>> >> specifying 'project0/foo', 'project1/foo'... Granted it's not fun to
>> >> type all that when 'project0/foo' is something long, and bash
>> >> completion probably does not help much either.
>> >
>> > So with this I'll be able to create new worktrees, using paths having
>> > the same basename, but in different let's say "project directories"?
>>
>> Well, internal name is still out of your control, but if you want to
>> do something to a worktree you can say "do project0/foo". With 'next'
>> those verbs can be lock and unlock. We probably can make 'worktree
>> list' take filter and show just one worktree (and just add "git
>> worktree show" for that).
>
> Hmm, so if I understand correctly my use case still won't be supported,
> as adding a new worktree with the same basename will fail. Or did I miss
> something?

Hm... _what_ fails? If you create two worktrees project0/foo and
project1/foo, you'll get .git/worktrees/foo and .git/worktrees/foo1
but worktree creation should succeed both times. As long as you don't
have to look into .git/worktrees/ everything should be fine, you won't
see foo vs foo1.

If you absolutely have to, I'm thinking of "git --rev-parse
--worktree= --git-dir" (or something similar) that will give you
$GIT_DIR to a specific worktree (e.g. .git/worktrees/foo or
.git/worktrees/foo1). Then you can specify "git rev-parse
--worktree=project0/foo --git-dir" and still don't have to see foo vs
foo1.
-- 
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: Git and SHA-1 security (again)

[Duy Nguyen]

On Tue, Jul 19, 2016 at 8:58 PM, Herczeg Zsolt  wrote:
> 2016-07-19 20:04 GMT+02:00 Duy Nguyen :
>> On Tue, Jul 19, 2016 at 7:59 PM, David Lang  wrote:
>>> On Tue, 19 Jul 2016, Duy Nguyen wrote:
>>>
 On Tue, Jul 19, 2016 at 7:34 PM, David Lang  wrote:
>
> On Tue, 19 Jul 2016, Duy Nguyen wrote:
>
>> On Tue, Jul 19, 2016 at 9:18 AM, Johannes Schindelin
>>  wrote:


 But we can recreate SHA-1 from the same content and verify GPG, right?
 I know it's super expensive, but it feels safer to not carry SHA-1
 around when it's not secure anymore (I recall something about
 exploiting the weakest link when you have both sha1 and sha256 in the
 object content). Rehashing would be done locally and is better
 controlled.
>>>
>>>
>>>
>>> You could. But how would you determine whether to recreate the commit
>>> object from a SHA-1-ified version of the commit buffer? Fall back if
>>> the
>>> original did not match the signature?
>>
>>
>>
>> Any repo would have a cut point when they move to sha256 (or whatever
>> new hash), if we can record this somewhere (e.g. as a tag or a bunch
>> of tags, or some dummy commits to mark the heads of the repo) then we
>> only verify gpg signatures _in_ the repository before this point.
>
>
>
> remember that a repo doesn't have a single 'now', each branch has it's
> own
> head, and you can easily go back to prior points and branch off from
> there.
>
> Since timestamps in repos can't be trusted (different people's clocks may
> not be in sync), how would you define this cutoff point?


 The set of all heads at the time the conversion happens (maybe plus
 all the real tags). We can make an octopus merge commit to cover all
 the heads, then it can be the reference point.
>>>
>>>
>>> so to make sure I'm understanding this, anything not reachable from that
>>> merge must be the new hash, correct? Including forks, merges, etc that
>>> happen from earlier points in the history.
>>
>> Yes everything except that merge and everything reachable from it, the
>> whole old clone, basically.
>
> It could work, but does it worth it?
>
> 1) If you use multihash, you should assume that anything with SHA1
> could be manipulated. That means you can "inject" something later to
> that "old clone" anyway.

No it's not multihash. The repo only uses sha256, but by substituting
it with sha1 using the same dag, we can recreate the exact same sha1
repo (up to the conversion point). This is mostly to avoid people
injecting something because _you_ generate the repo locally.

> 2) Even if the content is re-hashed, it's hard to understand for a
> user where the trust comes from. The user should decide weather he
> trust (or not) the person who signed that octopus breakpoint.
>
> Even without git you can achieve this security: Get the complete old
> repository, make a signed tarball of it. If anytime later you want to
> check that signatures, you can just use that tarball. I don't think
> it's worth the trouble to create a native method for something which
> is rare, and can be worked around easily. It's actually easier for a
> user to understand the "trust relation" when using this workaround.
>
> Referring to that signed-tarball approach, you may just as well drop
> all signature data on conversion... As long as you can look up the
> references to old hashes easily, I think it's usable enough.

It's more or less the signed-tarball approach in my view, except that
you recreate that tarball dynamically with your sha256 repo (so this
tarball is "signed" with sha256).
-- 
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: Git and SHA-1 security (again)

[Duy Nguyen]

On Wed, Jul 20, 2016 at 2:28 PM, Johannes Schindelin
 wrote:
> But that strategy *still* ignores the distributed nature of Git. Just
> because *you* make that merge at a certain point does not necessarily mean
> that I make it at that point, too.
>
> Any approach that tries to have one single point of conversion will most
> likely fall short of a solution.

OK I see the difference in our views now. To me an sha256 repo would
see an sha1 repo as a _foreign_ DVCS, pretty much like git sees
mercurial now. So a transition from sha1 to sha256 is not that
different from cvs -> svn -> a dvcs bubble -> git.

> To be honest, I am less concerned about the GPG-signed commits (after all,
> after switching to a more secure hash algorithm, a maintainer could
> cross-sign all signed commits, or only the branch tips or tags, as new
> tags, to reinstitute trust).
>
> I am much more concerned about references to commits, both inside and
> outside the repository. That is, if I read anywhere on the internet about
> Git having added support for `git add --chmod=+x ` in 4e55ed3 (add:
> add --chmod=+x / --chmod=-x options, 2016-05-31), I want to find that
> commit by that reference.
>
> And I am of course concerned what should happen if a user wants to fetch
> from, or push to, a SHA-1-hashed remote repository into, or from, a
> SHA-256-hashed local one.

to follow the above, in my view, interaction with sha1 repos go
through some conversion bridges like what we have with hg and svn. I
don't know if we are going this route. It's certainly simpler and
people already have experiences (from previous migration) to prepare
for it.
-- 
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: How to generate feature branch statistics?

[Jakub Narębski]

W dniu 2016-07-20 o 10:05, Ernesto Maserati pisze:

> I assume that feature branches are not frequently enough merged into
> master. Because of that we discover bugs later than we could with a more
> continuous code integration. I don't want to discuss here whether feature
> branches are good or bad.
> 
> I want just to ask is there a way how to generate a statistic for the
> average duration of feature branches until they are merged to the master? I
> would like to know if it is 1 day, 2 days or lets say 8 or 17 days. Also it
> would be interesting to see the statistical outliers.
> 
> I hope my motivation became clear and what kind of git repository data I
> would like to produce.
> 
> Any ideas?

There are at least two tools to generate statistics about git repository,
namely Gitstat (https://sourceforge.net/projects/gitstat) and GitStats
(https://github.com/hoxu/gitstats), both generating repo statistics as
a web page. You can probably find more... but I don't know if any includes
the statistics you need.

I assume that you have some way of determining if the merge in 'master'
branch is a merge of a topic branch, or of long-lived graduation branch
(e.g. 'maint' or equivalent). To simplify the situation, I assume that
the only merges in master are merges of topic branches:

  git rev-list --min-parents=2 master | 
  while read merge_rev; do 

You might want to add "--grep=maint --invert-grep" or something like
that to exclude merges of 'maint' branch.

We can get date of merge (authordate with %ad/%at, or committerdate
with %cd/%ct), as an epoch (seconds since 1970 -- which is good for
comparing datetimes and getting the interval between two events)

 MERGE_DATE=$(git show -s --date=format:%s --pretty=%ad $merge_rev)

Assuming that topic branches are always merged using two-head merge
as a second parent (--first-parent ancestry for master in master branch
only), then we can get the first revision on a merged topic branch with

 FIRST_REV=$(git rev-list $merge_rev^2 ^$merge_rev^1 | tail -1)

We can extract the date from this revision in the same way

 FIRST_DATE=$(git show -s --pretty=%at $FIRST_REV)

Print the difference (here to standard output, you might want to write
to a file)

 echo $(expr $MERGE_DATE - $FIRST_DATE)

And finish the loop.

  done

Then pass the output to some histogramming or statistics tool... or use
a spreadsheet. Note the results are in seconds.

HTH (not checked much)
-- 
Jakub Narębski
--
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: Looking for help to understand external filter driver code

[Jeff King]

On Tue, Jul 19, 2016 at 02:33:09PM -0700, Junio C Hamano wrote:

> > Git writes --> 4 byte content length
> > Git writes --> content string
> > Git reads <-- 4 byte filtered content length
> > Git reads <-- filtered content
> 
> Do you really need to force the sender to know the length in
> advance?  Together with the sequential nature of the above exchange,
> i.e. the filter is forbidden from producing even a single byte of
> its output before reading everything Git feeds it, you are making it
> impossible to use filters that perform streaming conversion.

Another option: use pkt-lines with a flush packet to indicate
end-of-input. That allows arbitrary sized data, with streaming, and
reuses existing concepts from git. There is proportional overhead, but
it's only 4 bytes per 64k, which is a tiny percent.

It does make some implementations easier if they know the size ahead of
time, though, so if we are _sure_ that nobody will want streaming later,
it may not be a good tradeoff. If we do print a size ahead of time, the
"normal" thing in git would be to do so in base-10 ascii followed by a
newline (e.g., as found in "cat-file --batch", or fast-import's "data"
command).

-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: git-prompt.sh incompatible with non-basic global grep.patternType

[Jeff King]

On Mon, Jul 18, 2016 at 03:56:09PM -0700, Richard Soderberg wrote:

> ps. git log --basic-regexp does not fix the issue, as for unknown
> reasons (I'll start another thread) the command-line option doesn't
> override grep.patternType.

Dscho gave a fix for your immediate issue, but this "ps" definitely
seems like a bug to me. Command-line options should always take
precedence over config.

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

Re: [PATCH v2] fetch-pack: grow stateless RPC windows exponentially

[Jeff King]

On Tue, Jul 19, 2016 at 12:53:47PM -0700, Jonathan Nieder wrote:

> Junio C Hamano wrote:
> 
> > Even if it is conservative, I wonder if it is truly a good idea to
> > make it exponentially grow forever from that point of view.  Would
> > it give essentially the same result to you if we discard the patch
> > in question and just raise LARGE_FLUSH to 10k instead?
> 
> I don't think it would be essentially the same result.  As discussed
> before, unlike the bidi (ssh:// and git:// protocols) case, linear
> growth is expensive in the stateless-rpc (https://) case --- each
> round of negotiation requires re-sending the existing 'have's and
> requires the peer repeatedly processing this increasingly large list
> of 'have's.
> 
> For comparison, in the bidi case, linear growth of next_flush means
> sending a bounded number of 'have's per round and is quite sensible.
> 
> In the stateless-rpc case, linear growth means getting a bounded
> number of 'have's worth of benefit (new 'have's) in each round, in
> exchange for a linearly increasing cost (existing 'have's).  That is a
> high cost for limited benefit.  Exponential growth is a better deal.

This kind of reasoning would be great in the commit message (and if
possible, numbers showing empirical improvement). On reading it, I
understood the "what", but not why or to what extent the slower growth
is a problem.

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

Re: [PATCH v4] config: add conditional include

[Jeff King]

On Sun, Jul 17, 2016 at 10:15:55AM +0200, Johannes Schindelin wrote:

> FWIW I am slightly less worried about the conditional includes (it is
> already a horrible mess to figure out too-long include chains now, before
> having conditional includes, for example). I am slightly more worried
> about eventually needing to introduce support for something like
> 
>   [if-gitdir(...):section]
>   thisSettingIsConditional = ...
> 
> or even
> 
>   [if (worktree==...):section]
>   anotherConditional = ...
> 
> and then having two incompatible conditional constructs, one generic, the
> other one specific to [include].
> 
> In other words, if we already introduce a conditional construct, I'd
> rather have one that could easily be used for other conditions/sections
> when (and if) needed.

I had assumed we would resist introducing anything like that, simply
because of backwards compatibility issues with the syntax. But I admit
that was just an assumption in my head; future compatibility with
reality is not guaranteed. :)

-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