Re: [PATCH v7 20/22] commit-graph: add '--reachable' option

2018-09-11 Thread Christian Couder
On Tue, Sep 11, 2018 at 1:19 PM, Derrick Stolee  wrote:
> On 9/11/2018 1:22 AM, Christian Couder wrote:

>> It would be nice if the "Future Work" section of
>> Documentation/technical/commit-graph.txt had something about
>> integration with 'git gc'.
>
> I'm a bit confused about this statement, because at the point in time of
> this patch we had a spot in the "Future Work" section about automatically
> updating the graph _somewhere_. The "future integration with 'git gc'" I
> refer to in this patch is implemented in PATCH 21/22. In PATCH 22/22 we
> removed this section from the technical doc:

Yeah, sorry I had not seen PATCH 21/22. I was interested in an update
about the commit graph feature and especially to know if the way to
use it had been simplified in v2.19.0, so I took a look at the
Documentation/technical/commit-graph.txt and
Documentation/git-commit-graph.txt, but I didn't find anything there
except the `--reachable` option. So I thought that integrating with gc
was still something that needed to be worked on.

> Now that this feature is shipped in Git 2.19.0, this no longer belongs in
> "Future Work".

This is great!

It would be nice though to have something in
Documentation/technical/commit-graph.txt and perhaps also
Documentation/git-commit-graph.txt about it, as it is easy to overlook
the gc.writeCommitGraph documentation in Documentation/config.txt.

>> The "EXAMPLES" section still contains:
>>
>> * Write a graph file containing all reachable commits.
>> +
>> 
>> $ git show-ref -s | git commit-graph write --stdin-commits
>> 
>>
>> I wonder if this should have been changed to use `--reachable`.
>>
>> Thanks!
>
> This is a good idea. I can work on that.

Thanks,
Christian.


Re: [PATCH v7 20/22] commit-graph: add '--reachable' option

2018-09-11 Thread Derrick Stolee

On 9/11/2018 1:22 AM, Christian Couder wrote:

On Wed, Jun 27, 2018 at 3:24 PM, Derrick Stolee  wrote:

When writing commit-graph files, it can be convenient to ask for all
reachable commits (starting at the ref set) in the resulting file. This
is particularly helpful when writing to stdin is complicated, such as a
future integration with 'git gc'.

It would be nice if the "Future Work" section of
Documentation/technical/commit-graph.txt had something about
integration with 'git gc'.


Hi Christian,

I'm a bit confused about this statement, because at the point in time of 
this patch we had a spot in the "Future Work" section about 
automatically updating the graph _somewhere_. The "future integration 
with 'git gc'" I refer to in this patch is implemented in PATCH 21/22. 
In PATCH 22/22 we removed this section from the technical doc:


-
-- The current design uses the 'commit-graph' subcommand to generate the graph.
-  When this feature stabilizes enough to recommend to most users, we should
-  add automatic graph writes to common operations that create many commits.
-  For example, one could compute a graph on 'clone', 'fetch', or 'repack'
-  commands.
-

Now that this feature is shipped in Git 2.19.0, this no longer belongs 
in "Future Work".



  With the `--stdin-commits` option, generate the new commit graph by
  walking commits starting at the commits specified in stdin as a list
  of OIDs in hex, one OID per line. (Cannot be combined with
---stdin-packs.)
+`--stdin-packs` or `--reachable`.)
++
+With the `--reachable` option, generate the new commit graph by walking
+commits starting at all refs. (Cannot be combined with `--stdin-commits`
+or `--stdin-packs`.)
  +
  With the `--append` option, include all commits that are present in the
  existing commit-graph file.

The "EXAMPLES" section still contains:

* Write a graph file containing all reachable commits.
+

$ git show-ref -s | git commit-graph write --stdin-commits


I wonder if this should have been changed to use `--reachable`.

Thanks!

This is a good idea. I can work on that.

Thanks,
-Stolee


Re: [PATCH v7 20/22] commit-graph: add '--reachable' option

2018-09-10 Thread Christian Couder
On Wed, Jun 27, 2018 at 3:24 PM, Derrick Stolee  wrote:
> When writing commit-graph files, it can be convenient to ask for all
> reachable commits (starting at the ref set) in the resulting file. This
> is particularly helpful when writing to stdin is complicated, such as a
> future integration with 'git gc'.

It would be nice if the "Future Work" section of
Documentation/technical/commit-graph.txt had something about
integration with 'git gc'.

>  With the `--stdin-commits` option, generate the new commit graph by
>  walking commits starting at the commits specified in stdin as a list
>  of OIDs in hex, one OID per line. (Cannot be combined with
> ---stdin-packs.)
> +`--stdin-packs` or `--reachable`.)
> ++
> +With the `--reachable` option, generate the new commit graph by walking
> +commits starting at all refs. (Cannot be combined with `--stdin-commits`
> +or `--stdin-packs`.)
>  +
>  With the `--append` option, include all commits that are present in the
>  existing commit-graph file.

The "EXAMPLES" section still contains:

* Write a graph file containing all reachable commits.
+

$ git show-ref -s | git commit-graph write --stdin-commits


I wonder if this should have been changed to use `--reachable`.

Thanks!


Re: [PATCH v7 20/22] commit-graph: add '--reachable' option

2018-06-27 Thread Junio C Hamano
Derrick Stolee  writes:

