Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 11:12:25AM -0700, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> Cool! Thanks for working on this.
> >
> > Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
> > do that with a failing test suite.
> 
> Let's do 2.9.2 together, as this is not limited to GfW.
> 
> Taking Peff's suggestions into account, perhaps like the attached?

It looks good to me.

> It wasn't readily apparent to me why 2038 check worked, so I added a
> short paragraph at the end, but those who know the test helper well
> enough may find it redundant, in which case I am fine with removing
> it.

Definitely keep that paragraph. I am quite familiar with the test
helper and it was not the outcome I initially expected.

> +test_lazy_prereq 64BIT_TIME '
> + case "$(test-date show:iso 99)" in
> + *" -> 2038-"*)
> + # on this platform, unsigned long is 32-bit, i.e. not large 
> enough
> + false

I see you tightened up the match a little. TBH, I think we could
probably just match the whole output string, but I doubt there's much
chance of a false positive either way.

-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] Git v2.9.1

[Junio C Hamano]

On Tue, Jul 12, 2016 at 11:12 AM, Junio C Hamano  wrote:
>
> Let's do 2.9.2 together, as this is not limited to GfW.
>
> Taking Peff's suggestions into account, perhaps like the attached?

If I can get positive comments soon enough, I can do 2.9.2 early
tomorrow my time.
Or a reasonable counter-proposal (including "without Peff's
suggestion, because...")
would work well, too, for that.

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

[GSOC Update] Week 10

[Pranit Bauva]

= SUMMARY ==
My public git.git is available here[1]. I regularly keep pushing my work so
anyone interested can track me there. Feel free to participate in the
discussions going on PRs with my mentors. Your comments are valuable.


=== INTRODUCTION  ==
The purpose of this project is to convert the git-bisect utility which partly
exists in the form of shell scripts to C code so as to make it more portable.
I plan to do this by converting each function to C and then calling it from
git-bisect.sh so as to use the existing test suite to test the function which
is converted.

Mentors:
Christian Couder 
Lars Schneider 


== Updates =
Things which were done in this week:

 * I have converted check_and_set_terms(), bisect_next_check() and
   bisect_terms() and have also sent an RFC[7] to the
   mailing list for discussion which hasn't yet collected any comments from
   the list. Some discussion is happening on github and I will send out the
   patched when the dust settles.

 * I have converted bisect_start() but there is a bug which I am still working
   on.

= NEXT STEPS 
Things which would be done in the coming week:

 * Finish off bisect_start().

 * bisect_next() has become a top priority because it would then help
   converting bisect_auto_next() and then it can be called by other important
   functions like bisect_start().

 * Following that I will convert bisect_auto_start()

 * Then bisect_replay().

=== My Patches (GSoC project only) ==

 * check_term_format patch[5]. This is in pu branch. The topic is pb/bisect.

 * bisect_write patch[6]. This is the v3 in the series. This has some minor
   nits as provided by Lars and I will send out a re-roll soon. This is also
   in the pu branch and is applied on top of pb/bisect.

 * bisect_terms patch[7]. This is an RFC and open for discussions. It would
   be very helpful if you can review it over github[8] as my mentors have also
   provided some comments.

= Notification ==
I will be travelling to my university campus from 14th July to 15th July so
I won't be available. I will resume my work after that.

[1]: https://github.com/pranitbauva1997/git
[3]: https://github.com/pranitbauva1997/git/pull/17
[4]: http://thread.gmane.org/gmane.comp.version-control.git/297266
[5]: http://thread.gmane.org/gmane.comp.version-control.git/295518
[6]: http://thread.gmane.org/gmane.comp.version-control.git/298263
[7]: http://thread.gmane.org/gmane.comp.version-control.git/298279
[8]: https://github.com/pranitbauva1997/git/pull/16

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

windows 10 git 2.9.0 with vim 7.4.1023 display editing bug

[Jesse Zhuang]

I am using git 2.9.0.windows.1 in windows 10. With core.editor set to 
vim.

Vim was downloaded from vim.org latest version binary: VIM - Vi IMproved 
7.4.1023 (2013 Aug 10, compiled Jan 2 2016 14:24:35) MS-Windows 32-bit 
console version

When I do a git commit in a cmd window, it goes into a vim right in the 
command window.

But the display is incorrect, please check the first image in this album 
http://imgur.com/a/VSfuj​ with title "git2.9.0-vim7.4.1023-1".

Notice that

1, the scroll bar on the right is way too long than it should be.
2, after I pressed i to insert and inserted "test", notice how the four 
letters were kind positioned as a falling ladder. the behaviors of using 
hjkl keys to navigate the file is also incorrect.

The album also contains some other images with git versions spanning 
2.7.0, 2.8.0, 2.9.0. Just for your reference, there was an issue with 
text color incorrect with versions 2.7 and 2.8. The display issue was 
probably present in those earlier versions too but I am not 100% sure 
because of the black text on black screen issue. I want to focus on the 
bug with version 2.9.0.

Starting vim up from the command line is totally fine. If I use "git 
bash here", vim behaves totally fine.

Thanks in advance for everyone who will look into this bug.

Jesse

Jesse Zhuang
http://jessezhuang.github.io

[PATCH 6/9] 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 @@ next:
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;
+}
-- 
2.9.0

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

[PATCH 4/9] 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 bec63d6..3089433 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 

[PATCH 3/9] 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 3c748d1..bec63d6 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 

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

[Pranit Bauva]

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

-- 
2.9.0

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

[PATCH 9/9] 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 88b5d0a..88a1df8 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 5/9] 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).

Signed-off-by: Pranit Bauva 
---
 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
-- 
2.9.0

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

[PATCH 7/9] 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 3089433..636044a 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" || {
-   

[PATCH 2/9] 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..3c748d1 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)
+{
+   struct strbuf new_term = STRBUF_INIT;
+   strbuf_addf(_term, "refs/bisect/%s", term);
+
+   if (check_refname_format(new_term.buf, 0)) {
+   strbuf_release(_term);
+   return error(_("'%s' is not a valid term"), term);
+   }
+   strbuf_release(_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)
-  

[PATCH 1/9] 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;
 }
-- 
2.9.0

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

[PATCH 8/9] 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 636044a..88b5d0a 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.")" ;;
*)
-- 
2.9.0

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

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

[Philip Oakley]

From: "Junio C Hamano" 

Philip Oakley  writes:


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

The series has gained additional patches following the discussions
($gmane/298790).

..


The patches can be squashed together if required.


Looked mostly sensible, except for a few things mentioned in the
reviews by Marc (to which I mostly agree with).



Thanks. I'll update in the next few days.

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

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

[Philip Oakley]

From: "Junio C Hamano" 

Marc Branchaud  writes:


+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 the ones reachable from 'r1'.


All of these headings render poorly in the manpage, at least for me
(Ubuntu 16.04).  Only the first word appears in bold; the '-quoted
text is not bold but underlined, and the rest of the header is plain.


Also, I think calling this "The ^ notation" is confusing, because
there's already an earlier paragraph on the "^" syntax.

Maybe we don't need a header here?  I only suggest that because I'm
having trouble coming up with a nice alternative.  "Commit Exclusion"?


Thanks for pointing out the potential confusion between ^X (exclude
reachable), and X^ (the first parent).  Commit exclusion is probably
a good heading.


OK - I'll see about incorporating that.


-This set operation appears so often that there is a shorthand
+The '..' (two-dot) range notation
+~


Perhaps "Range notation", to mirror the capitalization of "Symmetric
Difference" in the next header?

...
+The '...' (three dot) Symmetric Difference notation


This uses a strange capitalization rule.  s/notation/Notation/
perhaps?  The same comment for "Additional Shothand notation" below.



I'd just capitalised the specific term. Will change.


+~~~
  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.
+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.


Unfortunately the new headings make it appear that this paragraph is
exclusively part of the '...' notation section.  Folks reading the
..' section are likely to skip it.

I like the examples, though.  I think it would be worthwhile to remove
this paragraph and fold it explicitly into the '..' and '...' notation
sections.


An alternative would be to have

   - Dotted range notations
 - Two-dot notation
 - Three-dot notation

which would help make it stand out that defaulting is common
characteristics between .. and ... notations.  But I can imagine
that your "with slight duplication" variant below would work well,
too.


I'll look into that.




So add something like this to the '..' section (only the first
sentence here is new):

Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  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.

And also, add the same first sentence and a different example to the
...' section.  Something like this:

Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  For example, 'origin...' is a shorthand for
'origin...HEAD' and asks "What have I and origin both done
since I forked from the origin branch?"  Note that 'origin...'
and '...origin' ask the same question.



+Additional '{caret}' Shorthand notations
+
  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.
+and its parent commits exist.


I think descriptions of ^@ and ^! should live under the main
description of ^.  That part already describes the numeric
suffix, so describing a couple of special suffixes there seems like a
natural fit.


I actually think this is a good place to have them described.
^ is about specifying a single commit.  These two are
not that (you can say HEAD^2^@ but you cannot say HEAD^@^2, for
example).


These two are special cases I'm not too familiar with, particularly the r1^! 
which I didn't undesrtand from the description...


--
Philip 


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

Re: What's cooking in git.git (Jun 2016, #05; Thu, 16)

[Joey Hess]

Torsten Bögershausen wrote re jh/clean-smudge-annex:
> The thing is that we need to check the file system to find .gitatttibutes,
> even if we just did it 1 nanosecond ago.
> 
> So the .gitattributes is done 3 times:
> -1 would_convert_to_git_filter_fd(
> -2 assert(would_convert_to_git_filter_fd(path));
> -3 convert.c/convert_to_git_filter_fd()
> 
> The only situation where this could be useful is when the .gitattributes
> change between -1 and -2,
> but then they would have changed between -1 and -3, and convert.c
> will die().
> 
> Does it make sense to remove -2 ?

There's less redundant work going on than at first seems, because 
.gitattribute files are only read once and cached. Verified by strace.

So, the redundant work is only in the processing that convert_attrs() does
of the cached .gitattributes.

Notice that there was a similar redundant triple call to convert_attrs()
before my patch set:

1. index_fd checks would_convert_to_git_filter_fd
2. index_stream_convert_blob does assert(would_convert_to_git_filter_fd(path))
   (Again redundantly since 1. is its only caller and has already
   checked.)
3. in convert_to_git_filter_fd

If convert_attrs() is somehow expensive, it might be worth passing a
struct conv_attrs * into the functions that currently call
convert_attrs(). But it does not look expensive to me.

I think it would be safe enough to remove both asserts, at least as the
code is structured now.

-- 
see shy jo


signature.asc
Description: PGP signature

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

[Jeff King]

On Tue, Jul 12, 2016 at 10:41:35PM +0100, Philip Oakley wrote:

> > > +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 the ones reachable from 'r1'.
> > 
> > All of these headings render poorly in the manpage, at least for me
> > (Ubuntu 16.04).  Only the first word appears in bold; the '-quoted text
> > is not bold but underlined, and the rest of the header is plain.
> 
> Which doc package is that with? It had formatted OK for the html web pages.

I get the same with:

  make gitrevisions.7
  man -l gitrevisions.7

Asciidoc 8.6.9, docbook-xsl 4.5 if it matters.

Rendering single-quotes as underline is normal in this case (though it's
not great for punctuation like this, as it kind of blends with the dots;
I know we use it elsewhere in this document, though).  The failure to
continue the bold through the end of line looks like a bug, though.

The generated XML (from asciidoc) looks reasonable:

  The .. (two-dot) range notation

The roff looks like:

  .SS "The \fI\&.\&.\fR (two\-dot) range notation"

The "\fR" switches us back to "Roman" from italics, which is presumably
the problem. We really want to say "switch back what we were using
before \fI".

Switching it to "\fP" fixes it, but it's not clear to me if that's
actually portable, or a groff-ism. I don't know roff very well and
documentation seems to be quite hard to find. So it's either a bug in
docbook, or an intentional decision they've made because roff can't
portably do better. I'm not sure which.

-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 v3 7/8] doc: revisions - define `reachable`

[Philip Oakley]

From: "Marc Branchaud" 

On 2016-07-11 04:25 PM, Philip Oakley wrote:

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 1c59e87..a3cd28b 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 `reachable` set of commits of the given
+commit.


Better as "... means the set of commits `reachable` from the given 
commit."


OK. The main aspect is show that 'reachable' is a specific term.




+
+A commit's reachable set is the commit itself and the commits of
+its ancestry chain.
+


s/of/in/


OK.



--
Philip 


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

Re: [PATCH v3 2/8] doc: revisions - name the Left and Right sides

[Philip Oakley]

From: "Junio C Hamano" 

Philip Oakley  writes:


The terms Left and Right side originate from the symmetric
difference. Name them there.
---


Sign-off?


Oops - will fix.



 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 19314e3..79f6d03 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.


I think it is a good idea to call them explicitly left and right,
but I do not think they need to be capitalized here or on the title
of the patch.


OK - can fix.



 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



--
Philip 


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

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

[Philip Oakley]

From: "Marc Branchaud" 

On 2016-07-11 04:25 PM, Philip Oakley wrote:

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 | 23 +--
  1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 79f6d03..1c59e87 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,46 @@ 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.

+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 the ones reachable from 'r1'.


All of these headings render poorly in the manpage, at least for me 
(Ubuntu 16.04).  Only the first word appears in bold; the '-quoted text is 
not bold but underlined, and the rest of the header is plain.


Which doc package is that with? It had formatted OK for the html web pages.




Also, I think calling this "The ^ notation" is confusing, because there's 
already an earlier paragraph on the "^" syntax.


Yes, I noticed that after sending. Maybe "^" (i.e. include the  
part) to show that its a prefix not a suffix




Maybe we don't need a header here?  I only suggest that because I'm having 
trouble coming up with a nice alternative.  "Commit Exclusion"?




Part of the earlier discussions as about avoiding new termininologies just 
for the sake of it.




-This set operation appears so often that there is a shorthand
+The '..' (two-dot) range notation
+~


Perhaps "Range notation", to mirror the capitalization of "Symmetric 
Difference" in the next header?



OK


+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 shorthands, you can omit one end and let it default to 
HEAD.
+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.


Unfortunately the new headings make it appear that this paragraph is 
exclusively part of the '...' notation section.  Folks reading the '..' 
section are likely to skip it.


OK



I like the examples, though.  I think it would be worthwhile to remove 
this paragraph and fold it explicitly into the '..' and '...' notation 
sections.


So add something like this to the '..' section (only the first sentence 
here is new):


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  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.

And also, add the same first sentence and a different example to the '...' 
section.  Something like this:


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  For example, 'origin...' is a shorthand for
'origin...HEAD' and asks "What have I and origin both done
since I forked from the origin branch?"  Note that 'origin...'
and '...origin' ask the same question.



+Additional '{caret}' Shorthand notations
+
  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.
+and its parent commits exist.


I think descriptions of ^@ and ^! should live under the main 
description 

Re: [PATCH v3 00/16] Use merge_recursive() directly in the builtin am

[Junio C Hamano]

Johannes Schindelin  writes:

> This is the second iteration of the long-awaited re-roll of the attempt to
> avoid spawning merge-recursive from the builtin am and use merge_recursive()
> directly instead.

This is actually the third iteration.

I am trying to tease dependencies apart and apply this on a more
reasonable base than a commit that happened to be at 'pu' on one
day, but this would probably take some time, and I may give up
merging it anywhere for today's integration cycle.  We'll see.





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

Re: [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> diff --git a/cache-tree.c b/cache-tree.c
> index c2676e8..2d50640 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
>   continue;
>   }
>  
> + /*
> +  * "sub" can be an empty tree if subentries are i-t-a.
> +  */
> + if (sub && sub->cache_tree->entry_count < 0 &&
> + !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
> + continue;
> +

Looks sensible, except that it is unclear if it is correct to assume
that "subentries are ita" always equals to "entry_count < 0" here.
I _think_ it is OK, as the function itself does use the latter as
the sign that it hit to_invalidate=1 case when it returns.

Thanks.

>   strbuf_grow(, entlen + 100);
>   strbuf_addf(, "%o %.*s%c", mode, entlen, path + baselen, 
> '\0');
>   strbuf_add(, sha1, 20);
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 24aed2e..f4b2fac 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -99,5 +99,19 @@ test_expect_success 'cache-tree does not ignore dir that 
> has i-t-a entries' '
>   )
>  '
>  
> +test_expect_success 'cache-tree does skip dir that becomes empty' '
> + rm -fr ita-in-dir &&
> + git init ita-in-dir &&
> + (
> + cd ita-in-dir &&
> + mkdir -p 1/2/3 &&
> + echo 4 >1/2/3/4 &&
> + git add -N 1/2/3/4 &&
> + git write-tree >actual &&
> + echo $_EMPTY_TREE >expected &&
> + test_cmp expected actual
> + )
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> This is a special SHA1. Let's keep it at one place, easier to replace
> later when the hash change comes, easier to recognize. Start with an
> underscore to reduce the chances somebody may override it without
> realizing it's predefined.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  t/t-basic.sh|  2 +-
>  t/t1100-commit-tree-options.sh  |  2 +-
>  t/t4010-diff-pathspec.sh|  6 ++
>  t/t4054-diff-bogus-tree.sh  | 10 --
>  t/t5504-fetch-receive-strict.sh |  4 ++--
>  t/test-lib.sh   |  4 +++-
>  6 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/t/t-basic.sh b/t/t-basic.sh
> index 60811a3..48214e9 100755
> --- a/t/t-basic.sh
> +++ b/t/t-basic.sh
> @@ -834,7 +834,7 @@ test_expect_success 'git write-tree should be able to 
> write an empty tree' '
>  '
>  
>  test_expect_success 'validate object ID of a known tree' '
> - test "$tree" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904
> + test "$tree" = $_EMPTY_TREE
>  '

I doubt the point of, and I'd rather not to see, the leading
underscore.  Are there existing uses of the name that want it to
mean something different?
--
To unsubscribe from this list: send the line "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/5] Number truncation with 4+ GB files on 32-bit systems

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Since I now could reproduce the problem that Christoph showed, I
> decided to send the good patches out. To sum up, we use "unsigned
> long" in some places related to file size. On 32-bit systems, it's
> limited to 32 bits even though the system can handle files larger than
> that (off_t is 64-bit). This fixes it.
>
> clang -Wshorten-64-to-32 is very helpful to spot these problems. I
> have a couple more patches to clean all these warnings, but some need
> more code study to see what is the right way to do.
>
> Most of the rest seems harmless, except for the local variable "size"
> in builtin/pack-objects.c:write_one(). I might send 6/5 for that one.

With 1-7/5 it appears t1050 is broken?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

[Junio C Hamano]

Torsten Bögershausen  writes:

>> How do things look at this point?  This version is what I ended up
>> queuing in 'pu', but I took your "Thanks" in $gmane/299120 to only
>> mean "Thanks for feeding some ideas to help me move forward", not
>> necessarily "Tnanks that looks like the right approach." yet, so
>> right now both topics are stalled and waiting for an action from
>> you.
> Yes, the code looks good to me.
> And the commit message does explain what is going on.
>
> For my taste, these 3 lines don't explain too much,may be remove them ?
>> The test update was taken from a series by Torsten Bögershausen
>> that attempted to fix this with a different approach (which was a
>> lot more intrusive).

OK. I wanted to make sure the resulting log message not only gives
you credit for the test portion of the change, but also for the fact
that you thought long and hard about the issue.  I'll tone it down
by removing "(which was...)" part.

> So thanks for your efforts, ack from my side.

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

Re: [PATCH v5 0/8] extend smudge/clean filters with direct file access (for pu)

[Junio C Hamano]

Joey Hess  writes:

> Since tb/convert-peek-in-index is not currently in pu, this reroll isn't
> based on it, and will conflict if that topic gets added back into pu.
> Not sure what the status of tb/convert-peek-in-index is at this point?

It appears that we are converging on _not_ using that topic after
all (cf. $gmane/299320).

I'll try to apply these on top of a merge between the 'cc/am-apply'
topic and the current 'master' branch and requeue.

> Improvements from Junio's review:
>
>   fix build with DEVELOPER=1
>   style fixes
>   use test_cmp in test cases
>   improve robustness of a test case
>   clean up some confusing code
>   small performance tweak
>
> Joey Hess (8):
>   clarify %f documentation
>   add smudgeToFile and cleanFromFile filter configs
>   use cleanFromFile in git add
>   use smudgeToFile in git checkout etc
>   warn on unusable smudgeToFile/cleanFromFile config
>   better recovery from failure of smudgeToFile filter
>   use smudgeToFile filter in git am
>   use smudgeToFile filter in recursive merge
>
>  Documentation/config.txt|  18 -
>  Documentation/gitattributes.txt |  42 
>  apply.c |  16 +
>  convert.c   | 148 
> 
>  convert.h   |  10 +++
>  entry.c |  59 
>  merge-recursive.c   |  53 +++---
>  sha1_file.c |  42 ++--
>  t/t0021-conversion.sh   | 117 +++
>  9 files changed, 459 insertions(+), 46 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files

[Christian Couder]

On Tue, Jul 12, 2016 at 5:12 PM, Duy Nguyen  wrote:
>
> No. People could create an index file anywhere in theory. So you don't
> know how many index files there are.

Maybe when an index file is created, its path and its sharedindex file
could be appended into a log file.
We could check this log file to see if we can remove sharedindex files.
We would need to remove the entries in this log file for the indexes
that are no longer there.

Or instead of one log file we could have a file for each index file in
a special directory called for example "indexinfo".
So we could just delete the file if its related index is no longer there.

> It really depends. If the shared part is too small for old indexes, we
> might as well unsplit them. In practice though, the only long-term
> index file is $GIT_DIR/index. If we don't delete old shared index
> files too close to their creation time, temp index files will go away.

We could treat $GIT_DIR/index specially so that if there are no temp
index files, there should be nothing in "indexinfo".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Philip Oakley  writes:

> This is the re-roll of the po/range-doc (2016-07-01) 3 commits and its
> follow on patch.
>
> The series has gained additional patches following the discussions
> ($gmane/298790).
>
> The original first 3 patches are unchanged, though 2/8 has been inserted
> to name the Left and Right ranges.
>
> The extra four 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.

Looked mostly sensible, except for a few things mentioned in the
reviews by Marc (to which I mostly agree with).

Thanks.

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

Re: [PATCH 4/5] index-pack: report correct bad object offsets even if they are large

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

>   va_end(params);
> - die(_("pack has bad object at offset %lu: %s"), offset, buf);
> + die(_("pack has bad object at offset %"PRIiMAX": %s"),
> + (intmax_t)offset, buf);

Subject: [PATCH] SQUASH???

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 73f7cd2..e2d8ae4 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -349,8 +349,8 @@ static NORETURN void bad_object(off_t offset, const char 
*format, ...)
va_start(params, format);
vsnprintf(buf, sizeof(buf), format, params);
va_end(params);
-   die(_("pack has bad object at offset %"PRIiMAX": %s"),
-   (intmax_t)offset, buf);
+   die(_("pack has bad object at offset %"PRIuMAX": %s"),
+   (uintmax_t)offset, buf);
 }
 
 static inline struct thread_local *get_thread_data(void)
-- 
2.9.1-500-g4c1e1e0

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

Re: [PATCH 2/5] sha1_file.c: use type off_t* for object_info->disk_sizep

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> - strbuf_addf(sb, "%lu", data->disk_size);
> + strbuf_addf(sb, "%"PRIuMAX, data->disk_size);

Subject: [PATCH] SQUASH???

