* Re: [PATCH 2/2] describe: add support for multiple match patterns
From: Jacob Keller @ 2016-12-07 23:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqqa8c7wfxu.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 7, 2016 at 2:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> ... Suppose that you version all
>> your official releases such as "v1.2", "v1.3", "v1.4", "v2.1" and so on.
>> Now, you also have other tags which represent -rc releases and other
>> such tags. If you want to find the first major release that contains
>> a given commit you might try
>>
>> git describe --contains --match="v?.?" <commit>
>>
>> This will work as long as you have only single digits. But if you start
>> adding multiple digits, the pattern becomes not enough to match all the
>> tags you wanted while excluding the ones you didn't.
>
> Isn't what you really want for the use case a negative pattern,
> i.e. "I want ones that match v* but not the ones that match *-rc*",
> though?
That's another way of implementing it. I just went with straight
forward patterns that I was already using in sequence.
Basically, this started as a script to try each pattern in sequence,
but this is slow, cumbersome and easy to mess up.
You're suggesting just add a single second pattern that we will do
matches and discard any tag that matches that first?
I think I can implement that pretty easily, and it should have simpler
semantics. We can discard first, and then match what remains easily.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH v8 00/19] port branch.c to use ref-filter's printing options
From: Jacob Keller @ 2016-12-08 0:01 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Git mailing list, Junio C Hamano, Jakub Narębski
In-Reply-To: <20161207153627.1468-1-Karthik.188@gmail.com>
On Wed, Dec 7, 2016 at 7:36 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
> This is part of unification of the commands 'git tag -l, git branch -l
> and git for-each-ref'. This ports over branch.c to use ref-filter's
> printing options.
>
> Initially posted here: $(gmane/279226). It was decided that this series
> would follow up after refactoring ref-filter parsing mechanism, which
> is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c).
>
> v1 can be found here: $(gmane/288342)
> v2 can be found here: $(gmane/288863)
> v3 can be found here: $(gmane/290299)
> v4 can be found here: $(gmane/291106)
> v5b can be found here: $(gmane/292467)
> v6 can be found here: http://marc.info/?l=git&m=146330914118766&w=2
> v7 can be found here: http://marc.info/?l=git&m=147863593317362&w=2
>
> Changes in this version:
>
> 1. use an enum for holding the comparision type in
> %(if:[equals/notequals=...]) options.
> 2. rename the 'strip' option to 'lstrip' and introduce an 'rstrip'
> option. Also modify them to take negative values. This drops the
> ':dri' and ':base' options.
> 3. Drop unecessary code.
> 4. Cleanup code and fix spacing.
> 5. Add more comments wherever required.
> 6. Add quote_literal_for_format(const char *s) for safer string
> insertions in branch.c:build_format().
>
> Thanks to Jacob, Jackub, Junio and Matthieu for their inputs on the
> previous version.
>
> Interdiff below.
>
> Karthik Nayak (19):
> ref-filter: implement %(if), %(then), and %(else) atoms
> ref-filter: include reference to 'used_atom' within 'atom_value'
> ref-filter: implement %(if:equals=<string>) and
> %(if:notequals=<string>)
> ref-filter: modify "%(objectname:short)" to take length
> ref-filter: move get_head_description() from branch.c
> ref-filter: introduce format_ref_array_item()
> ref-filter: make %(upstream:track) prints "[gone]" for invalid
> upstreams
> ref-filter: add support for %(upstream:track,nobracket)
> ref-filter: make "%(symref)" atom work with the ':short' modifier
> ref-filter: introduce refname_atom_parser_internal()
> ref-filter: introduce refname_atom_parser()
> ref-filter: make remote_ref_atom_parser() use
> refname_atom_parser_internal()
> ref-filter: rename the 'strip' option to 'lstrip'
> ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
> ref-filter: add an 'rstrip=<N>' option to atoms which deal with
> refnames
> ref-filter: allow porcelain to translate messages in the output
> branch, tag: use porcelain output
> branch: use ref-filter printing APIs
> branch: implement '--format' option
>
> Documentation/git-branch.txt | 7 +-
> Documentation/git-for-each-ref.txt | 86 +++++--
> builtin/branch.c | 290 +++++++---------------
> builtin/tag.c | 6 +-
> ref-filter.c | 488 +++++++++++++++++++++++++++++++------
> ref-filter.h | 7 +
> t/t3203-branch-output.sh | 16 +-
> t/t6040-tracking-info.sh | 2 +-
> t/t6300-for-each-ref.sh | 88 ++++++-
> t/t6302-for-each-ref-filter.sh | 94 +++++++
> 10 files changed, 784 insertions(+), 300 deletions(-)
>
> Interdiff:
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index f4ad297..c72baeb 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -92,13 +92,14 @@ refname::
> The name of the ref (the part after $GIT_DIR/).
> For a non-ambiguous short name of the ref append `:short`.
> The option core.warnAmbiguousRefs is used to select the strict
> - abbreviation mode. If `strip=<N>` is appended, strips `<N>`
> - slash-separated path components from the front of the refname
> - (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
> - `<N>` must be a positive integer. If a displayed ref has fewer
> - components than `<N>`, the command aborts with an error. For the base
> - directory of the ref (i.e. foo in refs/foo/bar/boz) append
> - `:base`. For the entire directory path append `:dir`.
> + abbreviation mode. If `lstrip=<N>` or `rstrip=<N>` option can
Grammar here, drop the If before `lstrip since you're referring to
multiples and you say "x can be appended to y" rather than "if x is
added, do y"
> + be appended to strip `<N>` slash-separated path components
> + from or end of the refname respectively (e.g.,
> + `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo` and
> + `%(refname:rstrip=2)` turns `refs/tags/foo` into `refs`). if
> + `<N>` is a negative number, then only `<N>` path components
> + are left behind. If a displayed ref has fewer components than
> + `<N>`, the command aborts with an error.
>
Would it make more sense to not die and instead just return the empty
string? On the one hand, if we die() it's obvious that you tried to
strip too many components. But on the other hand, it's also somewhat
annoying to have the whole command fail because we happen upon a
single ref that has fewer components?
So, for positive numbers, we simply strip what we can, which may
result in the empty string, and for negative numbers, we keep up to
what we said, while potentially keeping the entire string. I feel
that's a better alternative than a die() in the middle of a ref
filter..
What are other people's thoughts on this?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules
From: Stefan Beller @ 2016-12-08 0:02 UTC (permalink / raw)
To: vi0oss; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20161207224948.7957-1-vi0oss@gmail.com>
On Wed, Dec 7, 2016 at 2:49 PM, <vi0oss@gmail.com> wrote:
> Notes:
> Resolved issues pointed by Stefan Beller except of
> the one about loosened path check, which he aggreed
> to be relaxed for this case.
I am sorry to have given an incomplete review at the first time. :/
More below.
> + /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
> + git_config_get_string("submodule.alternateLocation", &sm_alternate);
> + if (sm_alternate) {
> + git_config_set_in_file(p, "submodule.alternateLocation",
> + sm_alternate);
> + }
For a single command, we usually omit the braces for
an if clause, i.e.
if (foo)
bar(...);
/* code goes on */
> + git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
> + if (error_strategy) {
> + git_config_set_in_file(p, "submodule.alternateErrorStrategy",
> + error_strategy);
> + }
> +
> + free(sm_alternate);
> + free(error_strategy);
> +
The indentation seems a bit off for the free here?
(The main nit that motivated me writing the email)
> strbuf_release(&sb);
> strbuf_release(&rel_path);
> free(sm_gitdir);
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index 1c1e289..ef7771b 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -125,4 +125,76 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
> )
> '
>
> +test_expect_success 'preparing second superproject with a nested submodule' '
> + test_create_repo supersuper &&
> + (
> + cd supersuper &&
> + echo "I am super super." >file &&
> + git add file &&
> + git commit -m B-super-super-initial
> + git submodule add "file://$base_dir/super" subwithsub &&
> + git commit -m B-super-super-added &&
> + git submodule update --init --recursive &&
> + git repack -ad
> + )
> +'
> +
> +# At this point there are three root-level positories: A, B, super and super2
> +
> +test_expect_success 'nested submodule alternate in works and is actually used' '
> + test_when_finished "rm -rf supersuper-clone" &&
> + git clone --recursive --reference supersuper supersuper supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # immediate submodule has alternate:
> + test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> + # nested submodule also has alternate:
> + test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
> +'
> +
> +test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
> + test_when_finished "rm -rf supersuper-clone supersuper2" &&
> + git clone supersuper supersuper2 &&
> + (
> + cd supersuper2 &&
> + git submodule update --init
> + ) &&
> + test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # update of the submodule fails
> + test_must_fail git submodule update --init --recursive &&
> + # immediate submodule has alternate:
> + test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> + # but nested submodule has no alternate:
> + test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
> +'
> +
> +test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
> + test_when_finished "rm -rf supersuper-clone supersuper2" &&
> + git clone supersuper supersuper2 &&
> + (
> + cd supersuper2 &&
> + git submodule update --init
> + ) &&
> + git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # update of the submodule fails
> + test_must_fail git submodule update --init --recursive &&
> + # immediate submodule has alternate:
> + test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> + # but nested submodule has no alternate:
> + test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
Both the first and the last part are the same in the last two tests,
the only difference is the line with git clone --reference ...
Maybe we can use a function somehow to make this a bit more
obvious?
Otherwise the tests look good to me.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 02/17] dir: convert create_simplify to use the pathspec struct interface
From: Brandon Williams @ 2016-12-08 0:03 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <CACsJy8AX09pxkyUkLU905v1MpXocLzV5bK0APuNmMUNb50Lavg@mail.gmail.com>
On 12/07, Duy Nguyen wrote:
> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> > Convert 'create_simplify()' to use the pathspec struct interface from
> > using the '_raw' entry in the pathspec.
>
> It would be even better to kill this create_simplify() and let
> simplify_away() handle struct pathspec directly.
>
> There is a bug in this code, that might have been found if we
> simpify_away() handled pathspec directly: the memcmp() in
> simplify_away() will not play well with :(icase) magic. My bad. If
> :(icase) is used, the easiest/safe way is simplify nothing. Later on
> maybe we can teach simplify_away() to do strncasecmp instead. We could
> ignore exclude patterns there too (although not excluding is not a
> bug).
So are you implying that the simplify struct needs to be killed? That
way the pathspec struct itself is being passed around instead?
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 2/2] describe: add support for multiple match patterns
From: Junio C Hamano @ 2016-12-08 0:20 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CA+P7+xrPivwMzGhzKxu30jns+YvSQGXBKUc4JDmfbenTy27tZg@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> Basically, this started as a script to try each pattern in sequence,
> but this is slow, cumbersome and easy to mess up.
>
> You're suggesting just add a single second pattern that we will do
> matches and discard any tag that matches that first?
I am not suggesting anything. I was just trying to see how well what
was designed and implemented supports the use case that motivated
the feature. Think of it as a sanity check and review of the design.
> I think I can implement that pretty easily, and it should have simpler
> semantics. We can discard first, and then match what remains easily.
I actually think "multiple" and "negative" are orthogonal and both
are good things. If we are enhancing the filtering by refname
patterns to allow multiple patterns (i.e. your patch), that is good,
and it would be ideal if we can also have support for negative ones.
^ permalink raw reply
* Re: [PATCH 01/17] mv: convert to using pathspec struct interface
From: Brandon Williams @ 2016-12-08 0:36 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <CACsJy8Bm+Nif_PL1=BzTYLKGrnz94rjKOB=_PXz6OB47Js6v2A@mail.gmail.com>
On 12/07, Duy Nguyen wrote:
> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> > Convert the 'internal_copy_pathspec()' function to use the pathspec
> > struct interface from using the deprecated 'get_pathspec()' interface.
> >
> > In addition to this, fix a memory leak caused by only duplicating some
> > of the pathspec elements. Instead always duplicate all of the the
> > pathspec elements as an intermediate step (with modificationed based on
> > the passed in flags). This way the intermediate strings can then be
> > freed prior to duplicating the result of parse_pathspec (which contains
> > each of the elements with the prefix prepended).
> >
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> > builtin/mv.c | 45 ++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 32 insertions(+), 13 deletions(-)
>
> Stefan did something similar last year [1] but I couldn't find why it
> did not get merged. Not sure if the reasons are still relevant or
> not...
>
> [1] http://public-inbox.org/git/%3C1438885632-26470-1-git-send-email-sbeller@google.com%3E/
>
> > diff --git a/builtin/mv.c b/builtin/mv.c
> > index 2f43877..4df4a12 100644
> > --- a/builtin/mv.c
> > +++ b/builtin/mv.c
> > @@ -4,6 +4,7 @@
> > * Copyright (C) 2006 Johannes Schindelin
> > */
> > #include "builtin.h"
> > +#include "pathspec.h"
> > #include "lockfile.h"
> > #include "dir.h"
> > #include "cache-tree.h"
> > @@ -25,25 +26,43 @@ static const char **internal_copy_pathspec(const char *prefix,
> > {
> > int i;
> > const char **result;
> > + struct pathspec ps;
> > ALLOC_ARRAY(result, count + 1);
> > - COPY_ARRAY(result, pathspec, count);
> > - result[count] = NULL;
> > +
> > + /* Create an intermediate copy of the pathspec based on the flags */
> > for (i = 0; i < count; i++) {
> > - int length = strlen(result[i]);
> > + int length = strlen(pathspec[i]);
> > int to_copy = length;
> > + char *it;
> > while (!(flags & KEEP_TRAILING_SLASH) &&
> > - to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
> > + to_copy > 0 && is_dir_sep(pathspec[i][to_copy - 1]))
> > to_copy--;
> > - if (to_copy != length || flags & DUP_BASENAME) {
> > - char *it = xmemdupz(result[i], to_copy);
> > - if (flags & DUP_BASENAME) {
> > - result[i] = xstrdup(basename(it));
> > - free(it);
> > - } else
> > - result[i] = it;
> > - }
> > +
> > + it = xmemdupz(pathspec[i], to_copy);
> > + if (flags & DUP_BASENAME) {
> > + result[i] = xstrdup(basename(it));
> > + free(it);
> > + } else
> > + result[i] = it;
> > + }
> > + result[count] = NULL;
> > +
> > + parse_pathspec(&ps,
> > + PATHSPEC_ALL_MAGIC &
> > + ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
> > + PATHSPEC_KEEP_ORDER | PATHSPEC_PREFER_CWD,
> > + prefix, result);
> > + assert(count == ps.nr);
> > +
> > + /* Copy the pathspec and free the old intermediate strings */
> > + for (i = 0; i < count; i++) {
> > + const char *match = xstrdup(ps.items[i].match);
> > + free((char *) result[i]);
> > + result[i] = match;
>
> Sigh.. it looks so weird that we do all the parsing (in a _copy_
> pathspec function) then remove struct pathspec and return the plain
> string. I guess we can't do anything more until we rework cmd_mv code
> to handle pathspec natively.
>
> At the least I think we should rename this function to something else.
> But if you have time I really wish we could kill this function. I
> haven't stared at cmd_mv() long and hard, but it looks to me that we
> combining two separate functionalities in the same function here.
>
> If "mv" takes n arguments, then the first <n-1> arguments may be
> pathspec, the last one is always a plain path. The "dest_path =
> internal_copy_pathspec..." could be as simple as "dest_path =
> prefix_path(argv[argc - 1])". the special treatment for this last
> argument [1] can live here. Then, we can do parse_pathspec for the
> <n-1> arguments in cmd_mv(). It's still far from perfect, because
> cmd_mv can't handle pathspec properly, but it reduces the messy mess
> in internal_copy_pathspec a bit, I hope.
>
> [1] c57f628 (mv: let 'git mv file no-such-dir/' error out - 2013-12-03)
>
> > }
> > - return get_pathspec(prefix, result);
> > +
> > + clear_pathspec(&ps);
> > + return result;
> > }
> >
> > static const char *add_slash(const char *path)
> > --
> > 2.8.0.rc3.226.g39d4020
> >
>
>
>
> --
> Duy
Actually, after looking at this a bit more it seems like we could
technically use prefix_path for both source and dest (based on how the
current code is structured) since the source's provied must all exist (as
in no wildcards are allowed). We could drop using the pathspec struct
completely in addition to renaming the function (to what I'm still
unsure). I agree that this code should probably be rewritten and made a
bit cleaner, I don't know if that fits in the scope of this series or
should be done as a followup patch. If you think it fits here then I
can try and find some time to do the rework.
--
Brandon Williams
^ permalink raw reply
* [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-08 0:39 UTC (permalink / raw)
To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela
From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>
In 31224cbdc7 (clone: recursive and reference option triggers
submodule alternates, 2016-08-17) a mechanism was added to
have submodules referenced. It did not address _nested_
submodules, however.
This patch makes all not just the root repository, but also
all submodules (recursively) have submodule.alternateLocation
and submodule.alternateErrorStrategy configured, making Git
search for possible alternates for nested submodules as well.
As submodule's alternate target does not end in .git/objects
(rather .git/modules/qqqqqq/objects), this alternate target
path restriction for in add_possible_reference_from_superproject
relates from "*.git/objects" to just */objects".
New tests have been added to t7408-submodule-reference.
Signed-off-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
---
Notes:
Another round of fixups:
* Corrected code style
* Refactored and fixed the test
Previously test contained errorneous
test_must_fail, which was masked by
missing &&.
Mailmap patch omitted this time.
builtin/submodule--helper.c | 19 ++++++++++--
t/t7408-submodule-reference.sh | 66 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..92fd676 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -498,9 +498,9 @@ static int add_possible_reference_from_superproject(
/*
* If the alternate object store is another repository, try the
- * standard layout with .git/modules/<name>/objects
+ * standard layout with .git/(modules/<name>)+/objects
*/
- if (ends_with(alt->path, ".git/objects")) {
+ if (ends_with(alt->path, "/objects")) {
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
@@ -583,6 +583,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
+ char *sm_alternate = NULL, *error_strategy = NULL;
struct option module_clone_options[] = {
OPT_STRING(0, "prefix", &prefix,
@@ -672,6 +673,20 @@ static int module_clone(int argc, const char **argv, const char *prefix)
die(_("could not get submodule directory for '%s'"), path);
git_config_set_in_file(p, "core.worktree",
relative_path(path, sm_gitdir, &rel_path));
+
+ /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
+ git_config_get_string("submodule.alternateLocation", &sm_alternate);
+ if (sm_alternate)
+ git_config_set_in_file(p, "submodule.alternateLocation",
+ sm_alternate);
+ git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
+ if (error_strategy)
+ git_config_set_in_file(p, "submodule.alternateErrorStrategy",
+ error_strategy);
+
+ free(sm_alternate);
+ free(error_strategy);
+
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1c1e289..9325297 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -125,4 +125,70 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
)
'
+test_expect_success 'preparing second superproject with a nested submodule plus partial clone' '
+ test_create_repo supersuper &&
+ (
+ cd supersuper &&
+ echo "I am super super." >file &&
+ git add file &&
+ git commit -m B-super-super-initial
+ git submodule add "file://$base_dir/super" subwithsub &&
+ git commit -m B-super-super-added &&
+ git submodule update --init --recursive &&
+ git repack -ad
+ ) &&
+ git clone supersuper supersuper2 &&
+ (
+ cd supersuper2 &&
+ git submodule update --init
+ )
+'
+
+# At this point there are three root-level positories: A, B, super and super2
+
+test_expect_success 'nested submodule alternate in works and is actually used' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference supersuper supersuper supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ # test superproject has alternates setup correctly
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
+ # nested submodule also has alternate:
+ test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+ )
+'
+
+check_that_two_of_three_alternates_are_used() {
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # but nested submodule has no alternate:
+ test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+}
+
+
+test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule fails
+ test_must_fail git submodule update --init --recursive
+ )
+'
+
+test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule succeeds
+ git submodule update --init --recursive
+ )
+'
+
test_done
--
2.10.2
^ permalink raw reply related
* Re: [PATCHv5 5/5] submodule: add embed-git-dir function
From: Brandon Williams @ 2016-12-08 1:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, pclouds, gitster
In-Reply-To: <20161207210157.18932-6-sbeller@google.com>
On 12/07, Stefan Beller wrote:
> + argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
> + "submodule--helper",
> + "embed-git-dirs", NULL);
check the spacing on these lines, looks like there is an extra space
before "submodule--helper"
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 2/2] describe: add support for multiple match patterns
From: Jacob Keller @ 2016-12-08 1:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqq60mvuv8v.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 7, 2016 at 4:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>> Basically, this started as a script to try each pattern in sequence,
>> but this is slow, cumbersome and easy to mess up.
>>
>> You're suggesting just add a single second pattern that we will do
>> matches and discard any tag that matches that first?
>
> I am not suggesting anything. I was just trying to see how well what
> was designed and implemented supports the use case that motivated
> the feature. Think of it as a sanity check and review of the design.
>
Makes sense.
>> I think I can implement that pretty easily, and it should have simpler
>> semantics. We can discard first, and then match what remains easily.
>
> I actually think "multiple" and "negative" are orthogonal and both
> are good things. If we are enhancing the filtering by refname
> patterns to allow multiple patterns (i.e. your patch), that is good,
> and it would be ideal if we can also have support for negative ones.
I can add support for negative matches pretty easily. I personally
don't see the value of "logical and" filters, ie to match only tags
that match all the given filters, though that does allow some other
forms of expression.
I do like the idea of negative filters, and I'll go ahead and work on
adding that as another extension.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCHv5 4/5] worktree: get worktrees from submodules
From: Brandon Williams @ 2016-12-08 1:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git@vger.kernel.org, Duy Nguyen
In-Reply-To: <CAGZ79kZiS9dx6QUOcFYh8sSNoVsrv2eNLXJd6X54UekzUiC8VQ@mail.gmail.com>
On 12/07, Stefan Beller wrote:
> On Wed, Dec 7, 2016 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> + submodule_common_dir = strbuf_detach(&sb, NULL);
> >> + ret = get_worktrees_internal(submodule_common_dir, flags);
> >> +
> >> + free(submodule_gitdir);
> >
> > This sequence felt somewhat unusual. I would have written this
> > without an extra variable, i.e.
> >
> > ret = get_worktrees_internal(sb.buf, flags);
> > strbuf_release(&sb);
>
> Yours is cleaner; I don't remember what I was thinking.
>
> Feel free to squash it in; in case a resend is needed I will do that.
Just make sure to leave that free in as it refers to another variable
(submodule_gitdir). It actually turns out there is a memory leak in the
original code because submodule_common_dir is never freed after being
detached from the strbuf.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: Stefan Beller @ 2016-12-08 1:22 UTC (permalink / raw)
To: vi0oss, Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20161208003940.28794-1-vi0oss@gmail.com>
On Wed, Dec 7, 2016 at 4:39 PM, <vi0oss@gmail.com> wrote:
>
> Previously test contained errorneous
> test_must_fail, which was masked by
> missing &&.
I wonder if we could make either
the test_must_fail intelligent to detect such a broken && call chain
or the test_expect_success macro to see for those broken chains.
Patch looks good to me except one very minor nit. :)
> +test_expect_success 'nested submodule alternate in works and is actually used' '
> + test_when_finished "rm -rf supersuper-clone" &&
> + git clone --recursive --reference supersuper supersuper supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # immediate submodule has alternate:
> + test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
here is a && missing ;)
^ permalink raw reply
* [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-08 1:38 UTC (permalink / raw)
To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela
From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>
In 31224cbdc7 (clone: recursive and reference option triggers
submodule alternates, 2016-08-17) a mechanism was added to
have submodules referenced. It did not address _nested_
submodules, however.
This patch makes all not just the root repository, but also
all submodules (recursively) have submodule.alternateLocation
and submodule.alternateErrorStrategy configured, making Git
search for possible alternates for nested submodules as well.
As submodule's alternate target does not end in .git/objects
(rather .git/modules/qqqqqq/objects), this alternate target
path restriction for in add_possible_reference_from_superproject
relates from "*.git/objects" to just */objects".
New tests have been added to t7408-submodule-reference.
Signed-off-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
---
Notes:
Third review: missing && in test fixed.
Mailmap change not included.
builtin/submodule--helper.c | 19 ++++++++++--
t/t7408-submodule-reference.sh | 66 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..92fd676 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -498,9 +498,9 @@ static int add_possible_reference_from_superproject(
/*
* If the alternate object store is another repository, try the
- * standard layout with .git/modules/<name>/objects
+ * standard layout with .git/(modules/<name>)+/objects
*/
- if (ends_with(alt->path, ".git/objects")) {
+ if (ends_with(alt->path, "/objects")) {
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
@@ -583,6 +583,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
+ char *sm_alternate = NULL, *error_strategy = NULL;
struct option module_clone_options[] = {
OPT_STRING(0, "prefix", &prefix,
@@ -672,6 +673,20 @@ static int module_clone(int argc, const char **argv, const char *prefix)
die(_("could not get submodule directory for '%s'"), path);
git_config_set_in_file(p, "core.worktree",
relative_path(path, sm_gitdir, &rel_path));
+
+ /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
+ git_config_get_string("submodule.alternateLocation", &sm_alternate);
+ if (sm_alternate)
+ git_config_set_in_file(p, "submodule.alternateLocation",
+ sm_alternate);
+ git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
+ if (error_strategy)
+ git_config_set_in_file(p, "submodule.alternateErrorStrategy",
+ error_strategy);
+
+ free(sm_alternate);
+ free(error_strategy);
+
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1c1e289..e159fc5 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -125,4 +125,70 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
)
'
+test_expect_success 'preparing second superproject with a nested submodule plus partial clone' '
+ test_create_repo supersuper &&
+ (
+ cd supersuper &&
+ echo "I am super super." >file &&
+ git add file &&
+ git commit -m B-super-super-initial
+ git submodule add "file://$base_dir/super" subwithsub &&
+ git commit -m B-super-super-added &&
+ git submodule update --init --recursive &&
+ git repack -ad
+ ) &&
+ git clone supersuper supersuper2 &&
+ (
+ cd supersuper2 &&
+ git submodule update --init
+ )
+'
+
+# At this point there are three root-level positories: A, B, super and super2
+
+test_expect_success 'nested submodule alternate in works and is actually used' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference supersuper supersuper supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ # test superproject has alternates setup correctly
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # nested submodule also has alternate:
+ test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+ )
+'
+
+check_that_two_of_three_alternates_are_used() {
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # but nested submodule has no alternate:
+ test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+}
+
+
+test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule fails
+ test_must_fail git submodule update --init --recursive
+ )
+'
+
+test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule succeeds
+ git submodule update --init --recursive
+ )
+'
+
test_done
--
2.10.2
^ permalink raw reply related
* [PATCHv6 0/7] submodule embedgitdirs
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
v6:
* renamed embedgitdirs to absorbgitdirs embedding may be interpreted as
embedding the git dir into the working directory, whereas absorbing sounds
more like the submodule is absorbed by the superproject, making the
submodule less independent
* Worktrees API offer uses_worktrees(void) and submodule_uses_worktree(path).
* moved the printing to stderr and one layer up (out of the pure
relocate_git_dir function).
* connect_... is in dir.h now.
v5:
* Add another layer of abstraction, i.e. the relocate_git_dir is only about
moving a git dir of one repository. The submodule specific stuff (e.g.
recursion into nested submodules) is in submodule.{c,h}
This was motivated by reviews on the series of checkout aware of submodules
building on top of this series, as we want to directly call the embed-git-dirs
function without the overhead of spawning a child process.
v4:
* rebuilt on top of nd/worktree-list-fixup
* fix and test behavior for un-init submodules (don't crash, rather do nothing)
* incorporated a "static" as pointed out by Ramsay
* use internal functions instead of duplicating code in worktree.c
(use get_common_dir_noenv for the submodule to actually get the common dir)
* fixed a memory leak in relocate_gitdir
v3:
* have a slightly more generic function "relocate_gitdir".
The recursion is strictly related to submodules, though.
* bail out if a submodule is using worktrees.
This also lays the groundwork for later doing the proper thing,
as worktree.h offers a function `get_submodule_worktrees(path)`
* nit by duy: use git_path instead of git_common_dir
v2:
* fixed commit message for patch:
"submodule: use absolute path for computing relative path connecting"
* a new patch "submodule helper: support super prefix"
* redid the final patch with more tests and fixing bugs along the way
* "test-lib-functions.sh: teach test_commit -C <dir>" unchanged
v1:
The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory.
So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.
The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.
Thanks,
Stefan
Stefan Beller (7):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
worktree: get worktrees from submodules
worktree: add function to check if worktrees are in use
move connect_work_tree_and_git_dir to dir.h
submodule: add absorb-git-dir function
Documentation/git-submodule.txt | 15 +++++
builtin/submodule--helper.c | 69 ++++++++++++++++----
dir.c | 38 +++++++++++
dir.h | 4 ++
git-submodule.sh | 7 +-
git.c | 2 +-
submodule.c | 127 ++++++++++++++++++++++++++++++-------
submodule.h | 5 +-
t/t7412-submodule-absorbgitdirs.sh | 101 +++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++--
worktree.c | 70 +++++++++++++++++---
worktree.h | 13 ++++
12 files changed, 418 insertions(+), 53 deletions(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply
* [PATCHv6 1/7] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
The current caller of connect_work_tree_and_git_dir passes
an absolute path for the `git_dir` parameter. In the future patch
we will also pass in relative path for `git_dir`. Extend the functionality
of connect_work_tree_and_git_dir to take relative paths for parameters.
We could work around this in the future patch by computing the absolute
path for the git_dir in the calling site, however accepting relative
paths for either parameter makes the API for this function much harder
to misuse.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
{
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- const char *real_work_tree = xstrdup(real_path(work_tree));
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, real_work_tree, &rel_path));
+ relative_path(real_git_dir, real_work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
+ relative_path(real_work_tree, real_git_dir,
&rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(real_work_tree);
+ free(real_git_dir);
}
int parallel_submodules(void)
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 2/7] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Just like main commands in Git, the submodule helper needs
access to the superproject prefix. Enable this in the git.c
but have its own fuse in the helper code by having a flag to
turn on the super prefix.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------
git.c | 2 +-
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..33676a57cf 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+#define SUPPORT_SUPER_PREFIX (1<<0)
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
+ int option;
};
static struct cmd_struct commands[] = {
- {"list", module_list},
- {"name", module_name},
- {"clone", module_clone},
- {"update-clone", update_clone},
- {"relative-path", resolve_relative_path},
- {"resolve-relative-url", resolve_relative_url},
- {"resolve-relative-url-test", resolve_relative_url_test},
- {"init", module_init},
- {"remote-branch", resolve_remote_submodule_branch}
+ {"list", module_list, 0},
+ {"name", module_name, 0},
+ {"clone", module_clone, 0},
+ {"update-clone", update_clone, 0},
+ {"relative-path", resolve_relative_path, 0},
+ {"resolve-relative-url", resolve_relative_url, 0},
+ {"resolve-relative-url-test", resolve_relative_url_test, 0},
+ {"init", module_init, 0},
+ {"remote-branch", resolve_remote_submodule_branch, 0},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
die(_("submodule--helper subcommand must be "
"called with a subcommand"));
- for (i = 0; i < ARRAY_SIZE(commands); i++)
- if (!strcmp(argv[1], commands[i].cmd))
+ for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ if (!strcmp(argv[1], commands[i].cmd)) {
+ if (get_super_prefix() &&
+ !(commands[i].option & SUPPORT_SUPER_PREFIX))
+ die("%s doesn't support --super-prefix",
+ commands[i].cmd);
return commands[i].fn(argc - 1, argv + 1, prefix);
+ }
+ }
die(_("'%s' is not a valid submodule--helper "
"subcommand"), argv[1]);
diff --git a/git.c b/git.c
index efa1059fe0..98dcf6c518 100644
--- a/git.c
+++ b/git.c
@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
- { "submodule--helper", cmd_submodule__helper, RUN_SETUP },
+ { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tag", cmd_tag, RUN_SETUP },
{ "unpack-file", cmd_unpack_file, RUN_SETUP },
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 4/7] worktree: get worktrees from submodules
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
In a later patch we want to move around the the git directory of
a submodule. Both submodules as well as worktrees are involved in
placing git directories at unusual places, so their functionality
may collide. To react appropriately to situations where worktrees
in submodules are in use, offer a new function to query the
worktrees for submodules.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 46 ++++++++++++++++++++++++++++++++++++----------
worktree.h | 6 ++++++
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/worktree.c b/worktree.c
index eb6121263b..75db689672 100644
--- a/worktree.c
+++ b/worktree.c
@@ -72,7 +72,7 @@ static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
/**
* get the main worktree
*/
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(const char *git_common_dir)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+ strbuf_add_absolute_path(&worktree_path, git_common_dir);
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+ strbuf_addf(&path, "%s/HEAD", git_common_dir);
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(const char *git_common_dir,
+ const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -112,7 +113,7 @@ static struct worktree *get_linked_worktree(const char *id)
if (!id)
die("Missing linked worktree name");
- strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
+ strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -125,7 +126,7 @@ static struct worktree *get_linked_worktree(const char *id)
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -167,7 +168,8 @@ static int compare_worktree(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
-struct worktree **get_worktrees(unsigned flags)
+static struct worktree **get_worktrees_internal(const char *git_common_dir,
+ unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -177,9 +179,9 @@ struct worktree **get_worktrees(unsigned flags)
list = xmalloc(alloc * sizeof(struct worktree *));
- list[counter++] = get_main_worktree();
+ list[counter++] = get_main_worktree(git_common_dir);
- strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+ strbuf_addf(&path, "%s/worktrees", git_common_dir);
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -188,7 +190,7 @@ struct worktree **get_worktrees(unsigned flags)
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(d->d_name))) {
+ if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -209,6 +211,30 @@ struct worktree **get_worktrees(unsigned flags)
return list;
}
+struct worktree **get_worktrees(unsigned flags)
+{
+ return get_worktrees_internal(get_git_common_dir(), flags);
+}
+
+struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
+{
+ char *submodule_gitdir;
+ struct strbuf sb = STRBUF_INIT;
+ struct worktree **ret;
+
+ submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+ if (!submodule_gitdir)
+ return NULL;
+
+ /* the env would be set for the superproject */
+ get_common_dir_noenv(&sb, submodule_gitdir);
+ ret = get_worktrees_internal(sb.buf, flags);
+
+ strbuf_release(&sb);
+ free(submodule_gitdir);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..157fbc4a66 100644
--- a/worktree.h
+++ b/worktree.h
@@ -27,6 +27,12 @@ struct worktree {
*/
extern struct worktree **get_worktrees(unsigned flags);
+/*
+ * Get the worktrees of a submodule named by `path`.
+ */
+extern struct worktree **get_submodule_worktrees(const char *path,
+ unsigned flags);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 7/7] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be absorbed
into the superprojects git directory.
The newly added code in this patch is structured such that other areas of
Git can also make use of it. The code in the submodule--helper is a mere
wrapper and option parser for the function
`absorb_git_dir_into_superproject`, that takes care of embedding the
submodules git directory into the superprojects git dir. That function
makes use of the more abstract function for this use case
`relocate_gitdir`, which can be used by e.g. the worktree code eventually
to move around a git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 15 ++++++
builtin/submodule--helper.c | 38 ++++++++++++++
dir.c | 12 +++++
dir.h | 3 ++
git-submodule.sh | 7 ++-
submodule.c | 103 +++++++++++++++++++++++++++++++++++++
submodule.h | 4 ++
t/t7412-submodule-absorbgitdirs.sh | 101 ++++++++++++++++++++++++++++++++++++
8 files changed, 282 insertions(+), 1 deletion(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..918bd1d1bd 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] absorbgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,20 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+absorbgitdirs::
+ If a git directory of a submodule is inside the submodule,
+ move the git directory of the submodule into its superprojects
+ `$GIT_DIR/modules` path and then connect the git directory and
+ its working directory by setting the `core.worktree` and adding
+ a .git file pointing to the git directory embedded in the
+ superprojects git directory.
++
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
++
+This command is recursive by default.
+
OPTIONS
-------
-q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 33676a57cf..0108afac93 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,43 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = ABSORB_GITDIR_RECURSE_SUBMODULES;
+
+ struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
+ OPT_BIT(0, "--recursive", &flags, N_("recurse into submodules"),
+ ABSORB_GITDIR_RECURSE_SUBMODULES),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper embed-git-dir [<path>...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, embed_gitdir_options,
+ git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ absorb_git_dir_into_superproject(prefix,
+ list.entries[i]->name, flags);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1094,6 +1131,7 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"init", module_init, 0},
{"remote-branch", resolve_remote_submodule_branch, 0},
+ {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/dir.c b/dir.c
index 8b74997c66..cc5729f733 100644
--- a/dir.c
+++ b/dir.c
@@ -2774,3 +2774,15 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
free(real_work_tree);
free(real_git_dir);
}
+
+/*
+ * Migrate the git directory of the given path from old_git_dir to new_git_dir.
+ */
+void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
+{
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+}
diff --git a/dir.h b/dir.h
index 051674a431..bf23a470af 100644
--- a/dir.h
+++ b/dir.h
@@ -336,4 +336,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
+extern void relocate_gitdir(const char *path,
+ const char *old_git_dir,
+ const char *new_git_dir);
#endif
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..9285b5c43d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_absorbgitdirs()
+{
+ git submodule--helper absorb-git-dirs --prefix "$wt_prefix" "$@"
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 0bb50b4b62..6477746ce4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -14,6 +14,7 @@
#include "blob.h"
#include "thread-utils.h"
#include "quote.h"
+#include "worktree.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
static int parallel_jobs = 1;
@@ -1237,3 +1238,105 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Embeds a single submodules git directory into the superprojects git dir,
+ * non recursively.
+ */
+static void relocate_single_git_dir_into_superproject(const char *prefix,
+ const char *path)
+{
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (submodule_uses_worktrees(path))
+ die(_("relocate_gitdir for submodule '%s' with "
+ "more than one worktree not supported"), path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ return;
+
+ real_old_git_dir = xstrdup(real_path(old_git_dir));
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = xstrdup(real_path(new_git_dir));
+
+ if (!prefix)
+ prefix = get_super_prefix();
+
+ fprintf(stderr, "Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_old_git_dir, real_new_git_dir);
+
+ relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+}
+
+/*
+ * Migrate the git directory of the submodule given by path from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags)
+{
+ const char *sub_git_dir, *v;
+ char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ struct strbuf gitdir = STRBUF_INIT;
+
+ strbuf_addf(&gitdir, "%s/.git", path);
+ sub_git_dir = resolve_gitdir(gitdir.buf);
+
+ /* Not populated? */
+ if (!sub_git_dir)
+ goto out;
+
+ /* Is it already absorbed into the superprojects git dir? */
+ real_sub_git_dir = xstrdup(real_path(sub_git_dir));
+ real_common_git_dir = xstrdup(real_path(get_git_common_dir()));
+ if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~ABSORB_GITDIR_RECURSE_SUBMODULES)
+ die("BUG: we don't know how to pass the flags down?");
+
+ if (get_super_prefix())
+ strbuf_addstr(&sb, get_super_prefix());
+ strbuf_addstr(&sb, path);
+ strbuf_addch(&sb, '/');
+
+ cp.dir = path;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+ "submodule--helper",
+ "absorb-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("could not recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index 4e3bf469b4..6229054b99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,4 +74,8 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
new file mode 100755
index 0000000000..7c4e8b8d79
--- /dev/null
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='Test submodule absorbgitdirs
+
+This test verifies that `git submodue absorbgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+ git init sub1 &&
+ test_commit -C sub1 first &&
+ git submodule add ./sub1 &&
+ test_tick &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorb the git dir' '
+ >expect.1 &&
+ >expect.2 &&
+ >actual.1 &&
+ >actual.2 &&
+ git status >expect.1 &&
+ git -C sub1 rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ git fsck &&
+ test -f sub1/.git &&
+ test -d .git/modules/sub1 &&
+ git status >actual.1 &&
+ git -C sub1 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'absorbing does not fail for deinitalized submodules' '
+ test_when_finished "git submodule update --init" &&
+ git submodule deinit --all &&
+ git submodule absorbgitdirs &&
+ test -d .git/modules/sub1 &&
+ test -d sub1 &&
+ ! test -e sub1/.git
+'
+
+test_expect_success 'setup nested submodule' '
+ git init sub1/nested &&
+ test_commit -C sub1/nested first_nested &&
+ git -C sub1 submodule add ./nested &&
+ test_tick &&
+ git -C sub1 commit -m "add nested" &&
+ git add sub1 &&
+ git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+ git init sub2 &&
+ test_commit -C sub2 first &&
+ git add sub2 &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorbing the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule absorbgitdirs &&
+ git -C sub2 fsck &&
+ test -d sub2/.git &&
+ git status >actual &&
+ git -C sub2 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a submodule with multiple worktrees' '
+ # first create another unembedded git dir in a new submodule
+ git init sub3 &&
+ test_commit -C sub3 first &&
+ git submodule add ./sub3 &&
+ test_tick &&
+ git commit -m "add another submodule" &&
+ git -C sub3 worktree add ../sub3_second_work_tree
+'
+
+test_expect_success 'absorb a submodule with multiple worktrees' '
+ test_must_fail git submodule absorbgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
+test_done
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 5/7] worktree: add function to check if worktrees are in use
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 24 ++++++++++++++++++++++++
worktree.h | 7 +++++++
2 files changed, 31 insertions(+)
diff --git a/worktree.c b/worktree.c
index 75db689672..2559f33846 100644
--- a/worktree.c
+++ b/worktree.c
@@ -406,3 +406,27 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
+
+static int uses_worktree_internal(struct worktree **worktrees)
+{
+ int i;
+ for (i = 0; worktrees[i]; i++)
+ ; /* nothing */
+
+ free_worktrees(worktrees);
+ return i > 1;
+}
+
+int uses_worktrees(void)
+{
+ return uses_worktree_internal(get_worktrees(0));
+}
+
+int submodule_uses_worktrees(const char *path)
+{
+ struct worktree **worktrees = get_submodule_worktrees(path, 0);
+ if (!worktrees)
+ return 0;
+
+ return uses_worktree_internal(worktrees);
+}
diff --git a/worktree.h b/worktree.h
index 157fbc4a66..76027b1fd2 100644
--- a/worktree.h
+++ b/worktree.h
@@ -33,6 +33,13 @@ extern struct worktree **get_worktrees(unsigned flags);
extern struct worktree **get_submodule_worktrees(const char *path,
unsigned flags);
+/*
+ * Returns 1 if more than one worktree exists.
+ * Returns 0 if only the main worktree exists.
+ */
+extern int uses_worktrees(void);
+extern int submodule_uses_worktrees(const char *path);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 3/7] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib-functions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
GIT_TEST_GDB=1 "$@"
}
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
# message, and tag the resulting commit with the given tag name.
#
# <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
test_commit () {
notick= &&
signoff= &&
+ indir= &&
while test $# != 0
do
case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
--signoff)
signoff="$1"
;;
+ -C)
+ indir="$2"
+ shift
+ ;;
*)
break
;;
esac
shift
done &&
+ indir=${indir:+"$indir"/} &&
file=${2:-"$1.t"} &&
- echo "${3-$1}" > "$file" &&
- git add "$file" &&
+ echo "${3-$1}" > "$indir$file" &&
+ git ${indir:+ -C "$indir"} add "$file" &&
if test -z "$notick"
then
test_tick
fi &&
- git commit $signoff -m "$1" &&
- git tag "${4:-$1}"
+ git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+ git ${indir:+ -C "$indir"} tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 6/7] move connect_work_tree_and_git_dir to dir.h
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
That function was primarily used by submodule code, but the function
itself is not inherently about submodules. In the next patch we'll
introduce relocate_git_dir, which can be used by worktrees as well,
so find a neutral middle ground in dir.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 26 ++++++++++++++++++++++++++
dir.h | 1 +
submodule.c | 26 --------------------------
submodule.h | 1 -
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..8b74997c66 100644
--- a/dir.c
+++ b/dir.c
@@ -2748,3 +2748,29 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/* Update gitfile and core.worktree setting to connect work tree and git dir */
+void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
+{
+ struct strbuf file_name = STRBUF_INIT;
+ struct strbuf rel_path = STRBUF_INIT;
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
+
+ /* Update gitfile */
+ strbuf_addf(&file_name, "%s/.git", work_tree);
+ write_file(file_name.buf, "gitdir: %s",
+ relative_path(real_git_dir, real_work_tree, &rel_path));
+
+ /* Update core.worktree setting */
+ strbuf_reset(&file_name);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
+ git_config_set_in_file(file_name.buf, "core.worktree",
+ relative_path(real_work_tree, real_git_dir,
+ &rel_path));
+
+ strbuf_release(&file_name);
+ strbuf_release(&rel_path);
+ free(real_work_tree);
+ free(real_git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..051674a431 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
#endif
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..0bb50b4b62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1222,32 +1222,6 @@ int merge_submodule(unsigned char result[20], const char *path,
return 0;
}
-/* Update gitfile and core.worktree setting to connect work tree and git dir */
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
-{
- struct strbuf file_name = STRBUF_INIT;
- struct strbuf rel_path = STRBUF_INIT;
- char *real_git_dir = xstrdup(real_path(git_dir));
- char *real_work_tree = xstrdup(real_path(work_tree));
-
- /* Update gitfile */
- strbuf_addf(&file_name, "%s/.git", work_tree);
- write_file(file_name.buf, "gitdir: %s",
- relative_path(real_git_dir, real_work_tree, &rel_path));
-
- /* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", real_git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, real_git_dir,
- &rel_path));
-
- strbuf_release(&file_name);
- strbuf_release(&rel_path);
- free(real_work_tree);
- free(real_git_dir);
-}
-
int parallel_submodules(void)
{
return parallel_jobs;
diff --git a/submodule.h b/submodule.h
index d9e197a948..4e3bf469b4 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,7 +65,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
struct string_list *needs_pushing);
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
/*
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* Re: [PATCH v15 19/27] bisect--helper: `bisect_state` & `bisect_head` shell function in C
From: Pranit Bauva @ 2016-12-08 6:43 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Git List
In-Reply-To: <14b9ba3a-4568-2e4b-0b1a-f0ee75a7c677@gmx.net>
Hey Stephan,
On Wed, Dec 7, 2016 at 5:24 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi Pranit,
>
> On 12/06/2016 11:40 PM, Pranit Bauva wrote:
>> On Tue, Nov 22, 2016 at 5:42 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
>>> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>>>> +static int bisect_state(struct bisect_terms *terms, const char **argv,
>>>> + int argc)
>>>> +{
>>>> + const char *state = argv[0];
>>>> +
>>>> + get_terms(terms);
>>>> + if (check_and_set_terms(terms, state))
>>>> + return -1;
>>>> +
>>>> + if (!argc)
>>>> + die(_("Please call `--bisect-state` with at least one argument"));
>>>
>>> I think this check should move to cmd_bisect__helper. There are also the
>>> other argument number checks.
>>
>> Not really. After the whole conversion, the cmdmode will cease to
>> exists while bisect_state will be called directly, thus it is
>> important to check it here.
>
> Okay, that's a point.
> In that case, you should probably use "return error()" again and the
> message (mentioning "--bisect-state") does not make sense when
> --bisect-state ceases to exist.
True. Will change accordingly.
>>>> +
>>>> + if (argc == 1 && one_of(state, terms->term_good,
>>>> + terms->term_bad, "skip", NULL)) {
>>>> + const char *bisected_head = xstrdup(bisect_head());
>>>> + const char *hex[1];
>>>
>>> Maybe:
>>> const char *hex;
>>>
>>>> + unsigned char sha1[20];
>>>> +
>>>> + if (get_sha1(bisected_head, sha1))
>>>> + die(_("Bad rev input: %s"), bisected_head);
>>>
>>> And instead of...
>>>
>>>> + if (bisect_write(state, sha1_to_hex(sha1), terms, 0))
>>>> + return -1;
>>>> +
>>>> + *hex = xstrdup(sha1_to_hex(sha1));
>>>> + if (check_expected_revs(hex, 1))
>>>> + return -1;
>>>
>>> ... simply:
>>>
>>> hex = xstrdup(sha1_to_hex(sha1));
>>> if (set_state(terms, state, hex)) {
>>> free(hex);
>>> return -1;
>>> }
>>> free(hex);
>>>
>>> where:
>>
>> Yes I am planning to convert all places with hex rather than the sha1
>> but not yet, maybe in an another patch series because currently a lot
>> of things revolve around sha1 and changing its behaviour wouldn't
>> really be a part of a porting patch series.
>
> I was not suggesting a change of behavior, I was suggesting a simple
> helper function set_state() to get rid of code duplication above and
> some lines below:
>
>>> ... And replace this:
>>>
>>>> + const char **hex_string = (const char **) &hex.items[i].string;
>>>> + if(bisect_write(state, *hex_string, terms, 0)) {
>>>> + string_list_clear(&hex, 0);
>>>> + return -1;
>>>> + }
>>>> + if (check_expected_revs(hex_string, 1)) {
>>>> + string_list_clear(&hex, 0);
>>>> + return -1;
>>>> + }
>>>
>>> by:
>>>
>>> const char *hex_str = hex.items[i].string;
>>> if (set_state(terms, state, hex_string)) {
>>> string_list_clear(&hex, 0);
>>> return -1;
>>> }
>
> See?
I can do this change of using set_state() keeping aside the sha1 to
hex and such change.
>>>> @@ -184,8 +137,8 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
>>>> state="$TERM_GOOD"
>>>> fi
>>>>
>>>> - # We have to use a subshell because "bisect_state" can exit.
>>>> - ( bisect_state $state >"$GIT_DIR/BISECT_RUN" )
>>>> + # We have to use a subshell because "--bisect-state" can exit.
>>>> + ( git bisect--helper --bisect-state $state >"$GIT_DIR/BISECT_RUN" )
>>>
>>> The new comment is funny, but you don't need a subshell here any
>>> longer.
>>
>> True, but right now I didn't want to modify that part of the source
>> code to remove the comment. I will remove the comment all together
>> when I port bisect_run()
> For most of the patches, I was commenting on the current state, not on
> the big picture.
>
> Still I think that it is better to remove the comment and the subshell
> instead of making the comment weird ("yes the builtin can exit, but why
> do we need a subshell?" vs "yes the shell function calls exit, so we
> need a subshell because we do not want to exit this shell script")
Sure I will remove the comment.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Christian Couder @ 2016-12-08 7:50 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, SZEDER Gábor
In-Reply-To: <6facca6e-622a-ea8f-89d8-a18b7faee3cc@gmx.net>
Hi,
On Wed, Dec 7, 2016 at 7:36 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi,
>
> On 12/06/2016 07:58 PM, Junio C Hamano wrote:
>
>> (1) The third invocation of "cherry-pick" with "--abort" to get rid
>> of the state from the unfinished cherry-pick we did previously
>> is necessary, but the command does not notice that we resetted
>> to a new branch AND we even did some other work there. This
>> loses end-user's work.
>>
>> "git cherry-pick --abort" should learn from "git am --abort"
>> that has an extra safety to deal with the above workflow. The
>> state from the unfinished "am" is removed, but the head is not
>> rewound to avoid losing end-user's work.
>>
>> You can try by replacing two instances of
>>
>> $ git cherry-pick 0582a34f52..a94bb68397
>>
>> with
>>
>> $ git format-patch --stdout 0582a34f52..a94bb68397 | git am
>>
>> in the above sequence, and conclude with "git am--abort" to see
>> how much more pleasant and safe "git am --abort" is.
> Definitely. I'd volunteer to add that safety guard. (But (2) remains.)
That would be great if you could take care of that!
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Torsten Bögershausen @ 2016-12-08 7:55 UTC (permalink / raw)
To: Brandon Williams
Cc: Junio C Hamano, Ramsay Jones, git, sbeller, peff, jacob.keller
In-Reply-To: <20161207221335.GA116201@google.com>
On Wed, Dec 07, 2016 at 02:13:35PM -0800, Brandon Williams wrote:
> On 12/07, Junio C Hamano wrote:
> > Torsten Bögershausen <tboegi@web.de> writes:
> >
> > > But in any case it seems that e.g.
> > > //SEFVER/SHARE/DIR1/DIR2/..
> > > must be converted into
> > > //SEFVER/SHARE/DIR1
> > >
> > > and
> > > \\SEFVER\SHARE\DIR1\DIR2\..
> > > must be converted into
> > > \\SEFVER\SHARE\DIR1
> >
> > Additional questions that may be interesting are:
> >
> > //A/B/../C is it //A/C? is it an error?
Yes, at least under Windows.
If I have e.g. a Raspi with SAMBA, I can put a git Repository here:
//raspi/torsten/projects/git
If I use
git push //raspi/torsten/../junio/projects/git
that should be an error.
> > //A/B/../../C/D is it //C/D? is it an error?
> >
Same for
git push /raspi/../raspi2/torsten//projects/git
>
>
> Also is //.. the same as //? I would assume so since /.. is /
>
Under Windows //.. is simply illegal, I would say.
The documentation here
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
Mentions these 2 examples, what to feed into the WIN32 file API:
a)
\\?\D:\very-long-path
b)
\\server\share\path\file"
c)
"\\?\UNC\server\share\path"
So whatever we do, the ".." resoltion is only allowed to look at
"very-long-path" or "path".
Some conversion may be done in mingw.c:
https://github.com/github/git-msysgit/blob/master/compat/mingw.c
So what I understand, '/' in Git are already converted into '\' if needed ?
It seams that we may wnat a function get_start_of_path(uncpath),
which returns:
get_start_of_path_win("//?/D:/very-long-path") "/very-long-path"
get_start_of_path_win("//server/share/path/file") "/path/file"
get_start_of_path_win("//?/UNC/server/share/path") "/path"
(I don't know if we need the variant with '\', but is would'n hurt):
get_start_of_path_win("\\\\?\\D:\\very-long-path") "\\very-long-path"
get_start_of_path_win("\\\\server\\share\\path\\file") "\\path\\file"
get_start_of_path_win("\\\\?\\UNC\\server\\share\\path") "\\path"
Then the non-windows version could simply return
get_start_of_path_non_win(something) something
Does this make sense ?
^ permalink raw reply
* Re: [PATCH 11/17] pathspec: factor global magic into its own function
From: Duy Nguyen @ 2016-12-08 9:20 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161207223936.GD116201@google.com>
On Thu, Dec 8, 2016 at 5:39 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/07, Duy Nguyen wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> > Create helper functions to read the global magic environment variables
>> > in additon to factoring out the global magic gathering logic into its
>> > own function.
>> >
>> > Signed-off-by: Brandon Williams <bmwill@google.com>
>> > ---
>> > pathspec.c | 120 +++++++++++++++++++++++++++++++++++++------------------------
>> > 1 file changed, 74 insertions(+), 46 deletions(-)
>> >
>> > diff --git a/pathspec.c b/pathspec.c
>> > index 5afebd3..08e76f6 100644
>> > --- a/pathspec.c
>> > +++ b/pathspec.c
>> > @@ -87,6 +87,74 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
>> > strbuf_addf(sb, ",prefix:%d)", prefixlen);
>> > }
>> >
>> > +static inline int get_literal_global(void)
>> > +{
>> > + static int literal_global = -1;
>> > +
>> > + if (literal_global < 0)
>> > + literal_global = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT,
>> > + 0);
>>
>> These zeros look so lonely. I know it would exceed 80 columns if we
>> put it on the previous line. But I think it's ok for occasional
>> exceptions. Or you could rename noglob_global to noglob.
>
> I was thinking the same thing but was so torn between the char limit. I
> think it's probably ok to rename these vars by drooping the global since
> the function name themselves indicate they are global.
Exactly. I almost suggested just "ret" for that reason, but it was a
bit on the extreme side, relying entirely on the function's name for
context.
--
Duy
^ permalink raw reply
* Re: [PATCH 16/17] pathspec: small readability changes
From: Duy Nguyen @ 2016-12-08 9:23 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161207232724.GI116201@google.com>
On Thu, Dec 8, 2016 at 6:27 AM, Brandon Williams <bmwill@google.com> wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> > @@ -362,8 +368,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> > } else {
>> > item->original = xstrdup(elt);
>> > }
>> > - item->len = strlen(item->match);
>> > - item->prefix = prefixlen;
>> >
>> > if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
>> > strip_submodule_slash_cheap(item);
>> > @@ -371,13 +375,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> > if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
>> > strip_submodule_slash_expensive(item);
>> >
>> > - if (magic & PATHSPEC_LITERAL)
>> > + if (magic & PATHSPEC_LITERAL) {
>> > item->nowildcard_len = item->len;
>> > - else {
>> > + } else {
>> > item->nowildcard_len = simple_length(item->match);
>> > if (item->nowildcard_len < prefixlen)
>> > item->nowildcard_len = prefixlen;
>> > }
>> > +
>> > item->flags = 0;
>>
>> You probably can move this line up with the others too.
>
> I didn't move the item->flags assignment up since the code immediately
> following this assignment deal with setting item->flags. I made more
> sense to keep them grouped.
It's probably why I put it there in the beginning :) Yes let's leave
it where it is then.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox