Re: [PATCH 03/23] midx: add midx builtin
On 6/18/2018 3:55 PM, Stefan Beller wrote: But as this is plumbing and users should not need to worry about it this is optional, I would think. The design document is also in 'Documentation/technical' instead of just 'Documentation/'. Do we have a pattern of linking to the technical documents? Apparently we do (and I was not aware of it): $ git -C Documentation/ grep link:technical git-credential.txt:23:link:technical/api-credentials.html[the Git credential API] for more git.txt:839:link:technical/api-index.html[Git API documentation]. gitcredentials.txt:184:link:technical/api-credentials.html[credentials API] for details. technical/http-protocol.txt:517:link:technical/pack-protocol.html technical/http-protocol.txt:518:link:technical/protocol-capabilities.html user-manual.txt:3220:found in link:technical/pack-format.html[pack format]. Thanks! I'll add some links.
Re: [PATCH 03/23] midx: add midx builtin
> > But as this is plumbing and users should not need to worry about it > > this is optional, I would think. > > The design document is also in 'Documentation/technical' instead of just > 'Documentation/'. Do we have a pattern of linking to the technical > documents? Apparently we do (and I was not aware of it): $ git -C Documentation/ grep link:technical git-credential.txt:23:link:technical/api-credentials.html[the Git credential API] for more git.txt:839:link:technical/api-index.html[Git API documentation]. gitcredentials.txt:184:link:technical/api-credentials.html[credentials API] for details. technical/http-protocol.txt:517:link:technical/pack-protocol.html technical/http-protocol.txt:518:link:technical/protocol-capabilities.html user-manual.txt:3220:found in link:technical/pack-format.html[pack format].
Re: [PATCH 03/23] midx: add midx builtin
On 6/11/2018 5:02 PM, Stefan Beller wrote: Hi Derrick, On Thu, Jun 7, 2018 at 7:03 AM Derrick Stolee wrote: This new 'git midx' builtin will be the plumbing access for writing, reading, and checking multi-pack-index (MIDX) files. The initial implementation is a no-op. Let's talk about the name for a second: .idx files are written by git-index-pack or as part of git-pack-objects (which just calls write_idx_file as part of finish_tmp_packfile), and the name actually suggests it writes the index files. I have a hard time understanding what the git-midx command does[1]. With both commit graph as well as multi index we introduce a command that is centered around that concept (similar to git-remote or git-config that are centered around a concept, that is closely resembled by a file), but for indexes for packs it was integrated differently into Git. So I am not sure if I want to suggest to integrate it into the packfile commands as that doesn't really fit. But maybe we can have a name that is human readable instead of the file suffix? Maybe git multi-pack-index ? I suppose that eventually this command is not really used by users as it will be used by other porcelain commands in the background or even as part of repack/gc so I am not worried about a long name, but I'd be more worried about understandability. I'll use "git multi-pack-index" in v2. I'll keep "midx.c" in the root, though, if that is OK. [1] While these names are not perfect for the layman, it is okay? I am sure you are aware of https://git-man-page-generator.lokaltog.net/ I was not, and enjoyed that quite a bit. Thanks, -Stolee new file mode 100644 index 00..2bd886f1a2 --- /dev/null +++ b/Documentation/git-midx.txt @@ -0,0 +1,29 @@ +git-midx(1) + + +NAME + +git-midx - Write and verify multi-pack-indexes (MIDX files). The reading is done as part of all other commands. I like to think the 'read' verb is a subset of "verify" because we are checking for information about the MIDX, and mostly for tests or debugging. + + +SYNOPSIS + +[verse] +'git midx' [--object-dir ] + +DESCRIPTION +--- +Write or verify a MIDX file. + +OPTIONS +--- + +--object-dir :: + Use given directory for the location of Git objects. We check + /packs/multi-pack-index for the current MIDX file, and + /packs for the pack-files to index. + + Maybe we could have a SEE ALSO section that points at the explanation of multi index files? (c.f. man git-submodule that has a SEE ALSO gitsubmodules(7), gitmodules(5) explaining concepts(7) and the file(5)) But as this is plumbing and users should not need to worry about it this is optional, I would think. The design document is also in 'Documentation/technical' instead of just 'Documentation/'. Do we have a pattern of linking to the technical documents? Thanks, -Stolee
Re: [PATCH 03/23] midx: add midx builtin
On 6/7/2018 1:20 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 4:03 PM, Derrick Stolee wrote:
diff --git a/Documentation/git-midx.txt b/Documentation/git-midx.txt
new file mode 100644
index 00..2bd886f1a2
--- /dev/null
+++ b/Documentation/git-midx.txt
@@ -0,0 +1,29 @@
+git-midx(1)
+
+
+NAME
+
+git-midx - Write and verify multi-pack-indexes (MIDX files).
No full stop. This head line is collected automatically with others
and its having a full stop while the rest does not looks strange/
diff --git a/builtin/midx.c b/builtin/midx.c
new file mode 100644
index 00..59ea92178f
--- /dev/null
+++ b/builtin/midx.c
@@ -0,0 +1,38 @@
+#include "builtin.h"
+#include "cache.h"
+#include "config.h"
+#include "git-compat-util.h"
You only need either cache.h or git-compat-util.h. If cache.h is here,
git-compat-util can be removed.
+#include "parse-options.h"
+
+static char const * const builtin_midx_usage[] ={
+ N_("git midx [--object-dir ]"),
+ NULL
+};
+
+static struct opts_midx {
+ const char *object_dir;
+} opts;
+
+int cmd_midx(int argc, const char **argv, const char *prefix)
+{
+ static struct option builtin_midx_options[] = {
+ { OPTION_STRING, 0, "object-dir", _dir,
For paths (including dir), OPTION_FILENAME may be a better option to
handle correctly when the command is run in a subdir. See df217ed643
(parse-opts: add OPT_FILENAME and transition builtins - 2009-05-23)
for more info.
Thanks for the pointer!
+ N_("dir"),
+ N_("The object directory containing set of packfile and pack-index
pairs.") },
Other help strings do not have full stop either (I only checked a
couple commands though)
Also, doesn't OPT_STRING() work here too (if you avoid OPTION_FILENAME
for some reason)?
+ OPT_END(),
+ };
+
+ if (argc == 2 && !strcmp(argv[1], "-h"))
+ usage_with_options(builtin_midx_usage, builtin_midx_options);
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix,
+builtin_midx_options,
+builtin_midx_usage, 0);
+
+ if (!opts.object_dir)
+ opts.object_dir = get_object_directory();
+
+ return 0;
+}
diff --git a/git.c b/git.c
index c2f48d53dd..400fadd677 100644
--- a/git.c
+++ b/git.c
@@ -503,6 +503,7 @@ static struct cmd_struct commands[] = {
{ "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP |
NEED_WORK_TREE | NO_PARSEOPT },
{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE |
NO_PARSEOPT },
{ "merge-tree", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT },
+ { "midx", cmd_midx, RUN_SETUP },
If it's a plumbing and can take an --object-dir, then I don't think
you should require it to run in a repo (with RUN_SETUP).
RUN_SETUP_GENTLY may be better. You could even leave it empty here and
only call setup_git_directory() only when --object-dir is not set.
I agree. Good point. This could be run to maintain an alternate without
any .git folder.
{ "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT },
{ "mktree", cmd_mktree, RUN_SETUP },
{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
Re: [PATCH 03/23] midx: add midx builtin
Hi Derrick, On Thu, Jun 7, 2018 at 7:03 AM Derrick Stolee wrote: > > This new 'git midx' builtin will be the plumbing access for writing, > reading, and checking multi-pack-index (MIDX) files. The initial > implementation is a no-op. Let's talk about the name for a second: .idx files are written by git-index-pack or as part of git-pack-objects (which just calls write_idx_file as part of finish_tmp_packfile), and the name actually suggests it writes the index files. I have a hard time understanding what the git-midx command does[1]. With both commit graph as well as multi index we introduce a command that is centered around that concept (similar to git-remote or git-config that are centered around a concept, that is closely resembled by a file), but for indexes for packs it was integrated differently into Git. So I am not sure if I want to suggest to integrate it into the packfile commands as that doesn't really fit. But maybe we can have a name that is human readable instead of the file suffix? Maybe git multi-pack-index ? I suppose that eventually this command is not really used by users as it will be used by other porcelain commands in the background or even as part of repack/gc so I am not worried about a long name, but I'd be more worried about understandability. [1] While these names are not perfect for the layman, it is okay? I am sure you are aware of https://git-man-page-generator.lokaltog.net/ > new file mode 100644 > index 00..2bd886f1a2 > --- /dev/null > +++ b/Documentation/git-midx.txt > @@ -0,0 +1,29 @@ > +git-midx(1) > + > + > +NAME > + > +git-midx - Write and verify multi-pack-indexes (MIDX files). The reading is done as part of all other commands. > + > + > +SYNOPSIS > + > +[verse] > +'git midx' [--object-dir ] > + > +DESCRIPTION > +--- > +Write or verify a MIDX file. > + > +OPTIONS > +--- > + > +--object-dir :: > + Use given directory for the location of Git objects. We check > + /packs/multi-pack-index for the current MIDX file, and > + /packs for the pack-files to index. > + > + Maybe we could have a SEE ALSO section that points at the explanation of multi index files? (c.f. man git-submodule that has a SEE ALSO gitsubmodules(7), gitmodules(5) explaining concepts(7) and the file(5)) But as this is plumbing and users should not need to worry about it this is optional, I would think.
Re: [PATCH 03/23] midx: add midx builtin
On Thu, Jun 7, 2018 at 4:03 PM, Derrick Stolee wrote:
> diff --git a/Documentation/git-midx.txt b/Documentation/git-midx.txt
> new file mode 100644
> index 00..2bd886f1a2
> --- /dev/null
> +++ b/Documentation/git-midx.txt
> @@ -0,0 +1,29 @@
> +git-midx(1)
> +
> +
> +NAME
> +
> +git-midx - Write and verify multi-pack-indexes (MIDX files).
No full stop. This head line is collected automatically with others
and its having a full stop while the rest does not looks strange/
> diff --git a/builtin/midx.c b/builtin/midx.c
> new file mode 100644
> index 00..59ea92178f
> --- /dev/null
> +++ b/builtin/midx.c
> @@ -0,0 +1,38 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "config.h"
> +#include "git-compat-util.h"
You only need either cache.h or git-compat-util.h. If cache.h is here,
git-compat-util can be removed.
> +#include "parse-options.h"
> +
> +static char const * const builtin_midx_usage[] ={
> + N_("git midx [--object-dir ]"),
> + NULL
> +};
> +
> +static struct opts_midx {
> + const char *object_dir;
> +} opts;
> +
> +int cmd_midx(int argc, const char **argv, const char *prefix)
> +{
> + static struct option builtin_midx_options[] = {
> + { OPTION_STRING, 0, "object-dir", _dir,
For paths (including dir), OPTION_FILENAME may be a better option to
handle correctly when the command is run in a subdir. See df217ed643
(parse-opts: add OPT_FILENAME and transition builtins - 2009-05-23)
for more info.
> + N_("dir"),
> + N_("The object directory containing set of packfile and
> pack-index pairs.") },
Other help strings do not have full stop either (I only checked a
couple commands though)
Also, doesn't OPT_STRING() work here too (if you avoid OPTION_FILENAME
for some reason)?
> + OPT_END(),
> + };
> +
> + if (argc == 2 && !strcmp(argv[1], "-h"))
> + usage_with_options(builtin_midx_usage, builtin_midx_options);
> +
> + git_config(git_default_config, NULL);
> +
> + argc = parse_options(argc, argv, prefix,
> +builtin_midx_options,
> +builtin_midx_usage, 0);
> +
> + if (!opts.object_dir)
> + opts.object_dir = get_object_directory();
> +
> + return 0;
> +}
> diff --git a/git.c b/git.c
> index c2f48d53dd..400fadd677 100644
> --- a/git.c
> +++ b/git.c
> @@ -503,6 +503,7 @@ static struct cmd_struct commands[] = {
> { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP |
> NEED_WORK_TREE | NO_PARSEOPT },
> { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE |
> NO_PARSEOPT },
> { "merge-tree", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT },
> + { "midx", cmd_midx, RUN_SETUP },
If it's a plumbing and can take an --object-dir, then I don't think
you should require it to run in a repo (with RUN_SETUP).
RUN_SETUP_GENTLY may be better. You could even leave it empty here and
only call setup_git_directory() only when --object-dir is not set.
> { "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT },
> { "mktree", cmd_mktree, RUN_SETUP },
> { "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
--
Duy
[PATCH 03/23] midx: add midx builtin
This new 'git midx' builtin will be the plumbing access for writing,
reading, and checking multi-pack-index (MIDX) files. The initial
implementation is a no-op.
Signed-off-by: Derrick Stolee
---
.gitignore | 1 +
Documentation/git-midx.txt | 29 +
Makefile | 1 +
builtin.h | 1 +
builtin/midx.c | 38 ++
command-list.txt | 1 +
git.c | 1 +
7 files changed, 72 insertions(+)
create mode 100644 Documentation/git-midx.txt
create mode 100644 builtin/midx.c
diff --git a/.gitignore b/.gitignore
index 388cc4beee..e309644d6b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,6 +97,7 @@
/git-merge-subtree
/git-mergetool
/git-mergetool--lib
+/git-midx
/git-mktag
/git-mktree
/git-name-rev
diff --git a/Documentation/git-midx.txt b/Documentation/git-midx.txt
new file mode 100644
index 00..2bd886f1a2
--- /dev/null
+++ b/Documentation/git-midx.txt
@@ -0,0 +1,29 @@
+git-midx(1)
+
+
+NAME
+
+git-midx - Write and verify multi-pack-indexes (MIDX files).
+
+
+SYNOPSIS
+
+[verse]
+'git midx' [--object-dir ]
+
+DESCRIPTION
+---
+Write or verify a MIDX file.
+
+OPTIONS
+---
+
+--object-dir ::
+ Use given directory for the location of Git objects. We check
+ /packs/multi-pack-index for the current MIDX file, and
+ /packs for the pack-files to index.
+
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 1d27f36365..88958c7b42 100644
--- a/Makefile
+++ b/Makefile
@@ -1045,6 +1045,7 @@ BUILTIN_OBJS += builtin/merge-index.o
BUILTIN_OBJS += builtin/merge-ours.o
BUILTIN_OBJS += builtin/merge-recursive.o
BUILTIN_OBJS += builtin/merge-tree.o
+BUILTIN_OBJS += builtin/midx.o
BUILTIN_OBJS += builtin/mktag.o
BUILTIN_OBJS += builtin/mktree.o
BUILTIN_OBJS += builtin/mv.o
diff --git a/builtin.h b/builtin.h
index 4e0f64723e..7b5bd46c7d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -189,6 +189,7 @@ extern int cmd_merge_ours(int argc, const char **argv,
const char *prefix);
extern int cmd_merge_file(int argc, const char **argv, const char *prefix);
extern int cmd_merge_recursive(int argc, const char **argv, const char
*prefix);
extern int cmd_merge_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_midx(int argc, const char **argv, const char *prefix);
extern int cmd_mktag(int argc, const char **argv, const char *prefix);
extern int cmd_mktree(int argc, const char **argv, const char *prefix);
extern int cmd_mv(int argc, const char **argv, const char *prefix);
diff --git a/builtin/midx.c b/builtin/midx.c
new file mode 100644
index 00..59ea92178f
--- /dev/null
+++ b/builtin/midx.c
@@ -0,0 +1,38 @@
+#include "builtin.h"
+#include "cache.h"
+#include "config.h"
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+static char const * const builtin_midx_usage[] ={
+ N_("git midx [--object-dir ]"),
+ NULL
+};
+
+static struct opts_midx {
+ const char *object_dir;
+} opts;
+
+int cmd_midx(int argc, const char **argv, const char *prefix)
+{
+ static struct option builtin_midx_options[] = {
+ { OPTION_STRING, 0, "object-dir", _dir,
+ N_("dir"),
+ N_("The object directory containing set of packfile and
pack-index pairs.") },
+ OPT_END(),
+ };
+
+ if (argc == 2 && !strcmp(argv[1], "-h"))
+ usage_with_options(builtin_midx_usage, builtin_midx_options);
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix,
+builtin_midx_options,
+builtin_midx_usage, 0);
+
+ if (!opts.object_dir)
+ opts.object_dir = get_object_directory();
+
+ return 0;
+}
diff --git a/command-list.txt b/command-list.txt
index e1c26c1bb7..a21bd7470e 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -123,6 +123,7 @@ git-merge-index plumbingmanipulators
git-merge-one-file purehelpers
git-mergetool ancillarymanipulators
complete
git-merge-tree ancillaryinterrogators
+git-midxplumbingmanipulators
git-mktag plumbingmanipulators
git-mktree plumbingmanipulators
git-mv mainporcelain worktree
diff --git a/git.c b/git.c
index c2f48d53dd..400fadd677 100644
--- a/git.c
+++ b/git.c
@@ -503,6 +503,7 @@ static struct cmd_struct commands[] = {
{ "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP |
NEED_WORK_TREE | NO_PARSEOPT },
{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE |
NO_PARSEOPT },
{ "merge-tree", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT },
+ { "midx",