> diff --git a/commit-graph.c b/commit-graph.c
> index a06e7e9549..adf54e3fe7 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -7,6 +7,7 @@
>  #include "packfile.h"
>  #include "commit.h"
>  #include "object.h"
> +#include "refs.h"
>  #include "revision.h"
>  #include "sha1-lookup.h"
>  #include "commit-graph.h"
> @@ -656,6 +657,23 @@ static void compute_generation_numbers(struct 
> packed_commit_list* commits)
>   }
>  }
>  
> +static int add_ref_to_list(const char *refname,
> +const struct object_id *oid,
> +int flags, void *cb_data)
> +{
> + struct string_list *list = (struct string_list*)cb_data;

Style: "(struct string_list *)cb_data"

Also please have a blank line here between the decls and the first
statement.

> + string_list_append(list, oid_to_hex(oid));
> + return 0;
> +}


[PATCH v7 20/22] commit-graph: add '--reachable' option

2018-06-27 Thread Derrick Stolee
When writing commit-graph files, it can be convenient to ask for all
reachable commits (starting at the ref set) in the resulting file. This
is particularly helpful when writing to stdin is complicated, such as a
future integration with 'git gc'.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt |  8 ++--
 builtin/commit-graph.c | 16 
 commit-graph.c | 18 ++
 commit-graph.h |  1 +
 t/t5318-commit-graph.sh| 10 ++
 5 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index a222cfab08..dececb79d7 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -38,12 +38,16 @@ Write a commit graph file based on the commits found in 
packfiles.
 +
 With the `--stdin-packs` option, generate the new commit graph by
 walking objects only in the specified pack-indexes. (Cannot be combined
-with --stdin-commits.)
+with `--stdin-commits` or `--reachable`.)
 +
 With the `--stdin-commits` option, generate the new commit graph by
 walking commits starting at the commits specified in stdin as a list
 of OIDs in hex, one OID per line. (Cannot be combined with
---stdin-packs.)
+`--stdin-packs` or `--reachable`.)
++
+With the `--reachable` option, generate the new commit graph by walking
+commits starting at all refs. (Cannot be combined with `--stdin-commits`
+or `--stdin-packs`.)
 +
 With the `--append` option, include all commits that are present in the
 existing commit-graph file.
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index ea28bc311a..c7d0db5ab4 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -10,7 +10,7 @@ static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
N_("git commit-graph read [--object-dir ]"),
N_("git commit-graph verify [--object-dir ]"),
-   N_("git commit-graph write [--object-dir ] [--append] 
[--stdin-packs|--stdin-commits]"),
+   N_("git commit-graph write [--object-dir ] [--append] 
[--reachable|--stdin-packs|--stdin-commits]"),
NULL
 };
 
@@ -25,12 +25,13 @@ static const char * const builtin_commit_graph_read_usage[] 
= {
 };
 
 static const char * const builtin_commit_graph_write_usage[] = {
-   N_("git commit-graph write [--object-dir ] [--append] 
[--stdin-packs|--stdin-commits]"),
+   N_("git commit-graph write [--object-dir ] [--append] 
[--reachable|--stdin-packs|--stdin-commits]"),
NULL
 };
 
 static struct opts_commit_graph {
const char *obj_dir;
+   int reachable;
int stdin_packs;
int stdin_commits;
int append;
@@ -127,6 +128,8 @@ static int graph_write(int argc, const char **argv)
OPT_STRING(0, "object-dir", _dir,
N_("dir"),
N_("The object directory to store the graph")),
+   OPT_BOOL(0, "reachable", ,
+   N_("start walk at all refs")),
OPT_BOOL(0, "stdin-packs", _packs,
N_("scan pack-indexes listed by stdin for commits")),
OPT_BOOL(0, "stdin-commits", _commits,
@@ -140,11 +143,16 @@ static int graph_write(int argc, const char **argv)
 builtin_commit_graph_write_options,
 builtin_commit_graph_write_usage, 0);
 
-   if (opts.stdin_packs && opts.stdin_commits)
-   die(_("cannot use both --stdin-commits and --stdin-packs"));
+   if (opts.reachable + opts.stdin_packs + opts.stdin_commits > 1)
+   die(_("use at most one of --reachable, --stdin-commits, or 
--stdin-packs"));
if (!opts.obj_dir)
opts.obj_dir = get_object_directory();
 
+   if (opts.reachable) {
+   write_commit_graph_reachable(opts.obj_dir, opts.append);
+   return 0;
+   }
+
string_list_init(, 0);
if (opts.stdin_packs || opts.stdin_commits) {
struct strbuf buf = STRBUF_INIT;
diff --git a/commit-graph.c b/commit-graph.c
index a06e7e9549..adf54e3fe7 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -7,6 +7,7 @@
 #include "packfile.h"
 #include "commit.h"
 #include "object.h"
+#include "refs.h"
 #include "revision.h"
 #include "sha1-lookup.h"
 #include "commit-graph.h"
@@ -656,6 +657,23 @@ static void compute_generation_numbers(struct 
packed_commit_list* commits)
}
 }
 
+static int add_ref_to_list(const char *refname,
+  const struct object_id *oid,
+  int flags, void *cb_data)
+{
+   struct string_list *list = (struct string_list*)cb_data;
+   string_list_append(list, oid_to_hex(oid));
+   return 0;
+}
+
+void write_commit_graph_reachable(const char *obj_dir, int append)
+{
+   struct string_list list;
+