---
 builtin/cat-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 58ad284..13ed944 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -184,7 +184,7 @@ static void expand_atom(struct strbuf *sb, const char 
*atom, int len,
if (data->mark_query)
data->info.disk_sizep = >disk_size;
else
-   strbuf_addf(sb, "%"PRIuMAX, data->disk_size);
+   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
} else if (is_atom("rest", atom, len)) {
if (data->mark_query)
data->split_on_whitespace = 1;
-- 
2.9.1-500-g4c1e1e0

--
To unsubscribe from this list: send the line "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] pack-objects: Use reachability bitmap index when generating non-stdout pack too

[Kirill Smelkov]

On Fri, Jul 08, 2016 at 01:38:55PM +0300, Kirill Smelkov wrote:
> Peff first of all thanks for feedback,
> 
> On Thu, Jul 07, 2016 at 04:52:23PM -0400, Jeff King wrote:
> > On Thu, Jul 07, 2016 at 10:09:17PM +0300, Kirill Smelkov wrote:
> > 
> > > Starting from 6b8fda2d (pack-objects: use bitmaps when packing objects)
> > > if a repository has bitmap index, pack-objects can nicely speedup
> > > "Counting objects" graph traversal phase. That however was done only for
> > > case when resultant pack is sent to stdout, not written into a file.
> > > 
> > > We can teach pack-objects to use bitmap index for initial object
> > > counting phase when generating resultant pack file too:
> > 
> > I'm not sure this is a good idea in general. When bitmaps are in use, we
> > cannot fill out the details in the object-packing list as thoroughly. In
> > particular:
> > 
> >   - we will not compute the same write order (which is based on
> > traversal order), leading to packs that have less efficient cache
> > characteristics
> 
> I agree the order can be not exactly the same. Still if original pack is
> packed well (with good recency order), while using bitmap we will tend
> to traverse it in close to original order.
> 
> Maybe I'm not completely right on this, but to me it looks to be the
> case because if objects in original pack are put there linearly sorted
> by recency order, and we use bitmap index to set of all reachable
> objects from a root, and then just _linearly_ gather all those objects
> from original pack by 1s in bitmap and put them in the same order into
> destination pack, the recency order won't be broken.
> 
> Or am I maybe misunderstanding something?
> 
> Please also see below:
> 
> >   - we don't learn about the filename of trees and blobs, which is going
> > to make the delta step much less efficient. This might be mitigated
> > by turning on the bitmap name-hash cache; I don't recall how much
> > detail pack-objects needs on the name (i.e., the full name versus
> > just the hash).
> 
> If I understand it right, it uses only uint32_t name hash while searching. 
> From
> pack-objects.{h,c} :
> 
>  8< 
> struct object_entry {
>   ...
>   uint32_t hash;  /* name hint hash */
> 
> 
> /*
>  * We search for deltas in a list sorted by type, by filename hash, and then
>  * by size, so that we see progressively smaller and smaller files.
>  * That's because we prefer deltas to be from the bigger file
>  * to the smaller -- deletes are potentially cheaper, but perhaps
>  * more importantly, the bigger file is likely the more recent
>  * one.  The deepest deltas are therefore the oldest objects which are
>  * less susceptible to be accessed often.
>  */
> static int type_size_sort(const void *_a, const void *_b)
> {
> const struct object_entry *a = *(struct object_entry **)_a;
> const struct object_entry *b = *(struct object_entry **)_b;
> 
> if (a->type > b->type)
> return -1;
> if (a->type < b->type) 
> return 1;
> if (a->hash > b->hash)
> return -1;
> if (a->hash < b->hash)
> return 1;
>   ...
>  8< 
> 
> Documentation/technical/pack-heuristics.txt also confirms this:
> 
>  8< 
> ...
>  The quote from the above linus should be rewritten a
> bit (wait for it):
> - first sort by type.  Different objects never delta with
>   each other.
> - then sort by filename/dirname.  hash of the basename
>   occupies the top BITS_PER_INT-DIR_BITS bits, and bottom
>   DIR_BITS are for the hash of leading path elements.
> 
> ...
> 
> If I might add, the trick is to make files that _might_ be similar be
> located close to each other in the hash buckets based on their file
> names.  It used to be that "foo/Makefile", "bar/baz/quux/Makefile" and
> "Makefile" all landed in the same bucket due to their common basename,
> "Makefile". However, now they land in "close" buckets.
> 
> The algorithm allows not just for the _same_ bucket, but for _close_
> buckets to be considered delta candidates.  The rationale is
> essentially that files, like Makefiles, often have very similar
> content no matter what directory they live in.
>  8< 
> 
> 
> So yes, exactly as you say with pack.writeBitmapHashCache=true (ae4f07fb) the
> delta-search heuristics is almost as efficient as with just raw filenames.
> 
> I can confirm this also via e.g. (with my patch applied) :
> 
>  8< 
> $ time echo 0186ac99 | git pack-objects --no-use-bitmap-index --revs 
> erp5pack-plain
> Counting objects: 627171, done.
> Compressing objects: 100% (176949/176949), done.
> 50570987560d481742af4a8083028c2322a0534a
> Writing objects: 100% (627171/627171), done.
> Total 627171 (delta 439404), reused 594820 (delta 410210)
> 
> real0m37.272s
> user0m33.648s
> sys 

Re: [PATCH 7/5] fsck: use streaming interface for large blobs in pack

[Junio C Hamano]

Junio C Hamano  writes:

>> +if (type != OBJ_BLOB || size < big_file_threshold) {
>> +data = unpack_entry(p, entries[i].offset, , );
>> +data_valid = 1;
>
> This codepath slurps the data in-core to hash and data is later
> freed, i.e. non-blob objects and small ones are handled as before.
>
>> +}
>> +
>> +if (data_valid && !data)
>>  err = error("cannot unpack %s from %s at offset 
>> %"PRIuMAX"",
>>  sha1_to_hex(entries[i].sha1), p->pack_name,
>>  (uintmax_t)entries[i].offset);
>
> Otherwise, we'd go to check_sha1_signature() with map==NULL.  And
> that is exactly what we want---map==NULL is the way we tell the
> function to use the streaming interface to check.
>
> Good.

But not quite correct yet ;-)

Here is what I'd propose to squash in.  Even though data_valid
protects the above if() condition from accessing an otherwise
uninitialized "data", the call to check_sha1_signature() we have
later will get noticed by the compiler that "data" is passed
uninitialized.

More importantly, uninitialized data passed may be non-NULL, in
which case it would not trigger the streaming interface.

 pack-check.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/pack-check.c b/pack-check.c
index 066..ac263fd 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -123,7 +123,14 @@ static int verify_packfile(struct packed_git *p,
type = unpack_object_header(p, w_curs, , );
unuse_pack(w_curs);
 
-   if (type != OBJ_BLOB || size < big_file_threshold) {
+   if (type == OBJ_BLOB && big_file_threshold <= size) {
+   /*
+* Let check_sha1_signature() to check it with
+* the streaming interface; no point slurping
+* the data in-core only to discard.
+*/
+   data = NULL;
+   } else {
data = unpack_entry(p, entries[i].offset, , );
data_valid = 1;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Number truncation with 4+ GB files on 32-bit systems

[Junio C Hamano]

Junio C Hamano  writes:

> Nguyễn Thái Ngọc Duy   writes:
>
>> Since I now could reproduce the problem that Christoph showed, I
>> decided to send the good patches out. To sum up, we use "unsigned
>> long" in some places related to file size. On 32-bit systems, it's
>> limited to 32 bits even though the system can handle files larger than
>> that (off_t is 64-bit). This fixes it.
>>
>> clang -Wshorten-64-to-32 is very helpful to spot these problems. I
>> have a couple more patches to clean all these warnings, but some need
>> more code study to see what is the right way to do.
>>
>> Most of the rest seems harmless, except for the local variable "size"
>> in builtin/pack-objects.c:write_one(). I might send 6/5 for that one.
>
>
> Thanks.

All looked nicely done.  I'll queue with a few SQUASH??? (please ack
or reject them) in between on 'pu' and push the result out later
today.

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

Re: [PATCH 7/5] fsck: use streaming interface for large blobs in pack

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> For this particular operation, not unpacking the blob and letting
> check_sha1_signature, which supports streaming interface, do the job
> is sufficient.

That reasoning is sound, I would think, but...

> We will call the callback function "fn" with NULL as "data". The only
> callback of this function is fsck_obj_buffer(), which does not touch
> "data" at all if it's a blob.

... this may be a bit too magical that the assumption needs to be
left in an in-code comment to warn people against temptation to
using "data" over there, not just where the type of verify_fn is
declared.  Essentially, you are changing the expectation from
existing functions of that type.

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  pack-check.c | 15 +--
>  pack.h   |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/pack-check.c b/pack-check.c
> index 1da89a4..066 100644
> --- a/pack-check.c
> +++ b/pack-check.c
> @@ -105,6 +105,8 @@ static int verify_packfile(struct packed_git *p,
>   void *data;
>   enum object_type type;
>   unsigned long size;
> + off_t curpos;
> + int data_valid = 0;
>  
>   if (p->index_version > 1) {
>   off_t offset = entries[i].offset;
> @@ -116,8 +118,17 @@ static int verify_packfile(struct packed_git *p,
>   sha1_to_hex(entries[i].sha1),
>   p->pack_name, (uintmax_t)offset);
>   }
> - data = unpack_entry(p, entries[i].offset, , );
> - if (!data)
> +
> + curpos = entries[i].offset;
> + type = unpack_object_header(p, w_curs, , );
> + unuse_pack(w_curs);

This made me wonder where this "unuse" comes from; w_curs's in-use
count is incremented by calling unpack_object_header() and we are
done with that access at this point, hence we have unuse here.

> + if (type != OBJ_BLOB || size < big_file_threshold) {
> + data = unpack_entry(p, entries[i].offset, , );
> + data_valid = 1;

This codepath slurps the data in-core to hash and data is later
freed, i.e. non-blob objects and small ones are handled as before.

> + }
> +
> + if (data_valid && !data)
>   err = error("cannot unpack %s from %s at offset 
> %"PRIuMAX"",
>   sha1_to_hex(entries[i].sha1), p->pack_name,
>   (uintmax_t)entries[i].offset);

Otherwise, we'd go to check_sha1_signature() with map==NULL.  And
that is exactly what we want---map==NULL is the way we tell the
function to use the streaming interface to check.

Good.

> diff --git a/pack.h b/pack.h
> index 3223f5a..0e77429 100644
> --- a/pack.h
> +++ b/pack.h
> @@ -74,6 +74,7 @@ struct pack_idx_entry {
>  
>  
>  struct progress;
> +/* Note, the data argument could be NULL if object type is blob */
>  typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned 
> long, void*, int*);
>  
>  extern const char *write_idx_file(const char *index_name, struct 
> pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, 
> const unsigned char *sha1);
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Duy Nguyen]

On Tue, Jul 12, 2016 at 3:46 PM, Jeff King  wrote:
> On Tue, Jul 12, 2016 at 03:31:00PM +0200, Andreas Schwab wrote:
>
>> Johannes Schindelin  writes:
>>
>> > Hi Andreas,
>> >
>> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
>> >
>> >> Johannes Schindelin  writes:
>> >>
>> >> >> PRIuMAX isn't compatible with time_t.
>> >> >
>> >> > That statement is wrong.
>> >>
>> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
>> >> (even if they happen to have the same representation).
>> >
>> > Sigh.
>> >
>> > So if it is wrong, what is right?
>>
>> The right thing is to add a cast, of course.
>
> In general, I think the right cast for time_t should be to (intmax_t),
> and the formatting string should be PRIdMAX (which, as an aside, needs
> an entry in git-compat-util.h).

Coincidentally, I have the same problem with unsigned long being
32-bit and have to switch to off_t in some places. Does anybody know
what a fallback in git-compat-util for PRIdMAX would look like? I
guess it's "lld"...
-- 
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 v14 00/21] index-helper/watchman

[Duy Nguyen]

Just thinking out loud. I've been thinking about this more about this.
After the move from signal-based to unix socket for communication, we
probably are better off with a simpler design than the shm-alike one
we have now.

What if we send everything over a socket or a pipe? Sending 500MB over
a unix socket takes 253ms, that's insignificant when operations on an
index that size usually take seconds. If we send everything over
socket/pipe, we can trust data integrity and don't have to verify,
even the trailing SHA-1 in shm file.

So, what I have in mind is this, at read index time, instead of open a
socket, we run a separate program and communicate via pipes. We can
exchange capabilities if needed, then the program sends the entire
current index, the list of updated files back (and/or the list of dirs
to invalidate). The design looks very much like a smudge/clean filter.

For people who don't want extra daemon, they can write a short script
that saves indexes somewhere in tmpfs, and talk to watchman or
something else. I haven't written this script, but I don't think it
takes long to write one. Windows folks have total freedom to implement
a daemon, a service or whatever and use this program as front end. How
the service talks to this program is totally up to them. For people
who want to centralize everything, they can have just one daemon and
have the script to talk to this daemon.

I can see that getting rid of file-based stuff simplifies some
patches. We can still provide a daemon to do more advanced stuff (or
to make it work out of the box). But it's not a hard requirement and
we probably don't need to include one right now. And I think it makes
it easier to test as well because we can just go with some fake file
monitor service instead of real watchman.
--
Duy

On Sun, Jul 3, 2016 at 9:57 AM, David Turner  wrote:
> This addresses comments on v13:
> removed unnecessary no_mmap ifdef
> add an ifdef in unix-socket
> OS X fix for select()
> test improvement
>
> Thanks to all for suggestions.
>
> David Turner (10):
>   pkt-line: add gentle version of packet_write
>   index-helper: log warnings
>   unpack-trees: preserve index extensions
>   watchman: add a config option to enable the extension
>   index-helper: kill mode
>   index-helper: don't run if already running
>   index-helper: autorun mode
>   index-helper: optionally automatically run
>   index-helper: indexhelper.exitAfter config
>   mailmap: use main email address for dturner
>
> Nguyễn Thái Ngọc Duy (11):
>   read-cache: allow to keep mmap'd memory after reading
>   unix-socket.c: add stub implementation when unix sockets are not
> supported
>   index-helper: new daemon for caching index and related stuff
>   index-helper: add --strict
>   daemonize(): set a flag before exiting the main process
>   index-helper: add --detach
>   read-cache: add watchman 'WAMA' extension
>   watchman: support watchman to reduce index refresh cost
>   index-helper: use watchman to avoid refreshing index with lstat()
>   update-index: enable/disable watchman support
>   trace: measure where the time is spent in the index-heavy operations
>
>  .gitignore   |   2 +
>  .mailmap |   1 +
>  Documentation/config.txt |  12 +
>  Documentation/git-index-helper.txt   |  86 +
>  Documentation/git-update-index.txt   |   6 +
>  Documentation/technical/index-format.txt |  22 ++
>  Makefile |  22 ++
>  builtin/gc.c |   2 +-
>  builtin/update-index.c   |  15 +
>  cache.h  |  25 +-
>  command-list.txt |   1 +
>  config.c |   5 +
>  configure.ac |   8 +
>  contrib/completion/git-completion.bash   |   1 +
>  daemon.c |   2 +-
>  diff-lib.c   |   4 +
>  dir.c|  25 +-
>  dir.h|   6 +
>  environment.c|   2 +
>  git-compat-util.h|   1 +
>  index-helper.c   | 469 +++
>  name-hash.c  |   2 +
>  pkt-line.c   |  18 ++
>  pkt-line.h   |   2 +
>  preload-index.c  |   2 +
>  read-cache.c | 531 
> ++-
>  refs/files-backend.c |   2 +
>  setup.c  |   4 +-
>  t/t1701-watchman-extension.sh|  37 +++
>  t/t7063-status-untracked-cache.sh|  22 ++
>  t/t7900-index-helper.sh  |  79 +
>  t/test-lib-functions.sh  |   4 +
>  test-dump-watchman.c |  16 +
>  unix-socket.h   

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Jeff King  writes:

> In case it wasn't clear, I was mostly guessing there. So I dug a bit
> further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> on i386 because of the ABI headaches.

This is being worked on.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Junio C Hamano]

Johannes Schindelin  writes:

>> Cool! Thanks for working on this.
>
> Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
> do that with a failing test suite.

Let's do 2.9.2 together, as this is not limited to GfW.

Taking Peff's suggestions into account, perhaps like the attached?

It wasn't readily apparent to me why 2038 check worked, so I added a
short paragraph at the end, but those who know the test helper well
enough may find it redundant, in which case I am fine with removing
it.

Thanks.

-- >8 --
From: Johannes Schindelin 
Date: Tue, 12 Jul 2016 13:25:20 +0200
Subject: t0006: skip "far in the future" test when ulong is not long enough

Git's source code refers to timestamps as unsigned longs.  On 32-bit
platforms, as well as on Windows, unsigned long is not large enough
to capture dates that are "absurdly far in the future".

While we can fix this issue properly by replacing unsigned long with
a larger type, we want to be a bit more conservative and just skip
those tests on the maint track.

The test helper reads the timestamp given from the command line into
"unsigned long" using strtol(), which gives us LONG_MAX upon
overflow, so feeding 99 and seeing the resulting "timestamp"
shown in year 2038 is a sufficient check for that condition.

Signed-off-by: Johannes Schindelin 
Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
---
 t/t0006-date.sh | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 04ce535..6070c29 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -31,7 +31,7 @@ check_show () {
format=$1
time=$2
expect=$3
-   test_expect_${4:-success} "show date ($format:$time)" '
+   test_expect_success $4 "show date ($format:$time)" '
echo "$time -> $expect" >expect &&
test-date show:$format "$time" >actual &&
test_cmp expect actual
@@ -48,10 +48,22 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '146600 +0200'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +'
 
+test_lazy_prereq 64BIT_TIME '
+   case "$(test-date show:iso 99)" in
+   *" -> 2038-"*)
+   # on this platform, unsigned long is 32-bit, i.e. not large 
enough
+   false
+   ;;
+   *)
+   true
+   ;;
+   esac
+'
+
 # arbitrary time absurdly far in the future
 FUTURE="5758122296 -0400"
-check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
-check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +"
+check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400" 64BIT_TIME
+check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +" 64BIT_TIME
 
 check_parse() {
echo "$1 -> $2" >expect
-- 
2.9.1-500-g4c1e1e0



--
To unsubscribe from this list: send the line "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 4/5] index-pack: report correct bad object offsets even if they are large

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Use the right type for offsets in this case, off_t, which makes a
> difference on 32-bit systems with large file support, and change
> formatting code accordingly.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  builtin/index-pack.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index cafaab7..73f7cd2 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -338,10 +338,10 @@ static void parse_pack_header(void)
>   use(sizeof(struct pack_header));
>  }
>  
> -static NORETURN void bad_object(unsigned long offset, const char *format,
> +static NORETURN void bad_object(off_t offset, const char *format,
>  ...) __attribute__((format (printf, 2, 3)));
>  
> -static NORETURN void bad_object(unsigned long offset, const char *format, 
> ...)
> +static NORETURN void bad_object(off_t offset, const char *format, ...)
>  {
>   va_list params;
>   char buf[1024];
> @@ -349,7 +349,8 @@ static NORETURN void bad_object(unsigned long offset, 
> const char *format, ...)
>   va_start(params, format);
>   vsnprintf(buf, sizeof(buf), format, params);
>   va_end(params);
> - die(_("pack has bad object at offset %lu: %s"), offset, buf);
> + die(_("pack has bad object at offset %"PRIiMAX": %s"),
> + (intmax_t)offset, buf);
>  }

We seem to have a fallback definition only for PRIuMAX.

off_t is supposed to be a signed integer type [*1*], but we know
this is a positive offset when we issue this warning, so PRIuMAX
would probably be fine.


[Reference]

*1* http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.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 2/5] sha1_file.c: use type off_t* for object_info->disk_sizep

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> This field, filled by sha1_object_info() contains the on-disk size of
> an object, which could go over 4GB limit of unsigned long on 32-bit
> systems. Use off_t for it instead and update all callers.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  builtin/cat-file.c | 4 ++--
>  cache.h| 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 618103f..5b34bd0 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -131,7 +131,7 @@ struct expand_data {
>   unsigned char sha1[20];
>   enum object_type type;
>   unsigned long size;
> - unsigned long disk_size;
> + off_t disk_size;
>   const char *rest;
>   unsigned char delta_base_sha1[20];
>  
> @@ -191,7 +191,7 @@ static void expand_atom(struct strbuf *sb, const char 
> *atom, int len,
>   if (data->mark_query)
>   data->info.disk_sizep = >disk_size;
>   else
> - strbuf_addf(sb, "%lu", data->disk_size);
> + strbuf_addf(sb, "%"PRIuMAX, data->disk_size);

Doesn't this now need a cast?

>   } else if (is_atom("rest", atom, len)) {
>   if (data->mark_query)
>   data->split_on_whitespace = 1;
> diff --git a/cache.h b/cache.h
> index c73becb..a4465cb 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1508,7 +1508,7 @@ struct object_info {
>   /* Request */
>   enum object_type *typep;
>   unsigned long *sizep;
> - unsigned long *disk_sizep;
> + off_t *disk_sizep;
>   unsigned char *delta_base_sha1;
>   struct strbuf *typename;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] pack-objects: pass length to check_pack_crc() without truncation

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 8f5e358..df6ecd5 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -349,7 +349,7 @@ static unsigned long write_reuse_object(struct sha1file 
> *f, struct object_entry
>   struct revindex_entry *revidx;
>   off_t offset;
>   enum object_type type = entry->type;
> - unsigned long datalen;
> + off_t datalen;
>   unsigned char header[10], dheader[10];
>   unsigned hdrlen;
>  
> diff --git a/sha1_file.c b/sha1_file.c
> index d5e1121..cb571ac 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2281,7 +2281,7 @@ void *unpack_entry(struct packed_git *p, off_t 
> obj_offset,
>  
>   if (do_check_packed_object_crc && p->index_version > 1) {
>   struct revindex_entry *revidx = find_pack_revindex(p, 
> obj_offset);
> - unsigned long len = revidx[1].offset - obj_offset;
> + off_t len = revidx[1].offset - obj_offset;
>   if (check_pack_crc(p, _curs, obj_offset, len, 
> revidx->nr)) 

It is sad that check_pack_crc() already knew to take off_t here and
we (unknowingly) had a deliberate truncation by using ulong.
Looks obvioiusly correct.

Thanks.

{
>   const unsigned char *sha1 =
>   nth_packed_object_sha1(p, revidx->nr);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Number truncation with 4+ GB files on 32-bit systems

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Since I now could reproduce the problem that Christoph showed, I
> decided to send the good patches out. To sum up, we use "unsigned
> long" in some places related to file size. On 32-bit systems, it's
> limited to 32 bits even though the system can handle files larger than
> that (off_t is 64-bit). This fixes it.
>
> clang -Wshorten-64-to-32 is very helpful to spot these problems. I
> have a couple more patches to clean all these warnings, but some need
> more code study to see what is the right way to do.
>
> Most of the rest seems harmless, except for the local variable "size"
> in builtin/pack-objects.c:write_one(). I might send 6/5 for that one.


Thanks.

> Nguyễn Thái Ngọc Duy (5):
>   pack-objects: pass length to check_pack_crc() without truncation
>   sha1_file.c: use type off_t* for object_info->disk_sizep
>   index-pack: correct "len" type in unpack_data()
>   index-pack: report correct bad object offsets even if they are large
>   index-pack: correct "offset" type in unpack_entry_data()
>
>  builtin/cat-file.c |  4 ++--
>  builtin/index-pack.c   | 23 ---
>  builtin/pack-objects.c |  2 +-
>  cache.h|  2 +-
>  sha1_file.c|  2 +-
>  5 files changed, 17 insertions(+), 16 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Marc Branchaud  writes:

>> +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 the ones reachable from 'r1'.
>
> All of these headings render poorly in the manpage, at least for me
> (Ubuntu 16.04).  Only the first word appears in bold; the '-quoted
> text is not bold but underlined, and the rest of the header is plain.
>
>
> Also, I think calling this "The ^ notation" is confusing, because
> there's already an earlier paragraph on the "^" syntax.
>
> Maybe we don't need a header here?  I only suggest that because I'm
> having trouble coming up with a nice alternative.  "Commit Exclusion"?

Thanks for pointing out the potential confusion between ^X (exclude
reachable), and X^ (the first parent).  Commit exclusion is probably
a good heading.

>> -This set operation appears so often that there is a shorthand
>> +The '..' (two-dot) range notation
>> +~
>
> Perhaps "Range notation", to mirror the capitalization of "Symmetric
> Difference" in the next header?
>> ...
>> +The '...' (three dot) Symmetric Difference notation

This uses a strange capitalization rule.  s/notation/Notation/
perhaps?  The same comment for "Additional Shothand notation" below.

>> +~~~
>>   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.
>> +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.
>
> Unfortunately the new headings make it appear that this paragraph is
> exclusively part of the '...' notation section.  Folks reading the
> ..' section are likely to skip it.
>
> I like the examples, though.  I think it would be worthwhile to remove
> this paragraph and fold it explicitly into the '..' and '...' notation
> sections.

An alternative would be to have

- Dotted range notations
  - Two-dot notation
  - Three-dot notation

which would help make it stand out that defaulting is common
characteristics between .. and ... notations.  But I can imagine
that your "with slight duplication" variant below would work well,
too.

> So add something like this to the '..' section (only the first
> sentence here is new):
>
>   Either r1 or r2 can be omitted, in which case HEAD is used as
>   the default.  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.
>
> And also, add the same first sentence and a different example to the
> ...' section.  Something like this:
>
>   Either r1 or r2 can be omitted, in which case HEAD is used as
>   the default.  For example, 'origin...' is a shorthand for
>   'origin...HEAD' and asks "What have I and origin both done
>   since I forked from the origin branch?"  Note that 'origin...'
>   and '...origin' ask the same question.

>> +Additional '{caret}' Shorthand notations
>> +
>>   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.
>> +and its parent commits exist.
>
> I think descriptions of ^@ and ^! should live under the main
> description of ^.  That part already describes the numeric
> suffix, so describing a couple of special suffixes there seems like a
> natural fit.

I actually think this is a good place to have them described.
^ is about specifying a single commit.  These two are
not that (you can say HEAD^2^@ but you cannot say HEAD^@^2, for
example).

--
To unsubscribe from this list: send the line "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: Submodule's .git file contains absolute path when created using 'git clone --recursive'

[Stefan Beller]

On Tue, Jul 12, 2016 at 4:22 AM, Ricardo Sánchez-Sáez
 wrote:
> Stefan Beller  google.com> writes:
>
> Hi,
>
> sorry to awake an old thread. Has this been fixed? In which git version?
> It's hitting me on git version  2.7.4 (Apple Git-66) (default git client on
>  OS X 10.11.5 (15F34)).
>
> I think all submodule .git files should contain relative paths. Otherwise,
>  duplicating or moving the cloned  repository folder breaks the submodules.
>
> Best,
> Ricardo
>

The commit I referenced earlier:

$ git tag --contains f8eaa0ba98b3bd9cb9035eba184a2d9806d30b27
v2.8.3
v2.8.4
v2.9.0
v2.9.0-rc0
v2.9.0-rc1
v2.9.0-rc2
v2.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/8] doc: revisions - name the Left and Right sides

[Junio C Hamano]

Philip Oakley  writes:

> The terms Left and Right side originate from the symmetric
> difference. Name them there.
> ---

Sign-off?

>  Documentation/revisions.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index 19314e3..79f6d03 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.

I think it is a good idea to call them explicitly left and right,
but I do not think they need to be capitalized here or on the title
of the patch.

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

[PATCH v3] config: add conditional include

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

Helped-by: Jeff King 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 v3 fixes some memory leak and typos. Most importantly it no longer
 depends on core.ignorecase for case-insensitive matching. The choice
 must be explicitly made by the user when they choose "gitdir" or
 "gitdir/i" keyword.

 Documentation/config.txt  |  48 ++
 config.c  | 102 +-
 t/t1305-config-include.sh |  56 +
 3 files changed, 204 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index db05dec..18623ee 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -91,6 +91,43 @@ found at the location of the include directive. If the value 
of the
 relative to the configuration file in which the include directive was
 found.  See below for examples.
 
+Included files can be grouped into subsections where the subsection
+name is the condition that must be met for the files to be included.
+The condition starts with a keyword, followed by a colon and a
+pattern. The interpretation of the pattern depends on the keyword.
+Supported keywords are:
+
+`gitdir`::
+   The environment variable `GIT_DIR` must match the following
+   pattern for files to be included. The pattern can contain
+   standard globbing wildcards and two additional ones, `**/` and
+   `/**`, that can match multiple path components. Please refer
+   to linkgit:gitignore[5] for details. For convenience:
+
+ * If the pattern starts with `~/`, `~` will be substituted with the
+   content of the environment variable `HOME`.
+
+ * If the pattern starts with `./`, it is replaced with the directory
+   containing the current config file.
+
+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`
+   will be automatically prepended. For example, the pattern `foo/bar`
+   becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.
+
+ * If the pattern ends with `/`, `**` will be automatically added. For
+   example, the pattern `foo/` becomes `foo/**`. In other words, it
+   matches "foo" and everything inside, recursively.
+
+`gitdir/i`::
+   This is the same as `gitdir` except that matching is done
+   case-insensitively (e.g. on case-insensitive file sytems)
+
+A few more notes on matching:
+
+ * Symlinks in `$GIT_DIR` are not resolved before matching.
+
+ * Note that "../" is not special and will match literally, which is
+   unlikely what you want.
 
 Example
 ~~~
@@ -119,6 +156,17 @@ Example
path = foo ; expand "foo" relative to the current file
path = ~/foo ; expand "foo" in your `$HOME` directory
 
+   ; include if $GIT_DIR is /path/to/foo/.git
+   [include "gitdir:/path/to/foo/.git"]
+   path = /path/to/foo.inc
+
+   ; include for all repositories inside /path/to/group
+   [include "gitdir:/path/to/group/"]
+   path = /path/to/foo.inc
+
+   ; include for all repositories inside $HOME/to/group
+   [include "gitdir:~/to/group/"]
+   path = /path/to/foo.inc
 
 Values
 ~~
diff --git a/config.c b/config.c
index bea937e..ff44e00 100644
--- a/config.c
+++ b/config.c
@@ -13,6 +13,7 @@
 #include "hashmap.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "dir.h"
 
 struct config_source {
struct config_source *prev;
@@ -168,9 +169,102 @@ static int handle_path_include(const char *path, struct 
config_include_data *inc
return ret;
 }
 
+static int prepare_include_condition_pattern(struct strbuf *pat)
+{
+   int prefix = 0;
+
+   /* TODO: maybe support ~user/ too */
+   if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
+   struct strbuf path = STRBUF_INIT;
+   const char *home = getenv("HOME");
+
+   if (!home)
+   return error(_("$HOME is not defined"));
+
+   strbuf_add_absolute_path(, home);
+   strbuf_splice(pat, 0, 1, path.buf, path.len);
+   prefix = path.len + 1 /*slash*/;
+   strbuf_release();
+   } else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
+   struct strbuf path = STRBUF_INIT;
+   const char *slash;
+
+   if (!cf || !cf->path)
+   return error(_("relative config include "
+  "conditionals must come from files"));
+
+   strbuf_add_absolute_path(, cf->path);
+   slash = find_last_dir_sep(path.buf);
+   if (!slash)
+   die("BUG: how is this possible?");
+   strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);
+   prefix = slash - path.buf + 1 /* slash */;
+   strbuf_release();
+   } else if (!is_absolute_path(pat->buf))
+   strbuf_insert(pat, 0, "**/", 3);
+
+   if (pat->len && 

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King  writes:

> In case it wasn't clear, I was mostly guessing there. So I dug a bit
> further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> on i386 because of the ABI headaches.

X-< (yes, I knew).

> That being said, I still think the "clamp to time_t" strategy is
> reasonable. Unless you are doing something really exotic like pretending
> to be from the future, nobody will care for 20 years.

Yup.  It is a minor regression for them to go from ulong to time_t,
because they didn't have to care for 90 years or so but now they do
in 20 years, I'd guess, but hopefully after that many years,
everybody's time_t would be sufficiently large.

I suspect Cobol programmers in the 50s would have said a similar
thing about the y2k timebomb they created back then, though ;-)

> And at that point, systems with a 32-bit time_t are going to have
> to do _something_, because time() is going to start returning
> bogus values. So as long as we behave reasonably (e.g., clamping
> values and not generating wrapped nonsense), I think that's a fine
> solution.

OK.
--
To unsubscribe from this list: send the line "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] Add very verbose porcelain output to status

[Jeff Hostetler]

Thanks for the feedback.  I like the overall direction of your
suggestions.  Going for a porcelain V2 feels better than piling
onto verbose.  I think that would also give us a little more license
to address some of the line format issues that you point out.
Let me retool what I have along these lines and see how it looks.

Thank again,
Jeff




On 07/12/2016 11:07 AM, Jeff King wrote:

On Thu, Jul 07, 2016 at 03:26:28PM -0400, Jeff Hostetler wrote:


Tools interacting with Git repositories may need to know the complete
state of the working directory. For efficiency, it would be good to have
a single command to obtain this information.

We already have a `--porcelain` mode intended for tools' consumption,
and it only makes sense to enhance this mode to offer more information.

Just like we do elsewhere in Git's source code, we now interpret
multiple `--verbose` flags accumulatively, and show substantially more
information in porcelain mode at verbosity level 2.


Using "--verbose" to change the stable output feels a little funny to
me. Usually --verbose is about increasing the human-readable output to
stderr. Of course "--verbose" for git-status is already a little weird.
A single "-v" seems to do nothing, but two will turn on a diff for the
long-format output.

But it seems to me this just confuses things more. Why does "git status
--porcelain -v" do nothing, for example? What happens when we want to
add more information to the porcelain output later? Do we have to
require "-vvv" to trigger it, and if so, is there a way to get that
information without the "-vv" information?

I think the existing example for adding to the --porcelain format is
the --branch option. Could we have "--state" or something to trigger
the extra lines?


+If --branch is given, the first line shows a summary of the current
+operation in progress.  This line begins with "### state: ", the name
+of the operation in progress, and then operation-specific information.
+Fields are separated by a single space.


This seems like a good bit of information to have, and AFAIK we don't
expose the state information in a machine-readable way. A few nits:


+Operation   Fields Explanation
+--
+clean


Using "clean" here seems weird, as that term is usually meant to mean
the opposite of dirty (i.e., there is something in your working tree
that can be committed). But there is no "dirty" here. Would "normal" or
"none" be a good description?


+--
+merge  Number unmerged


This  (and others below) is redundant with the rest of the output,
right (i.e., you could count up the "U" entries yourself)? I don't mind
it as a convenience, but I'm kind of curious of why it's useful.


+--
+am  [E]Present if the current patch
+   is empty
+--
+rebase Number unmerged
+[S]Present if split commit in
+   progress during rebase
+[E]Present if editing a commit
+   during rebase


Can a rebased commit be empty? If so, should that state get "E" to match
"am"?

Does editing differentiate between "stopped because of a conflict that
needs addressing" versus "stopped because the interactive insn sheet
told us to"?


+[I(/)]Present if in an interactive
+   rebase. Step counts are given.
+[:] Rebase branches


I wonder if we ought to just use " " here, as separate
arguments. A ":" between refs usually signifies a refspec, but that is
not what this is.

I note this is optional, though. Is there a case where we might have one
but not the other? That would make things more difficult syntactically.


+If --branch is given, the second line shows branch tracking information.
+This line begins with "### track: ".  Fields are separated by a single
+space, unless otherwise indicated.
+
+FieldMeaning
+
+ | "(initial)"  Current commit
+ | "(detached)"  Current branch
+":"Upstream branch, if set
+"+"   Ahead count, if upstream present
+"-"  Behind count, if upstream present
+


This really _ought_ to have been how --branch worked in the first place
with --porcelain. But unfortunately the existing format has been in the
wild for a long time. To top it off, the existing format is translated!
So you cannot even 

Re: gc and repack ignore .git/*HEAD when checking reachability

[Duy Nguyen]

On Tue, Jul 12, 2016 at 5:51 PM, Jeff King  wrote:
> On Tue, Jul 12, 2016 at 05:46:12PM +0200, Duy Nguyen wrote:
>
>> I'm not opposed to letting one worktree see everything, but this move
>> makes it harder to write new scripts (or new builtin commands, even)
>> that works with both single and multiple worktrees because you refer
>> to one ref (in current worktree perspective) differently. If we kill
>> of the main worktree (i.e. git init always creates a linked worktree)
>> then it's less of a problem, but still a nuisance to write
>> refs/worktree/$CURRENT/ everywhere.
>
> True. I gave a suggestion for the reading side, but the writing side
> would still remain tedious.
>
> I wonder if, in a worktree, we could simply convert requests to read or
> write names that do not begin with "refs/" as "refs/worktree/$CURRENT/"?
> That makes it a read/write-time alias conversion, but the actual storage
> is just vanilla (so the ref storage doesn't need to care, and
> reachability just works).

A conversion like that is already happening, but it works at
git_path() level instead and maps anything outside refs/ to
worktrees/$CURRENT. Reorganizing all refs in a single ref storage is
probably possible, but...

> The trickiest thing, I think, is FETCH_HEAD, which is not really a
> ref (because it may have a bunch of values, and contain extra
> information).

Yeah.. I think David and Junio touched this when lmdb backend was
discussed, which resulted in leaving per-worktree refs to filesystem
backend even when shared refs are in lmdb. We probably can still make
it work, I think refs subsystem has special case for FETCH_HEAD
already. But 'refs' stuff is really not my area, I should stop writing
now before making too many wrong statements.
-- 
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: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 08:41:42AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I am not certain that there is a modern system with 32-bit time_t. We
> > know there are systems with 32-bit unsigned long, and I think that is
> > what produced the results people saw. I'd expect even 32-bit systems to
> > use "int64_t" or similar for their time_t these days.
> 
> OK.

In case it wasn't clear, I was mostly guessing there. So I dug a bit
further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
on i386 because of the ABI headaches.

That being said, I still think the "clamp to time_t" strategy is
reasonable. Unless you are doing something really exotic like pretending
to be from the future, nobody will care for 20 years. And at that point,
systems with a 32-bit time_t are going to have to do _something_,
because time() is going to start returning bogus values. So as long as
we behave reasonably (e.g., clamping values and not generating wrapped
nonsense), I think that's a fine solution.

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

Re: [RFC/PATCH 0/8] Add configuration options for split-index

[Duy Nguyen]

On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder
 wrote:
> Future work
> ~~~
>
> One thing that is probably missing is a mechanism to avoid having too
> many changes accumulating in the (split) index while in split index
> mode. The git-update-index documentation says:
>
> If split-index mode is already enabled and `--split-index` is
> given again, all changes in $GIT_DIR/index are pushed back to
> the shared index file.
>
> but it is probably better to not expect the user to think about it and
> to have a mechanism that pushes back all changes to the shared index
> file automatically when some threshold is reached. The threshold could
> be for example when $GIT_DIR/index size is larger than 25% of the
> shared index size. Opinions, test results or test ideas are welcome on
> this.

Oh yes I would like something like this. I stuck to the basics because
as you see you need to define some criteria to re-split again, but
without experimenting on real repos, I could just have gone the wrong
way. Index file size or the percentage of entries in linked/shared
indexes are two good candidates. You can also just re-split on
commands that likely lead to increasing linked index size a lot (maybe
git-reset), or run long enough that some extra processing won't get
noticed. For example git-gc should definitely re-split if this feature
is on, but I can't say if it's often enough.
-- 
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

I will be waiting for your reply

[CHAN CHAK]

Hello,
My name is Chan Chak a bank manager with an investment bank, I have a
business deal of mutual benefit that involve transfer of large sum of
money. Get back to me through my private email:- chanch...@gmail.com  for
more details if you are interested.
CHAN CHAK

--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Junio C Hamano]

Jeff King  writes:

> However, I was thinking that it might be handy to have this and some
> other information available for helping with debugging. E.g., that we
> could ask bug reporters for "git version --build-options" when we
> suspect something related to their config.

Sure.  I was wrong to phrase it as "I would have..."; either is fine
by me, and being able to ask "git version --build-options" is
probably a plus.
--
To unsubscribe from this list: send the line "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: gc and repack ignore .git/*HEAD when checking reachability

[Jeff King]

On Tue, Jul 12, 2016 at 05:46:12PM +0200, Duy Nguyen wrote:

> I'm not opposed to letting one worktree see everything, but this move
> makes it harder to write new scripts (or new builtin commands, even)
> that works with both single and multiple worktrees because you refer
> to one ref (in current worktree perspective) differently. If we kill
> of the main worktree (i.e. git init always creates a linked worktree)
> then it's less of a problem, but still a nuisance to write
> refs/worktree/$CURRENT/ everywhere.

True. I gave a suggestion for the reading side, but the writing side
would still remain tedious.

I wonder if, in a worktree, we could simply convert requests to read or
write names that do not begin with "refs/" as "refs/worktree/$CURRENT/"?
That makes it a read/write-time alias conversion, but the actual storage
is just vanilla (so the ref storage doesn't need to care, and
reachability just works).

The trickiest thing, I think, is FETCH_HEAD, which is not really a
ref (because it may have a bunch of values, and contain extra
information).

-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: Fast-forward able commit, otherwise fail

[Junio C Hamano]

li...@haller-berlin.de (Stefan Haller) writes:

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

One crucial difference is that at least you _know_ that $tip^2 has
been well tested, when you do not rebase, and you can check out
$tip^2 to study and understand how it was supposed to work, when the
merge of the topic into an updated mainline is found to be faulty.
That knowledge helps you to go forward--you'd need to adjust some
untold assumption $tip^2 made, which was broken somewhere between
$tip^2..$tip^1 on the mainline, to an updated reality.

Once you rebase, you end up with "This series of changes once was
working well, and during the rebase somewhere it stopped working",
unless you say something like "these 7 changes were originally
developed and tested on commit X" in the rebased commit.

--
To unsubscribe from this list: send the line "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: gc and repack ignore .git/*HEAD when checking reachability

[Duy Nguyen]

On Tue, Jul 12, 2016 at 5:26 PM, Jeff King  wrote:
> Likewise for other per-worktree items. If we used refs/MERGE_HEAD and
> refs/worktree/foo/MERGE_HEAD, then you could access them independently
> by using the fully qualified names.

I'm not opposed to letting one worktree see everything, but this move
makes it harder to write new scripts (or new builtin commands, even)
that works with both single and multiple worktrees because you refer
to one ref (in current worktree perspective) differently. If we kill
of the main worktree (i.e. git init always creates a linked worktree)
then it's less of a problem, but still a nuisance to write
refs/worktree/$CURRENT/ everywhere.

> The only downside I see is that the existing names are sometimes
> well-known. I wonder if we could simply add:
>
>   refs/worktree//%s
>
> to the dwim ref-lookup when a command is running in a worktree.
-- 
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: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King  writes:

> I am not certain that there is a modern system with 32-bit time_t. We
> know there are systems with 32-bit unsigned long, and I think that is
> what produced the results people saw. I'd expect even 32-bit systems to
> use "int64_t" or similar for their time_t these days.

OK.
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 08:25:51AM -0700, Junio C Hamano wrote:

> On Tue, Jul 12, 2016 at 8:16 AM, Jeff King  wrote:
> >>
> >> But moving the internal time representation used in various fields
> >> like commit->date to time_t is likely to be a wrong thing to do,
> >> because the first problem with "unsigned long", i.e. "may not be
> >> wide enough", is not limited to "not wide enough to hold time_t".
> >> It also includes "it may not be wide enough to hold time somebody
> >> else recorded in existing objects".
> >
> > But that's a problem no matter what size we choose.
> 
> Yes, if somebody's time_t is larger than my intmax_t, the problem
> cannot be solved for me, if that timestamp is too far in the future or
> in the past.

I am less worried about their time_t and more about whatever crap they
write in ascii into their objects. :)

> But that is not the problem I am pointing out. I heard earlier in the
> thread that time_t on one system was 32-bit (was it Linux?) but I think
> they have "long long". Choosing time_t is strictly inferior choice when
> we already know that a platform with not-wide-enough time_t need to
> be supported, and a type that is wider than that is available.

I am not certain that there is a modern system with 32-bit time_t. We
know there are systems with 32-bit unsigned long, and I think that is
what produced the results people saw. I'd expect even 32-bit systems to
use "int64_t" or similar for their time_t these days.

I'm also not convinced that we would be helping much to carry around a
wider gittime_t. Most of the display code ends up touching a system time
function one way or another, so I find it unlikely it would produce much
better output.

It would help for simple cases like commit->date where we really do just
parse it into a number and never do more with it. But...

> I was envisioning that we would have typedef  gittime_t
> with conversion helpers between it and time_t that allow us to do some
> range checks while at it.

I guess I am just willing to trust that time_t is basically that. And if
your platform has a grossly undersized time_t, then too bad, we clamp
everything it can't hold to 2038 or whatever, and hopefully your
terrible platform dies out or gets a clue sometime in the next 20 years.

-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: gc and repack ignore .git/*HEAD when checking reachability

[Jeff King]

On Tue, Jul 12, 2016 at 12:47:06PM +0200, Johannes Schindelin wrote:

> > Not so fast; it cuts both ways.
> > 
> > People who want multiple worktrees with branches checked out to work
> > in would want to do per-worktree things like bisection, which needs
> > tons more state than we'd be comfortable having directly under
> > $GIT_DIR (e.g. they may also want "git merge" or "git pull", which
> > would use MERGE_HEAD and FETCH_HEAD that are per-worktree and not
> > under refs/; "git bisect" would want to mark number of commits to
> > denote the perimeter of the area of the history being bisected and
> > they live refs/bisect/).
> 
> Sure, `git bisect` would need to realize that it is running in a worktree
> separate from the original one and use a different ref.

I am mostly a bystander in all of these worktree discussions, but your
comment here makes a lot of sense to me. We currently have a global ref
namespace, but the current proposals seem to want to slice it on
invisible lines into per-worktree and global bits, where "refs/bisect"
is no longer a global name. But we could also retain a global view, and
just let worktrees carve out their own portion of the namespace
("refs/worktree/foo/bisect", or even organize it by application area,
"refs/bisect/foo/bad", etc).

Besides being conceptually simpler in the code (global reachability Just
Works, because you see all of the refs), it would also let you access
individual refs between worktrees if you wanted. So for example, if you
are bisecting in worktree "foo", you can access its results from another
worktree with "git show bisect/foo/bad".

Likewise for other per-worktree items. If we used refs/MERGE_HEAD and
refs/worktree/foo/MERGE_HEAD, then you could access them independently
by using the fully qualified names.

The only downside I see is that the existing names are sometimes
well-known. I wonder if we could simply add:

  refs/worktree//%s

to the dwim ref-lookup when a command is running in a worktree.

-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] Git v2.9.1

[Junio C Hamano]

On Tue, Jul 12, 2016 at 8:16 AM, Jeff King  wrote:
>>
>> But moving the internal time representation used in various fields
>> like commit->date to time_t is likely to be a wrong thing to do,
>> because the first problem with "unsigned long", i.e. "may not be
>> wide enough", is not limited to "not wide enough to hold time_t".
>> It also includes "it may not be wide enough to hold time somebody
>> else recorded in existing objects".
>
> But that's a problem no matter what size we choose.

Yes, if somebody's time_t is larger than my intmax_t, the problem
cannot be solved for me, if that timestamp is too far in the future or
in the past.

But that is not the problem I am pointing out. I heard earlier in the
thread that time_t on one system was 32-bit (was it Linux?) but I think
they have "long long". Choosing time_t is strictly inferior choice when
we already know that a platform with not-wide-enough time_t need to
be supported, and a type that is wider than that is available.

I was envisioning that we would have typedef  gittime_t
with conversion helpers between it and time_t that allow us to do some
range checks while at it.
--
To unsubscribe from this list: send the line "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] Add very verbose porcelain output to status

[Jeff King]

On Thu, Jul 07, 2016 at 03:26:28PM -0400, Jeff Hostetler wrote:

> Tools interacting with Git repositories may need to know the complete
> state of the working directory. For efficiency, it would be good to have
> a single command to obtain this information.
> 
> We already have a `--porcelain` mode intended for tools' consumption,
> and it only makes sense to enhance this mode to offer more information.
> 
> Just like we do elsewhere in Git's source code, we now interpret
> multiple `--verbose` flags accumulatively, and show substantially more
> information in porcelain mode at verbosity level 2.

Using "--verbose" to change the stable output feels a little funny to
me. Usually --verbose is about increasing the human-readable output to
stderr. Of course "--verbose" for git-status is already a little weird.
A single "-v" seems to do nothing, but two will turn on a diff for the
long-format output.

But it seems to me this just confuses things more. Why does "git status
--porcelain -v" do nothing, for example? What happens when we want to
add more information to the porcelain output later? Do we have to
require "-vvv" to trigger it, and if so, is there a way to get that
information without the "-vv" information?

I think the existing example for adding to the --porcelain format is
the --branch option. Could we have "--state" or something to trigger
the extra lines?

> +If --branch is given, the first line shows a summary of the current
> +operation in progress.  This line begins with "### state: ", the name
> +of the operation in progress, and then operation-specific information.
> +Fields are separated by a single space.

This seems like a good bit of information to have, and AFAIK we don't
expose the state information in a machine-readable way. A few nits:

> +Operation   Fields Explanation
> +--
> +clean

Using "clean" here seems weird, as that term is usually meant to mean
the opposite of dirty (i.e., there is something in your working tree
that can be committed). But there is no "dirty" here. Would "normal" or
"none" be a good description?

> +--
> +merge  Number unmerged

This  (and others below) is redundant with the rest of the output,
right (i.e., you could count up the "U" entries yourself)? I don't mind
it as a convenience, but I'm kind of curious of why it's useful.

> +--
> +am  [E]Present if the current patch
> +   is empty
> +--
> +rebase Number unmerged
> +[S]Present if split commit in
> +   progress during rebase
> +[E]Present if editing a commit
> +   during rebase

Can a rebased commit be empty? If so, should that state get "E" to match
"am"?

Does editing differentiate between "stopped because of a conflict that
needs addressing" versus "stopped because the interactive insn sheet
told us to"?

> +[I(/)]Present if in an interactive
> +   rebase. Step counts are given.
> +[:] Rebase branches

I wonder if we ought to just use " " here, as separate
arguments. A ":" between refs usually signifies a refspec, but that is
not what this is.

I note this is optional, though. Is there a case where we might have one
but not the other? That would make things more difficult syntactically.

> +If --branch is given, the second line shows branch tracking information.
> +This line begins with "### track: ".  Fields are separated by a single
> +space, unless otherwise indicated.
> +
> +FieldMeaning
> +
> + | "(initial)"  Current commit
> + | "(detached)"  Current branch
> +":"Upstream branch, if set
> +"+"   Ahead count, if upstream present
> +"-"  Behind count, if upstream present
> +

This really _ought_ to have been how --branch worked in the first place
with --porcelain. But unfortunately the existing format has been in the
wild for a long time. To top it off, the existing format is translated!
So you cannot even match "Initial commit on (.*)". Yech.

So this looks like a big improvement. And I see now why you wanted
something like "-vv" to implement this, instead of "--state". In
addition to adding new lines, this is fixing mistakes made in the prior
format.

I'm still not convinced that it makes sense to trigger this 

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 07:34:14AM -0700, Junio C Hamano wrote:

> > Git's source code assumes that unsigned long is at least as precise as
> > time_t. Well, Git's source code is wrong.
> > ...
> 
> That is correct.  As people mentioned downthread already, "unsigned
> long" has two problems, it may not be wide enough, and it cannot
> represent time before the epoch.
> 
> But moving the internal time representation used in various fields
> like commit->date to time_t is likely to be a wrong thing to do,
> because the first problem with "unsigned long", i.e. "may not be
> wide enough", is not limited to "not wide enough to hold time_t".
> It also includes "it may not be wide enough to hold time somebody
> else recorded in existing objects".

But that's a problem no matter what size we choose. The ascii format in
the commit objects is arbitrary-length, so somebody can always overflow
it. So even with intmax_t we have to clamp it to a sentinel value at
some point. IMHO we are better off to do so at parse time, and then have
consistent sizes through the rest of the code base, without worrying
about juggling intmax_t to time_t truncation in multiple places.

IOW, I think we probably interact with the system time libraries more
often than we parse (and it's easy to wrap the parsing in a function,
but there are a lot of system time functions).

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

Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files

[Duy Nguyen]

On Tue, Jul 12, 2016 at 9:04 AM, Christian Couder
 wrote:
> On Mon, Jul 11, 2016 at 8:27 PM, Duy Nguyen  wrote:
>> On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder
>>  wrote:
>>> Everytime split index is turned on, it creates a "sharedindex."
>>> file in the git directory. This makes sure that old sharedindex
>>> files are removed after a new one has been created.
>>
>> Hmm it's one-way link, we don't know how many index files use this
>> shared index file, how can you be sure nobody else will need it? I'm
>> thinking about temporary indexes. If a temp index is created, saved on
>> disk, and use delete the shared index file, the real, main index may
>> become useless. Temp index will most likely replace the main index
>> (git commit) but if a failure happens, we can't fall back.
>
> Isn't there a way to scan all the current indexes (temp or not) to see
> which shared indexes they need?

No. People could create an index file anywhere in theory. So you don't
know how many index files there are.

>> A safer approach is "touch" the shared index every time a linked index
>> is used, then we can delete shared indexes with old mtime, older than
>> a grace period, in git-prune (or here).
>
> Maybe old linked indexes could be converted after some time to use a
> newer shared index, so that we can get rid of the old shared indexes.
> That seems safer than just deleting old linked indexes.

It really depends. If the shared part is too small for old indexes, we
might as well unsplit them. In practice though, the only long-term
index file is $GIT_DIR/index. If we don't delete old shared index
files too close to their creation time, temp index files will go away.
-- 
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: What's cooking in git.git (Jul 2016, #04; Mon, 11)

[Junio C Hamano]

On Tue, Jul 12, 2016 at 6:16 AM, Johannes Schindelin
 wrote:
>
> On Mon, 11 Jul 2016, Junio C Hamano wrote:
>
>> [New Topics]
>>
>> [...]
>
> What about http://thread.gmane.org/gmane.comp.version-control.git/299050?

Not forgotten.

It just is not one of the "New Topics" yet, together with several
other patches sent to
the list recently, hence it is not listed there.
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Junio C Hamano]

Johannes Schindelin  writes:

> Git's source code assumes that unsigned long is at least as precise as
> time_t. Well, Git's source code is wrong.
> ...

That is correct.  As people mentioned downthread already, "unsigned
long" has two problems, it may not be wide enough, and it cannot
represent time before the epoch.

But moving the internal time representation used in various fields
like commit->date to time_t is likely to be a wrong thing to do,
because the first problem with "unsigned long", i.e. "may not be
wide enough", is not limited to "not wide enough to hold time_t".
It also includes "it may not be wide enough to hold time somebody
else recorded in existing objects".

Since some platforms have time_t that is not wide enough, but still
have intmax_t that is wider, I think we would be better off to pick
an integral type to use for the internal representation that is the
widest throughout the API, and use time_t only at places that we
interact with the system libraries (e.g. when we ask "what is the
time now?" to time(2), when we ask "break down this timestamp" to
gmtime(3)).

Thanks for starting this, and from a brief read, the hotfix to skip
the test downthread looked good.  The places this "starting point"
patch covers look like a good set that interacts with time obtained
locally (e.g. prune that compares with filesystem timestamp); just
make sure you don't go too far and end up shoving timestamps from
other people into time_t, which may not fit.

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: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Mon, Jul 11, 2016 at 08:40:56PM -0400, Anders Kaseorg wrote:

> On 07/11/2016 07:54 PM, Jeff King wrote:
> > Yes, that's somewhat the point of the test.
> > 
> > How does it fail for you (what does it look like with "-v")? We may be
> > able to check for an outcome that matches both cases.
> 
> On Ubuntu i386 and Ubuntu armhf, I get the following verbose output from
> t0006-date.sh:
> 
> 
> expecting success:
>   echo "$time -> $expect" >expect &&
>   test-date show:$format "$time" >actual &&
>   test_cmp expect actual
>   
> --- expect2016-07-11 23:20:55.835136188 +
> +++ actual2016-07-11 23:20:55.835136188 +
> @@ -1 +1 @@
> -5758122296 -0400 -> 2152-06-19 18:24:56 -0400
> +5758122296 -0400 -> 2038-01-18 23:14:07 -0400

Thank you for this, by the way. Your comment got drowned out by the rest
of the thread, but this would have been very helpful for me in writing a
patch (fortunately Dscho beat me to it, so I did not have to. :) ).

-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 v3 4/8] doc: give headings for the two and three dot notations

[Marc Branchaud]

On 2016-07-11 04:25 PM, Philip Oakley wrote:

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 | 23 +--
  1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 79f6d03..1c59e87 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,46 @@ 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.

+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 the ones reachable from 'r1'.


All of these headings render poorly in the manpage, at least for me 
(Ubuntu 16.04).  Only the first word appears in bold; the '-quoted text 
is not bold but underlined, and the rest of the header is plain.



Also, I think calling this "The ^ notation" is confusing, because 
there's already an earlier paragraph on the "^" syntax.


Maybe we don't need a header here?  I only suggest that because I'm 
having trouble coming up with a nice alternative.  "Commit Exclusion"?




-This set operation appears so often that there is a shorthand
+The '..' (two-dot) range notation
+~


Perhaps "Range notation", to mirror the capitalization of "Symmetric 
Difference" in the next header?



+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 shorthands, you can omit one end and let it default to HEAD.
+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.


Unfortunately the new headings make it appear that this paragraph is 
exclusively part of the '...' notation section.  Folks reading the '..' 
section are likely to skip it.


I like the examples, though.  I think it would be worthwhile to remove 
this paragraph and fold it explicitly into the '..' and '...' notation 
sections.


So add something like this to the '..' section (only the first sentence 
here is new):


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  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.

And also, add the same first sentence and a different example to the 
'...' section.  Something like this:


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  For example, 'origin...' is a shorthand for
'origin...HEAD' and asks "What have I and origin both done
since I forked from the origin branch?"  Note that 'origin...'
and '...origin' ask the same question.



+Additional '{caret}' Shorthand notations
+
  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.
+and its parent commits exist.


I think descriptions of ^@ and ^! should live under the main 
description of ^.  That part already describes the numeric suffix, 
so describing a couple of special suffixes there seems like a natural fit.


However, if you choose to keep this little section, you need to move the 
word "exist" earlier in the sentence:


Two other 

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 01:25:20PM +0200, Johannes Schindelin wrote:

> [PATCH] Work around test failures due to timestamps being unsigned long
> 
> Git's source code refers to timestamps as unsigned longs. On 32-bit
> platforms, as well as on Windows, unsigned long is not large enough to
> capture dates that are "absurdly far in the future".
> 
> While we will fix this issue properly by replacing unsigned long ->
> time_t, on the maint track we want to be a bit more conservative and
> just skip those tests.
> 
> Signed-off-by: Johannes Schindelin 
> ---

This looks like a reasonable interim fix for both Windows and for the
general maint track. If we reliably produce "2038" for the 999... value
then that's simpler than adding in magic to ask about sizeof(ulong). I
also wondered if we should test forthe _correct_ value, but that would
defeat the point of the test (999... is also far in the future).

One minor comment:

> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 04ce535..d185640 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -48,10 +48,17 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 
> +0200'
>  check_show raw "$TIME" '146600 +0200'
>  check_show iso-local "$TIME" '2016-06-15 14:13:20 +'
>  
> -# arbitrary time absurdly far in the future
> -FUTURE="5758122296 -0400"
> -check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
> -check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +"
> +case "$(test-date show:iso 99)" in
> +*2038*)
> + # on this platform, unsigned long is 32-bit, i.e. not large enough
> + ;;
> +*)
> + # arbitrary time absurdly far in the future
> + FUTURE="5758122296 -0400"
> + check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
> + check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +"
> + ;;
> +esac

It would be nice to wrap this in a prereq, like:

  test_lazy_prereq 64BIT_TIME '
case "$(test-date show:short 99)" in
*2038*)
false
;;
*)
true
;;
esac
  '

  ...
  check_show 64BIT_TIME iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
  check_show 64BIT_TIME iso-local "$FUTURE" "2152-06-19 22:24:56 +"

The main advantage is that it will number the tests consistently, and
give us a "skipped" message (which can remind us to drop the prereq
later when everything goes 64-bit).

But it also will do the right thing with test-date's output with respect
to "-v" (not that we expect it to produce any output).

You'll need to adjust check_show as I did in my earlier patch.

-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 v3 7/8] doc: revisions - define `reachable`

[Marc Branchaud]

On 2016-07-11 04:25 PM, Philip Oakley wrote:

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 1c59e87..a3cd28b 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 `reachable` set of commits of the given
+commit.


Better as "... means the set of commits `reachable` from the given commit."


+
+A commit's reachable set is the commit itself and the commits of
+its ancestry chain.
+


s/of/in/

M.

--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 03:31:00PM +0200, Andreas Schwab wrote:

> Johannes Schindelin  writes:
> 
> > Hi Andreas,
> >
> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
> >
> >> Johannes Schindelin  writes:
> >> 
> >> >> PRIuMAX isn't compatible with time_t.
> >> >
> >> > That statement is wrong.
> >> 
> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> >> (even if they happen to have the same representation).
> >
> > Sigh.
> >
> > So if it is wrong, what is right?
> 
> The right thing is to add a cast, of course.

In general, I think the right cast for time_t should be to (intmax_t),
and the formatting string should be PRIdMAX (which, as an aside, needs
an entry in git-compat-util.h).

In this particular code (show_date_relative), though, I think you can
get away with treating it as unsigned, because it's not actually a
time_t but rather a difference. And we handle the negative difference at
the top of the function already ("in the future").

-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: Fast-forward able commit, otherwise fail

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

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 energy when digging through the history.

Second, we did indeed run into the scalability problems that you
describe [*1*]. However, we ran into this way before starting to require
the rebase-before-merge; in my experience, rebasing or not makes no
difference here. After all, the resulting tree state of the merge commit
is identical in both cases; it's just the individual commits on the
merged topic branch that have not been tested in the rebased case. But
if the merge commit is green, it is pretty unlikely in my experience
that one of the individual commits is not. It's theoretically possible
of course, just very, very unlikely.

So what am I missing?


[Footnote]
*1* These problems were so annoying for us that we invented technical
measures to solve them. We now have a web interface where developers can
grab a lock on the repo, or put themselves into a queue for the lock
when it's taken. There's a push hook that only allows pushing when you
hold the lock. This solves it nicely, because once you have the lock,
you can take all the time you need to make sure your merge compiles, and
run the test suite locally.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Johannes Schindelin  writes:

> Hi Andreas,
>
> On Tue, 12 Jul 2016, Andreas Schwab wrote:
>
>> Johannes Schindelin  writes:
>> 
>> >> PRIuMAX isn't compatible with time_t.
>> >
>> > That statement is wrong.
>> 
>> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
>> (even if they happen to have the same representation).
>
> Sigh.
>
> So if it is wrong, what is right?

The right thing is to add a cast, of course.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Johannes Schindelin]

Hi Andreas,

On Tue, 12 Jul 2016, Andreas Schwab wrote:

> Johannes Schindelin  writes:
> 
> >> PRIuMAX isn't compatible with time_t.
> >
> > That statement is wrong.
> 
> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> (even if they happen to have the same representation).

Sigh.

So if it is wrong, what is right? I hoped that my gentle reprimand in my
previous reply would result in a more helpful response.

Let me guess: PRId64 is what you would have suggested if you had followed
up your negative statement with a "do this instead"?

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: What's cooking in git.git (Jul 2016, #04; Mon, 11)

[Johannes Schindelin]

Hi Junio,

On Mon, 11 Jul 2016, Junio C Hamano wrote:

> [New Topics]
> 
> [...]

What about http://thread.gmane.org/gmane.comp.version-control.git/299050?

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

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Johannes Schindelin  writes:

>> PRIuMAX isn't compatible with time_t.
>
> That statement is wrong.

No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
(even if they happen to have the same representation).

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Johannes Schindelin]

Hi Andreas,

On Tue, 12 Jul 2016, Andreas Schwab wrote:

> Johannes Schindelin  writes:
> 
> > @@ -88,11 +88,11 @@ static int local_tzoffset(unsigned long time)
> > return offset * eastwest;
> >  }
> >  
> > -void show_date_relative(unsigned long time, int tz,
> > +void show_date_relative(time_t time, int tz,
> >const struct timeval *now,
> >struct strbuf *timebuf)
> >  {
> > -   unsigned long diff;
> > +   time_t diff;
> > if (now->tv_sec < time) {
> > strbuf_addstr(timebuf, _("in the future"));
> > return;
> > @@ -100,65 +100,65 @@ void show_date_relative(unsigned long time, int tz,
> > diff = now->tv_sec - time;
> > if (diff < 90) {
> > strbuf_addf(timebuf,
> > -Q_("%lu second ago", "%lu seconds ago", diff), diff);
> > +Q_("%" PRIuMAX " second ago", "%" PRIuMAX " seconds 
> > ago", diff), diff);
> 
> PRIuMAX isn't compatible with time_t.

That statement is wrong. But you probably meant that PRIuMAX is *not
necessarily* the correct thing to use.

And I would agree with that. I had to have a patch that 1) compiles and 2)
fixes t0006 on Windows, and the patch I presented achieved both goals.

I hoped that my brief "starting point" hint would make it obvious that I
do not think that this patch is acceptable?

My idea was to introduce a TIME_T_LARGER_THAN_ULONG and take it from
there, but I had to switch contexts before I could finish that part of the
patch, yet I still wanted to let y'all know that patches are in the
working.

For future record: I appreciate feedback especially when it is
constructive, i.e. when "that's wrong" is not left on its own, but is
instead followed by "why not do XYZ instead".

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: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Peff,

On Tue, 12 Jul 2016, Jeff King wrote:

> On Tue, Jul 12, 2016 at 09:30:28AM +0200, Johannes Schindelin wrote:
> 
> > FWIW I have this monster patch as a starting point (I plan to work more on
> > this today):
> 
> Cool! Thanks for working on this.

Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
do that with a failing test suite.

> I suspect we should still do something about skipping those tests,
> though, if only because the v2.9.x maint track is broken, and switching
> to time_t is a sufficiently large change that we probably don't want it
> for maint (it _seems_ like it shouldn't cause any problems, but I'm
> wondering if we might inadvertently trigger funny issues around
> signedness or something).

I totally agree that this patch is very, very large. And yeah, this change
is intrusive enough that it should not hit maint (but then, IMO neither
should the change that triggered this test failure have hit maint).

So I think I'll go with this for the moment (as we all know, my patch
series often go through a couple of commenting rounds, and are not always
picked up quickly, and I do not wait that long with Git for Windows
v2.9.1):

-- snipsnap --
[PATCH] Work around test failures due to timestamps being unsigned long

Git's source code refers to timestamps as unsigned longs. On 32-bit
platforms, as well as on Windows, unsigned long is not large enough to
capture dates that are "absurdly far in the future".

While we will fix this issue properly by replacing unsigned long ->
time_t, on the maint track we want to be a bit more conservative and
just skip those tests.

Signed-off-by: Johannes Schindelin 
---
 t/t0006-date.sh | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 04ce535..d185640 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -48,10 +48,17 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '146600 +0200'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +'
 
-# arbitrary time absurdly far in the future
-FUTURE="5758122296 -0400"
-check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
-check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +"
+case "$(test-date show:iso 99)" in
+*2038*)
+   # on this platform, unsigned long is 32-bit, i.e. not large enough
+   ;;
+*)
+   # arbitrary time absurdly far in the future
+   FUTURE="5758122296 -0400"
+   check_show iso   "$FUTURE" "2152-06-19 18:24:56 -0400"
+   check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +"
+   ;;
+esac
 
 check_parse() {
echo "$1 -> $2" >expect
-- 
2.9.0.278.g1caae67

--
To unsubscribe from this list: send the line "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: Submodule's .git file contains absolute path when created using 'git clone --recursive'

[Ricardo Sánchez-Sáez]

Stefan Beller  google.com> writes:

> 
> On Thu, May 5, 2016 at 12:20 PM, Loet Avramson  forter.com> wrote:
> > It happened on 2.8.1, also reproducible on 2.8.2.
> > Haven't had the time to dive deeper into the code but my guess is that
> > relative_path() returns different results in those 2 cases or maybe
> > the way git-submodule.sh handles it.
> >
> 
> Then you found a new bug, congratulations. ;)
> Thanks for reporting.
> 
> The shell script uses relative_path() only for displaying paths,
> not for writing them to the .git file.
> 
> it really boils down to different environments
> "git submodule update --init --recursive" is called from
> (either manually or from `git clone`).
> 
> Apart from that there are no immediate bells ringing,
> are you doing any weird stuff with the file system (soft/hard
> links) ?
> 
> Thanks,
> Stefan
> 

Hi,

sorry to awake an old thread. Has this been fixed? In which git version?
It's hitting me on git version  2.7.4 (Apple Git-66) (default git client on
 OS X 10.11.5 (15F34)).

I think all submodule .git files should contain relative paths. Otherwise,
 duplicating or moving the cloned  repository folder breaks the submodules.

Best,
Ricardo

--
To unsubscribe from this list: send the line "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: gc and repack ignore .git/*HEAD when checking reachability

[Johannes Schindelin]

Hi Junio,

On Mon, 11 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> No, the point is, refs subsystem needs to know which refs is per-repo,
> >> which is per-worktree. So far the rules are  "everything under refs,
> >> except a few known exceptions, is per-repo" and "everything directly
> >> under $GIT_DIR is per-worktree", which work fine. But if you allow to
> >> move per-worktree to "refs" freely, then the "known exceptions" will
> >> have to be updated every time a new per-worktree ref appears. It'll be
> >> easier to modify the first rule as "everything under refs, except some
> >> legacy exceptions and refs/worktree, is per-repo".
> >
> > Given the substantial pain and suffering I have due to per-worktree
> > reflogs (and those are *just* HEAD's reflogs!), it appears to me that
> > per-worktree refs would be a particularly poor feature.
> >
> > I agree that HEAD needs to be per-worktree, but already the fact that the
> > HEADs of the worktrees, along with their reflogs, are *not* visible to
> > all other worktrees causes substantial trouble.
> 
> Not so fast; it cuts both ways.
> 
> People who want multiple worktrees with branches checked out to work
> in would want to do per-worktree things like bisection, which needs
> tons more state than we'd be comfortable having directly under
> $GIT_DIR (e.g. they may also want "git merge" or "git pull", which
> would use MERGE_HEAD and FETCH_HEAD that are per-worktree and not
> under refs/; "git bisect" would want to mark number of commits to
> denote the perimeter of the area of the history being bisected and
> they live refs/bisect/).

Sure, `git bisect` would need to realize that it is running in a worktree
separate from the original one and use a different ref.

> And when you are bisecting in the worktree dedicated for a topic,
> it is a feature that your other worktrees do not need to see how
> much history you narrowed down in that topic.

If you intentionally hide bisections from other worktrees, you will
invariably end up with the same problems I faced with auto-gc: in a
worktree, it is *much, much easier* to forget a bisect in progress.

In other words, your comments make me even more certain that per-worktree
refs are undesirable.

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

Re: [RFC] Long running Git clean/smudge filter

[Lars Schneider]

> On 10 Jul 2016, at 17:10, Joey Hess  wrote:
> 
> larsxschnei...@gmail.com wrote:
>> (2) Joey's topic, which is the base for my patch, looks stalled for more than
>> 2 weeks:
>> http://thread.gmane.org/gmane.comp.version-control.git/297994/focus=298006
>> I would be happy to address Junio's comments and post a reroll. However,
>> I don't want to interfere with Joey's plans.
> 
> I've been on vacation and plan to get back to that in the upcoming week.

Good to hear :-) ! I hope you had a great vacation!



>> @Joey (in case you are reading this):
>> My patch changes your initial idea quite a bit. However, I believe it is an
>> improvement that could be beneficial for git-annex, too. Would you prefer to
>> work with me on the combination of our ideas (file clean/smudge + long 
>> running
>> clean/smudge processes) or would you prefer to keep your interface?
> 
> Long running filters mean that you need a more complicated protocol to
> speak over the pipes. Seems that such a protocol could be designed to work
> with the original smudge/clean filters as well as with my
> smudgeToFile/cleanFromFile filters. Assuming that there's a way to
> tell whether the filters support being long-running or not.
> 
> Note that the interface we arrived at for smudgeToFile/cleanFromFile is as
> similar as possible to smudge/clean, so the filter developer only has to
> change one thing. That's a big plus, and so I don't like diverging the
> two interfaces.
I understand, thanks for the clarification. My plan is to implement long 
running 
filters only for smudgeToFile/cleanFromFile (at least initially) as this should
solve the major pain point already and is relatively straight forward.

What do you think about this kind of config? Any idea for a better config name?

[filter "bar"]
clean = foo1 %f
smudge = foo2 %f
cleanFromFile foo3
smudgeToFile foo4
longRunning = true

I think it would be easy to support both of our interfaces in parallel. Do
you understand why the "async" API is necessary?
https://github.com/larsxschneider/git/blob/74e22bd4e0b505785fa8ffc2ef15721909635d1c/convert.c#L585-L599

Would you be OK if I implement smudgeToFile/cleanFromFile as separate
"apply_filter" function as I have prototyped here:
https://github.com/larsxschneider/git/blob/74e22bd4e0b505785fa8ffc2ef15721909635d1c/convert.c#L1143-L1146
https://github.com/larsxschneider/git/blob/74e22bd4e0b505785fa8ffc2ef15721909635d1c/convert.c#L402-L488


Thanks,
Lars
--
To unsubscribe from this list: send the line "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] gitk: ru.po: Update Russian translation

[Dimitriy Ryazantcev]

Signed-off-by: Dimitriy Ryazantcev 
---
 po/ru.po | 640 ---
 1 file changed, 328 insertions(+), 312 deletions(-)

diff --git a/po/ru.po b/po/ru.po
index 17ed026..6a00dc2 100644
--- a/po/ru.po
+++ b/po/ru.po
@@ -3,15 +3,15 @@
 # Translators:
 # 0xAX , 2014
 # Alex Riesen , 2015
-# Dimitriy Ryazantcev , 2015
+# Dimitriy Ryazantcev , 2015-2016
 # Dmitry Potapov , 2009
 # Skip , 2011
 msgid ""
 msgstr ""
 "Project-Id-Version: Git Russian Localization Project\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-05-17 14:32+1000\n"
-"PO-Revision-Date: 2015-10-12 10:14+\n"
+"POT-Creation-Date: 2016-06-30 16:40+0300\n"
+"PO-Revision-Date: 2016-06-30 13:49+\n"
 "Last-Translator: Dimitriy Ryazantcev \n"
 "Language-Team: Russian 
(http://www.transifex.com/djm00n/git-po-ru/language/ru/)\n"
 "MIME-Version: 1.0\n"
@@ -24,11 +24,11 @@ msgstr ""
 msgid "Couldn't get list of unmerged files:"
 msgstr "Невозможно получить список файлов незавершённой операции слияния:"
 
-#: gitk:212 gitk:2381
+#: gitk:212 gitk:2399
 msgid "Color words"
 msgstr "Цветные слова"
 
-#: gitk:217 gitk:2381 gitk:8220 gitk:8253
+#: gitk:217 gitk:2399 gitk:8239 gitk:8272
 msgid "Markup words"
 msgstr "Помеченые слова"
 
@@ -58,15 +58,15 @@ msgstr "Ошибка запуска git log:"
 msgid "Reading"
 msgstr "Чтение"
 
-#: gitk:496 gitk:4525
+#: gitk:496 gitk:4544
 msgid "Reading commits..."
 msgstr "Чтение коммитов..."
 
-#: gitk:499 gitk:1637 gitk:4528
+#: gitk:499 gitk:1637 gitk:4547
 msgid "No commits selected"
 msgstr "Ничего не выбрано"
 
-#: gitk:1445 gitk:4045 gitk:12432
+#: gitk:1445 gitk:4064 gitk:12469
 msgid "Command line"
 msgstr "Командная строка"
 
@@ -78,1252 +78,1268 @@ msgstr "Ошибка обработки вывода команды git log:"
 msgid "No commit information available"
 msgstr "Нет информации о коммите"
 
-#: gitk:1903 gitk:1932 gitk:4315 gitk:9669 gitk:11241 gitk:11521
+#: gitk:1903 gitk:1932 gitk:4334 gitk:9702 gitk:11274 gitk:11554
 msgid "OK"
 msgstr "Ok"
 
-#: gitk:1934 gitk:4317 gitk:9196 gitk:9275 gitk:9391 gitk:9440 gitk:9671
-#: gitk:11242 gitk:11522
+#: gitk:1934 gitk:4336 gitk:9215 gitk:9294 gitk:9424 gitk:9473 gitk:9704
+#: gitk:11275 gitk:11555
 msgid "Cancel"
 msgstr "Отмена"
 
-#: gitk:2069
+#: gitk:2083
 msgid ""
 msgstr "Обновить"
 
-#: gitk:2070
+#: gitk:2084
 msgid ""
 msgstr "Перечитать"
 
-#: gitk:2071
+#: gitk:2085
 msgid "Reread re"
 msgstr "Обновить список ссылок"
 
-#: gitk:2072
+#: gitk:2086
 msgid " references"
 msgstr "Список ссылок"
 
-#: gitk:2074
+#: gitk:2088
 msgid "Start git "
 msgstr "Запустить git gui"
 
-#: gitk:2076
+#: gitk:2090
 msgid ""
 msgstr "Завершить"
 
-#: gitk:2068
+#: gitk:2082
 msgid ""
 msgstr "Файл"
 
-#: gitk:2080
+#: gitk:2094
 msgid ""
 msgstr "Настройки"
 
-#: gitk:2079
+#: gitk:2093
 msgid ""
 msgstr "Редактировать"
 
-#: gitk:2084
+#: gitk:2098
 msgid " view..."
 msgstr "Новое представление..."
 
-#: gitk:2085
+#: gitk:2099
 msgid " view..."
 msgstr "Редактировать представление..."
 
-#: gitk:2086
+#: gitk:2100
 msgid " view"
 msgstr "Удалить представление"
 
-#: gitk:2088 gitk:4043
+#: gitk:2102
 msgid " files"
 msgstr "Все файлы"
 
-#: gitk:2083 gitk:4067
+#: gitk:2097
 msgid ""
 msgstr "Представление"
 
-#: gitk:2093 gitk:2103 gitk:3012
+#: gitk:2107 gitk:2117
 msgid " gitk"
 msgstr "О gitk"
 
-#: gitk:2094 gitk:2108
+#: gitk:2108 gitk:2122
 msgid " bindings"
 msgstr "Назначения клавиатуры"
 
-#: gitk:2092 gitk:2107
+#: gitk:2106 gitk:2121
 msgid ""
 msgstr "Подсказка"
 
-#: gitk:2185 gitk:8652
+#: gitk:2199 gitk:8671
 msgid "SHA1 ID:"
 msgstr "SHA1 ID:"
 
-#: gitk:2229
+#: gitk:2243
 msgid "Row"
 msgstr "Строка"
 
-#: gitk:2267
+#: gitk:2281
 msgid "Find"
 msgstr "Поиск"
 
-#: gitk:2295
+#: gitk:2309
 msgid "commit"
 msgstr "коммит"
 
-#: gitk:2299 gitk:2301 gitk:4687 gitk:4710 gitk:4734 gitk:6755 gitk:6827
-#: gitk:6912
+#: gitk:2313 gitk:2315 gitk:4706 gitk:4729 gitk:4753 gitk:6774 gitk:6846
+#: gitk:6931
 msgid "containing:"
 msgstr "содержащее:"
 
-#: gitk:2302 gitk:3526 gitk:3531 gitk:4763
+#: gitk:2316 gitk:3545 gitk:3550 gitk:4782
 msgid "touching paths:"
 msgstr "касательно файлов:"
 
-#: gitk:2303 gitk:4777
+#: gitk:2317 gitk:4796
 msgid "adding/removing string:"
 msgstr "добавив/удалив строку:"
 
-#: gitk:2304 gitk:4779
+#: gitk:2318 gitk:4798
 msgid "changing lines matching:"
 msgstr "изменяя совпадающие строки:"
 
-#: gitk:2313 gitk:2315 gitk:4766
+#: gitk:2327 gitk:2329 gitk:4785
 msgid "Exact"
 msgstr "Точно"
 
-#: gitk:2315 gitk:4854 gitk:6723
+#: gitk:2329 gitk:4873 gitk:6742
 msgid "IgnCase"
 msgstr "Игнорировать большие/маленькие"
 
-#: gitk:2315 gitk:4736 gitk:4852 gitk:6719
+#: gitk:2329 gitk:4755 gitk:4871 gitk:6738
 msgid "Regexp"
 msgstr "Регулярные выражения"
 
-#: gitk:2317 gitk:2318 gitk:4874 gitk:4904 gitk:4911 gitk:6848 gitk:6916
+#: 

Re: git push doesn't update the status with multiple remotes

[Garoe]

Thanks for the quick reply.

On 12/07/16 06:26, Johannes Sixt wrote:

Am 11.07.2016 um 18:54 schrieb Garoe:

I have a repository on github, a clone on my desktop and bare repo on a
private server, in my desktop the remotes looks like this

allg...@github.com:user/repo.git (fetch)
allg...@github.com:user/repo.git (push)
allu...@server.com:user/repo.git (push)
serveru...@server.com:user/repo.git (fetch)
serveru...@server.com:user/repo.git (push)
origing...@github.com:user/repo.git (fetch)
origing...@github.com:user/repo.git (push)

If I commit to master in my desktop and run 'git push all master', the
github and the server repos are correctly updated, but if I run 'git
status' the message says:

Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)


But "all" and "origin" are different remotes. Just because you use the
same URL does not make them the same remote repository from the view of
your local repository.


I expected git to be "intelligent" enough to detect that if the url are 
the same, it had already exchanged information with the server by the 
push command, so it would update the message without explicitly pushing 
to origin.




(Additionally, "all" is not a special name, just in case you had
expected that.)


The message won't update unless I run git fetch or git push origin
master.


Yes, that's how it is supposed to work.


From my point of view the current behaviour is counter-intuitive. 
Anyhow, I understand by your answer that the current behaviour is 
desired and it won't be changed.



I think there is some way to configure that a single push command pushes
to several remote repositories, but I can't find it right now...

-- Hannes



Thanks again,
Garoe
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Andreas Schwab]

Johannes Schindelin  writes:

> @@ -88,11 +88,11 @@ static int local_tzoffset(unsigned long time)
>   return offset * eastwest;
>  }
>  
> -void show_date_relative(unsigned long time, int tz,
> +void show_date_relative(time_t time, int tz,
>  const struct timeval *now,
>  struct strbuf *timebuf)
>  {
> - unsigned long diff;
> + time_t diff;
>   if (now->tv_sec < time) {
>   strbuf_addstr(timebuf, _("in the future"));
>   return;
> @@ -100,65 +100,65 @@ void show_date_relative(unsigned long time, int tz,
>   diff = now->tv_sec - time;
>   if (diff < 90) {
>   strbuf_addf(timebuf,
> -  Q_("%lu second ago", "%lu seconds ago", diff), diff);
> +  Q_("%" PRIuMAX " second ago", "%" PRIuMAX " seconds 
> ago", diff), diff);

PRIuMAX isn't compatible with time_t.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 09:30:28AM +0200, Johannes Schindelin wrote:

> On Mon, 11 Jul 2016, Junio C Hamano wrote:
> 
> > Those who care about 32-bit builds need to start building and
> > testing 'next' and 'master' regularly, or similar breakages are
> > bound to continue happening X-<.
> 
> Please note that "unsigned long" is 32-bit even on 64-bit Windows.

Yeah, that was part of the reason my run-time test checked
sizeof(unsigned long), and not any kind of "are we 64-bit?" compiler
options or output. So the "64BIT" prereq is actually a bit of a
misnomer; it is really "is your ULL 64-bit, because that's what we use
for time". :-/

> FWIW I have this monster patch as a starting point (I plan to work more on
> this today):

Cool! Thanks for working on this.

I suspect we should still do something about skipping those tests,
though, if only because the v2.9.x maint track is broken, and switching
to time_t is a sufficiently large change that we probably don't want it
for maint (it _seems_ like it shouldn't cause any problems, but I'm
wondering if we might inadvertently trigger funny issues around
signedness or something).

> From b65de5a56e18b038d432ba828d7714880b8e285c Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin 
> Date: Tue, 12 Jul 2016 09:09:07 +0200
> Subject: [PATCH] HOTFIX: use time_t wherever appropriate
> 
> Git's source code assumes that unsigned long is at least as precise as
> time_t. Well, Git's source code is wrong.

Another problem with "unsigned long" is that we cannot handle times
older than 1970. Assuming most systems are sane enough to have a signed
time_t these days, this would fix that problem as well.

-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] Git v2.9.1

[Johannes Schindelin]

Hi Junio & Peff,

On Mon, 11 Jul 2016, Junio C Hamano wrote:

> Those who care about 32-bit builds need to start building and
> testing 'next' and 'master' regularly, or similar breakages are
> bound to continue happening X-<.

Please note that "unsigned long" is 32-bit even on 64-bit Windows.

FWIW I have this monster patch as a starting point (I plan to work more on
this today):

-- snipsnap --
>From b65de5a56e18b038d432ba828d7714880b8e285c Mon Sep 17 00:00:00 2001
From: Johannes Schindelin 
Date: Tue, 12 Jul 2016 09:09:07 +0200
Subject: [PATCH] HOTFIX: use time_t wherever appropriate

Git's source code assumes that unsigned long is at least as precise as
time_t. Well, Git's source code is wrong.

Signed-off-by: Johannes Schindelin 
---
 Documentation/technical/api-parse-options.txt |  4 +-
 builtin/fsck.c|  2 +-
 builtin/merge-base.c  |  2 +-
 builtin/prune.c   |  2 +-
 builtin/reflog.c  | 24 +++
 builtin/show-branch.c |  4 +-
 builtin/worktree.c|  2 +-
 cache.h   | 14 ++---
 date.c| 90 +++
 parse-options-cb.c|  4 +-
 reflog-walk.c |  8 +--
 refs.c| 14 ++---
 refs.h|  8 +--
 refs/files-backend.c  |  4 +-
 revision.c|  6 +-
 sha1_name.c   |  6 +-
 t/helper/test-date.c  |  2 +-
 t/helper/test-parse-options.c |  4 +-
 vcs-svn/svndump.c |  2 +-
 wt-status.c   |  2 +-
 20 files changed, 106 insertions(+), 98 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 27bd701..6801aad 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -178,11 +178,11 @@ There are some macros to easily define options:
scale the provided value by 1024, 1024^2 or 1024^3 respectively.
The scaled value is put into `unsigned_long_var`.
 
-`OPT_DATE(short, long, _var, description)`::
+`OPT_DATE(short, long, _t_var, description)`::
Introduce an option with date argument, see `approxidate()`.
The timestamp is put into `int_var`.
 
-`OPT_EXPIRY_DATE(short, long, _var, description)`::
+`OPT_EXPIRY_DATE(short, long, _t_var, description)`::
Introduce an option with expiry date argument, see 
`parse_expiry_date()`.
The timestamp is put into `int_var`.
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 3f27456..3dfa32b 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -385,7 +385,7 @@ static void fsck_handle_reflog_sha1(const char *refname, 
unsigned char *sha1)
 }
 
 static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-   const char *email, unsigned long timestamp, int tz,
+   const char *email, time_t timestamp, int tz,
const char *message, void *cb_data)
 {
const char *refname = cb_data;
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index c0d1822..85ba332 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -132,7 +132,7 @@ static void add_one_commit(unsigned char *sha1, struct 
rev_collect *revs)
 }
 
 static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
- const char *ident, unsigned long timestamp,
+ const char *ident, time_t timestamp,
  int tz, const char *message, void *cbdata)
 {
struct rev_collect *revs = cbdata;
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f052..5219504 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -13,7 +13,7 @@ static const char * const prune_usage[] = {
 };
 static int show_only;
 static int verbose;
-static unsigned long expire;
+static time_t expire;
 static int show_progress = -1;
 
 static int prune_tmp_file(const char *fullpath)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e..331c874 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -16,14 +16,14 @@ static const char reflog_delete_usage[] =
 static const char reflog_exists_usage[] =
 "git reflog exists ";
 
-static unsigned long default_reflog_expire;
-static unsigned long default_reflog_expire_unreachable;
+static time_t default_reflog_expire;
+static time_t default_reflog_expire_unreachable;
 
 struct cmd_reflog_expire_cb {
struct rev_info revs;
int stalefix;
-   unsigned long expire_total;
-   unsigned long expire_unreachable;
+   time_t 

Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files

[Christian Couder]

On Mon, Jul 11, 2016 at 8:27 PM, Duy Nguyen  wrote:
> On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder
>  wrote:
>> Everytime split index is turned on, it creates a "sharedindex."
>> file in the git directory. This makes sure that old sharedindex
>> files are removed after a new one has been created.
>
> Hmm it's one-way link, we don't know how many index files use this
> shared index file, how can you be sure nobody else will need it? I'm
> thinking about temporary indexes. If a temp index is created, saved on
> disk, and use delete the shared index file, the real, main index may
> become useless. Temp index will most likely replace the main index
> (git commit) but if a failure happens, we can't fall back.

Isn't there a way to scan all the current indexes (temp or not) to see
which shared indexes they need?

> A safer approach is "touch" the shared index every time a linked index
> is used, then we can delete shared indexes with old mtime, older than
> a grace period, in git-prune (or here).

Maybe old linked indexes could be converted after some time to use a
newer shared index, so that we can get rid of the old shared indexes.
That seems safer than just deleting old linked indexes.

Thanks for your review,
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