On Tue, Sep 17, 2019 at 6:56 PM Matheus Tavares
wrote:
>
> When grepping with --recurse-submodules, the object directory of the
> submodule is added to the in-memory alternates list. This makes git need
> to watch out for more packfiles which might bring bad consequences for
> memory and performan
On Mon, Jul 29, 2019 at 1:27 PM Matheus Tavares Bernardino
wrote:
>
> On Tue, Jul 16, 2019 at 3:09 PM Daniel Zaoui wrote:
> >
> > Hi Matheus,
>
> Hi, Daniel
>
> I'm sorry, your last message went to my spam folder for some reason :(
>
> > Thank you for your response.
> >
> > I really hope the chan
On Fri, Dec 7, 2018 at 10:08 PM Junio C Hamano wrote:
>
> Stefan Beller writes:
>
> > On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder wrote:
> >>
> >> Brandon Williams wrote:
> >>
> >> > Signed-off-by: Brandon Williams
> >> &
Signed-off-by: Brandon Williams
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index eb7b5fc7b..247a3deb7 100644
--- a/.mailmap
+++ b/.mailmap
@@ -27,6 +27,7 @@ Ben Walton
Benoit Sigoure
Bernt Hansen
Brandon Casey
+Brandon Williams
brian m
t; > time is probably now.
> >
> > Have we rejected the config approach?
>
> I did not reject that approach, but am rather waiting for patches. ;-)
Note I did send out a patch using this approach, so no need to wait any
longer! :D
https://public-inbox.org/git/20180816181940.46114-1-bmw...@google.com/
--
Brandon Williams
Introduce the config "submodule..gitdirpath" which is used to
indicate where a submodule's gitdir is located inside of a repository's
"modules" directory.
Signed-off-by: Brandon Williams
---
Maybe something like this on top? Do you think we should disallow "
> >>
> >> Sane?
Seems like a sensible thing to do. Let me work up some patches to
implement this using config primarily and these other schemes as
fallbacks.
> >
> > I'll keep thinking about it.
>
> Thanks.
>
> > FYI: the reduction in configuration was just sent out.
>
> https://public-inbox.org/git/20180816023100.161626-1-sbel...@google.com/
> for those following along.
>
> Ciao,
> Jonathan
--
Brandon Williams
-submodules file://...
>
> might clone the submodules that are in detached HEAD, which is totally
> not a long term viable good HEAD, so a subsequent fetch might want
> to change the detached HEAD in submodules or re-affix it to branches.
>
> Unrelated/extended: I think it would be cool to mirror a repository even
> more, e.g. it would be cool to be able to fetch (if configured as allowed)
> the remote reflog, (not to be confused with you local reflog of the remote).
> I think that work would be enabled once reftables are available, which you
> have an eye on?
--
Brandon Williams
On 08/09, Jeff King wrote:
> On Wed, Aug 08, 2018 at 03:33:23PM -0700, Brandon Williams wrote:
>
> > Commit 0383bbb901 (submodule-config: verify submodule names as paths,
> > 2018-04-30) introduced some checks to ensure that submodule names don't
> > include direct
rl newurl
> + )
> +'
> +
> +test_expect_success 'initialising submodule when the gitmodules config is
> not checked out' '
> + (cd super &&
> + git submodule init
> + )
> +'
> +
> +test_expect_success 'showing submodule summary when the gitmodules config is
> not checked out' '
> + (cd super &&
> + git submodule summary
> + )
> +'
> +
> +test_expect_success 'updating submodule when the gitmodules config is not
> checked out' '
> + (cd submodule &&
> + echo file2 >file2 &&
> + git add file2 &&
> + git commit -m "add file2 to submodule"
> + ) &&
> + (cd super &&
> + git submodule update
> + )
> +'
> +
> +test_expect_success 'not adding submodules when the gitmodules config is not
> checked out' '
> + (cd super &&
> + test_must_fail git submodule add ../new_submodule
> + )
> +'
> +
> +# This test checks that the previous "git submodule add" did not leave the
> +# repository in a spurious state when it failed.
> +test_expect_success 'init submodule still works even after the previous add
> failed' '
> + (cd super &&
> + git submodule init
> + )
> +'
> +
> +test_done
> --
> 2.18.0
>
--
Brandon Williams
--helper config submodule."$sm_name".path "$sm_path" &&
> + git submodule--helper config submodule."$sm_name".url "$repo" &&
> if test -n "$branch"
> then
> - git config -f .gitmodules submodule."$sm_name".branch "$branch"
> + git submodule--helper config submodule."$sm_name".branch
> "$branch"
> fi &&
> git add --force .gitmodules ||
> die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
> --
> 2.18.0
>
--
Brandon Williams
quot; >expected &&
> + git submodule--helper config submodule.submodule.url "new_url"
> &&
> + git submodule--helper config submodule.submodule.url >actual &&
> + test_cmp expected actual
> + )
> +'
> +
> +test_expect_success 'overwriting unstaged submodules config with
> "submodule--helper config"' '
> + (cd super &&
> + echo "newer_url" >expected &&
> + git submodule--helper config submodule.submodule.url
> "newer_url" &&
> + git submodule--helper config submodule.submodule.url >actual &&
> + test_cmp expected actual
> + )
> +'
> +
> test_done
> --
> 2.18.0
>
--
Brandon Williams
bmodule lets continue' '
> HEAD b \
> HEAD submodule \
> >actual &&
> - test_cmp expect_error actual &&
> - git reset --hard HEAD^
> + test_cmp expect_error actual
> )
> '
>
> --
> 2.18.0
>
--
Brandon Williams
the approach this series took vs what your part 1 did so that
we can get this change in.
--
Brandon Williams
rection(GIT_ATTR_CHECKOUT);
>
> if (should_update_submodules() && o->update && !o->dry_run)
> load_gitmodules_file(index, NULL);
> @@ -413,7 +413,7 @@ static int check_updates(struct unpack_trees_options *o)
> stop_progress(&progress);
> errs |= finish_delayed_checkout(&state);
> if (o->update)
> - git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
> + git_attr_set_direction(GIT_ATTR_CHECKIN);
> return errs != 0;
> }
>
> --
> 2.18.0.1004.g6639190530
>
--
Brandon Williams
e, namelen, item))
> return 0;
Yuck, all of this just because I added the ability to match against
attrs with pathspecs. Part of me wonders if it would be better to put a
pointer to the needed istate in the pathspec struct...but then I can
think of a ton of reasons why that wouldn't be good either.
So yes I think this is probably the right approach, I'm just sorry I
made it this messy :/
--
Brandon Williams
ue) {
> marker_size = atoi(check->items[1].value);
> @@ -398,7 +398,7 @@ int ll_merge_marker_size(const char *path)
>
> if (!check)
> check = attr_check_initl("conflict-marker-size", NULL);
> - if (!git_check_attr(path, check) && check->items[0].value) {
> + if (!git_check_attr(&the_index, path, check) && check->items[0].value) {
> marker_size = atoi(check->items[0].value);
> if (marker_size <= 0)
> marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
> diff --git a/userdiff.c b/userdiff.c
> index 36af25e7f9..f3f4be579c 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -278,7 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char
> *path)
> check = attr_check_initl("diff", NULL);
> if (!path)
> return NULL;
> - if (git_check_attr(path, check))
> + if (git_check_attr(&the_index, path, check))
> return NULL;
>
> if (ATTR_TRUE(check->items[0].value))
> diff --git a/ws.c b/ws.c
> index a07caedd5a..5b67b426e7 100644
> --- a/ws.c
> +++ b/ws.c
> @@ -78,7 +78,7 @@ unsigned whitespace_rule(const char *pathname)
> if (!attr_whitespace_rule)
> attr_whitespace_rule = attr_check_initl("whitespace", NULL);
>
> - if (!git_check_attr(pathname, attr_whitespace_rule)) {
> + if (!git_check_attr(&the_index, pathname, attr_whitespace_rule)) {
> const char *value;
>
> value = attr_whitespace_rule->items[0].value;
> --
> 2.18.0.1004.g6639190530
>
--
Brandon Williams
ttwen
commondir and gitdir for worktrees. Using the "repo_git_path" function
handles path rewritting when using worktrees. Here (when working with
worktrees) "subrepo.gitdir" refers to the worktree specific gitdir while
"subrepo.commondir" refers to the global common gitdir where the
repository config actually lives.
--
Brandon Williams
me=$(git submodule--helper name "$sm_path") || exit
> if ! test -z "$update"
> then
> @@ -577,11 +579,6 @@ cmd_update()
> die "$(eval_gettext "Unable to find current
> \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
> fi
>
> - if ! $(git config -f "$(git rev-parse
> --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
> - then
> - git submodule--helper connect-gitdir-workingtree
> "$name" "$sm_path"
> - fi
> -
> if test "$subsha1" != "$sha1" || test -n "$force"
> then
> subforce=$force
> --
> 2.18.0.132.g195c49a2227
>
--
Brandon Williams
On 08/10, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > Introduce a helper function "submodule_name_to_gitdir()" (and the
> > submodule--helper subcommand "gitdir") which constructs a path to a
> > submodule's gitdir, located in
On 08/08, Stefan Beller wrote:
> On Wed, Aug 8, 2018 at 3:33 PM Brandon Williams wrote:
> >
> > Introduce a helper function "submodule_name_to_gitdir()" (and the
> > submodule--helper subcommand "gitdir") which constructs a path to a
> > submodule
ld a path to the submodule's gitdir.
Signed-off-by: Brandon Williams
---
submodule.c | 14 ++
t/t7400-submodule-basic.sh | 32 +++-
t/t7406-submodule-update.sh | 21 ++---
3 files changed, 51 insertions(+), 16 deletions(-)
d up a path into a
repository's "modules" directory, abstracting away the fact that
submodule git directories are stored in a repository's common gitdir.
This makes it easier to adjust how submodules gitdir are stored in the
"modules" directory in a future patch.
Signe
Here's a more polished series taking into account some of the feedback
on the RFC. As Junio pointed out URL encoding makes the directories
much more human readable, but I'm open to other ideas if we don't think
URL encoding is the right thing to do.
Brandon Williams (2):
su
This allows for a single
location where we can munge the submodule name (by url encoding it)
before using it as part of a path.
Signed-off-by: Brandon Williams
---
Using submodule names as is continues to be not such a good idea. Maybe
we could apply something like this to stop using them
;t
interested in taking, shouldn't they at least get informed of that?
Anyway those are just some of my thoughts. If my thinking is mistaken or I'm
missing something please point it out to me.
[1]
https://public-inbox.org/git/cagz79kygs4dvoetygx01cinrxxlcqgxovsplhmgyz8b51lz...@mail.gmail.com/
This mail seems to counter that indicating that the "What's Cooking"
emails should not be used as a status update.
--
Brandon Williams
s to the
> tail of the input list. Upon hindsight, maybe this should have been the
> original API change instead of the "fetched_refs" mechanism.
Thanks for getting this out, it looks good to me. If we end up adding
patterns to ref-in-want then we can explore what changes would need to
be made then, I expect we may need to do a bit more work on the whole
fetching stack to get what we'd want in that case (because we would want
to avoid this issue again).
--
Brandon Williams
e the
input list again in this case and just simply add the resulting refs to
the input list?
>
> [1]
> https://public-inbox.org/git/86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathanta...@google.com/
> [2]
> https://public-inbox.org/git/eef2b77d88df0db08e4a1505b06e0af2d40143d5.1485381677.git.jonathanta...@google.com/
> [3] https://public-inbox.org/git/20180620213235.10952-1-bmw...@google.com/
--
Brandon Williams
On 07/30, brian m. carlson wrote:
> On Mon, Jul 30, 2018 at 10:50:51AM -0700, Brandon Williams wrote:
> > On 07/29, brian m. carlson wrote:
> > > The object ID parsing machinery is aware of "@" as a synonym for "HEAD"
> > > and this is docum
his will get the objects but fail to update refs
> git fetch
>
> # and fetching again will actually update the refs
> git fetch
> -- 8< --
>
> -Peff
--
Brandon Williams
010dce0c..57c2f65104 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -62,8 +62,12 @@ static int parse_refspec(struct refspec_item *item, const
> char *refspec, int fet
> return 0;
> }
>
> + if (llen == 1 && lhs[0] == '@')
> + item->src = xstrdup("HEAD");
> + else
> + item->src = xstrndup(lhs, llen);
> +
This is probably the easiest place to put the aliasing logic so I don't
have any issue with including it here.
--
Brandon Williams
sensitive.
> + git config -f caseSens v."A".r VAL &&
> + git config -f caseSens v."a".r val &&
> +
> + echo VAL >caseSens_expect &&
> + git config -f caseSens v."A".r >caseSens_actual &&
> + test_cmp caseSens_expect caseSens_actual &&
> +
> + echo val >caseSens_expect &&
> + git config -f caseSens v."a".r >caseSens_actual &&
> + test_cmp caseSens_expect caseSens_actual
> +'
> +
> for VAR in a .a a. a.0b a."b c". a."b c".0d
> do
> test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
> --
> 2.18.0.345.g5c9ce644c3-goog
>
--
Brandon Williams
use a ref-store (and instead
just calls the underlying impl with get_main_ref_store()). Am I missing
something here?
--
Brandon Williams
On 07/25, Duy Nguyen wrote:
> On Tue, Jul 24, 2018 at 9:29 PM Brandon Williams wrote:
> >
> > On 07/17, Brandon Williams wrote:
> > > Signed-off-by: Brandon Williams
> > > ---
> > >
> > > Since introducing protocol v2 and enabling fetch I&
On 07/24, Junio C Hamano wrote:
> Brandon Williams writes:
>
> >> Not about this patch, but I wonder if an organization along the
> >> following lines would make sense?
> >>
> >> 1. Rename pack-protocol.txt to protocol-v1.txt. Rename
>
), and I think the correct resolution is to move your
> removal of "&& !rs->nr" to do_fetch() function where that commit
> moved to.
>
> Thanks.
>
> [2] https://public-inbox.org/git/xmqqd0vwcfkr....@gitster-ct.c.googlers.com/
Yeah I still don't like it from a performance perspective, but given
people rely on this functionality I've been convinced its necessary for
correctness until we make other changes.
--
Brandon Williams
On 07/17, Brandon Williams wrote:
> Signed-off-by: Brandon Williams
> ---
>
> Since introducing protocol v2 and enabling fetch I've been thinking
> about what its inverse 'push' would look like. After talking with a
> number of people I have a longish lis
On 07/20, Jeff Hostetler wrote:
>
>
> On 7/18/2018 1:15 PM, Brandon Williams wrote:
> > On 07/18, Stefan Beller wrote:
> > > On Wed, Jul 18, 2018 at 6:31 AM Derrick Stolee wrote:
> > > >
> > > > On 7/17/2018 7:25 PM, Stefan Beller wrote:
&
On 07/23, Jonathan Nieder wrote:
> Hi,
>
> Brandon Williams wrote:
>
> > --- a/Documentation/technical/pack-protocol.txt
> > +++ b/Documentation/technical/pack-protocol.txt
> > @@ -50,7 +50,8 @@ Each Extra Parameter takes the form of `=` or
> > ``.
> >
On 07/23, Junio C Hamano wrote:
> Junio C Hamano writes:
>
> > Brandon Williams writes:
> >
> >> Signed-off-by: Brandon Williams
> >> ---
> >> fetch-pack.c | 16
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> &g
Signed-off-by: Brandon Williams
---
fetch-pack.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 0b4a9f288f..51abee6181 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1245,13 +1245,13 @@ static int process_section_header
On 07/22, Duy Nguyen wrote:
> On Thu, Jun 28, 2018 at 12:33 AM Brandon Williams wrote:
> > +static void receive_wanted_refs(struct packet_reader *reader, struct ref
> > *refs)
> > +{
> > + process_section_header(reader, "wanted-refs", 0);
> &g
Signed-off-by: Brandon Williams
---
Documentation/technical/pack-protocol.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index 7fee6b780a..25acd9edb1 100644
--- a/Documentation
Teach clone to send a list of ref-prefixes, when using protocol v2, to
allow the server to filter out irrelevant references from the
ref-advertisement. This reduces wasted time and bandwidth when cloning
repositories with a larger number of references.
Signed-off-by: Brandon Williams
Signed-off-by: Brandon Williams
---
Noticed we miss out on server side filtering of refs when cloning using
protocol v2, this will enable that.
builtin/clone.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index
> > SP new_oid
> > > > +update-error = PKT-LINE(txn_id SP "error" SP error-msg LF)
> > >
> > > Can we unify "ERR" and "error" ?
> >
> > No, these are very different. You could have one ref update succeed
> > while another doesn't for some reason, unless you want everything to be
> > atomic.
>
> I did not mean to unify them on the semantic level, but on the
> representation level, i.e. have both of them spelled the same,
> as they can still be differentiated by the leading txn id?
Oh I misunderstood again :) yeas we could standardize on ERR.
>
>
> Thanks,
> Stefan
>
> P.S.: another feature that just came to mind is localisation of error
> messages.
> But that is also easy to do with capabilities (the client sends a capability
> such as "preferred-i18n=DE" and the server may translate all its errors
> if it can.
>
> That brings me to another point: do we assume all errors to be read
> by humans? or do we want some markup things in there, too, similar to
> EAGAIN?
This sort of thing could be added as a protocol-level capability where
the client sends LANG= so that those sorts of msgs could
be translated server side before sending them.
--
Brandon Williams
On 07/18, Duy Nguyen wrote:
> On Wed, Jul 18, 2018 at 7:13 PM Brandon Williams wrote:
> > > > > What I've got now is a rough design for a more flexible push, more
> > > > > flexible because it allows for the server to do what it wants with the
> > >
On 07/18, Stefan Beller wrote:
> On Wed, Jul 18, 2018 at 6:31 AM Derrick Stolee wrote:
> >
> > On 7/17/2018 7:25 PM, Stefan Beller wrote:
> > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams
> > > wrote:
> > >> Signed-off-by: Brandon William
On 07/18, Derrick Stolee wrote:
> On 7/17/2018 7:25 PM, Stefan Beller wrote:
> > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote:
> > > Signed-off-by: Brandon Williams
> > > ---
> > >
> > > Since introducing protocol v2 and enabling fetch I&
On 07/17, Stefan Beller wrote:
> On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote:
> >
> > Signed-off-by: Brandon Williams
> > ---
> >
> > Since introducing protocol v2 and enabling fetch I've been thinking
> > about what its inverse 'push
Signed-off-by: Brandon Williams
---
Since introducing protocol v2 and enabling fetch I've been thinking
about what its inverse 'push' would look like. After talking with a
number of people I have a longish list of things that could be done to
improve push and I think I've
On 07/09, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > I agree with this observation, though I'm a bit sad about it. I think
> > that having tag auto-following the default is a little confusing (and
> > hurts perf[1] when using proto v2) but since
ut for now we've
had enough people comment on this lacking functionality that we should
fix it.
[1] Thankfully it doesn't completely undo what protocol v2 did, as we
still are able to eliminate refs/changes or refs/pull or other various
refs which significantly add to the number of refs advertised during
fetch.
--
Brandon Williams
On 07/09, Junio C Hamano wrote:
> Brandon Williams writes:
>
> >> > > diff --git a/refs/files-backend.c b/refs/files-backend.c
> >> > > index a9a066dcfb..252f835bae 100644
> >> > > --- a/refs/files-backend.c
> >> > > ++
re we do not write bogus reflog
> entries., 2007-01-26)
> and it appears to me that 2*40 + 5 ought to be sufficient, but no
> comments or commit
> messages are found as to why we rather choose 100.
Whats the reason for not using a strbuf here so that we don't have to
play with magic numbers?
--
Brandon Williams
is is used to fill
a ws_delta struct by calculating the whitespace delta between two lines.
If that is the case then why doesn't this function verify that the first
'd' characters in the longer line are indeed whitespace? Also, maybe
this is just because I'm not as familiar with the move detection code,
but how would the whitespace detection handle a line being moved from
being indented with tabs to spaces or vice versa? Is this something
already handled and not an issue?
--
Brandon Williams
ommit -m "add text" &&
> + q_to_tab <<-\EOF >text.txt &&
> + Qa long line to exceed per-line minimum
> + Qanother long line to exceed per-line minimum
> + new file
> + EOF
> +
> + # Make sure we get a different diff using -w
> + git diff --color --color-moved -w |
> + grep -v "index" |
> + test_decode_color >actual &&
> + q_to_tab <<-\EOF >expected &&
> + diff --git a/text.txt b/text.txt
> + --- a/text.txt
> + +++ b/text.txt
> + @@ -1,3 +1,3 @@
> + Qa long line to exceed per-line minimum
> + Qanother long line to exceed per-line minimum
> + -original file
> + +new file
> + EOF
> + test_cmp expected actual &&
> +
> + # And now ignoring white space only in the move detection
> + git diff --color --color-moved \
> +
> --color-moved-ws=ignore-all-space,ignore-space-change,ignore-space-at-eol |
> + grep -v "index" |
> + test_decode_color >actual &&
> + q_to_tab <<-\EOF >expected &&
> + diff --git a/text.txt b/text.txt
> + --- a/text.txt
> + +++ b/text.txt
> + @@ -1,3 +1,3 @@
> + -a long line to exceed per-line minimum
> + -another long line to exceed per-line minimum
> + -original file
> + +Qa long line to exceed per-line
> minimum
> + +Qanother long line to exceed per-line
> minimum
> + +new file
> + EOF
> + test_cmp expected actual
> '
>
> test_done
> --
> 2.18.0.399.gad0ab374a1-goog
>
--
Brandon Williams
ernativeDimmed "normal blue" &&
> test_config color.diff.newMovedAlternativeDimmed "normal yellow" &&
> + git diff HEAD --no-renames --color-moved=blocks --color >actual.raw &&
> + grep -v "index" actual.raw | test_decode_color >actual &&
> + cat <<-\EOF >expected &&
> + diff --git a/lines.txt b/lines.txt
> + --- a/lines.txt
> + +++ b/lines.txt
> + @@ -1,16 +1,16 @@
> + -long line 1
> + -long line 2
> + -long line 3
> + line 4
> + line 5
> + line 6
> + line 7
> + line 8
> + line 9
> + +long line 1
> + +long line 2
> + +long line 3
> + +long line 14
> + +long line 15
> + +long line 16
> + line 10
> + line 11
> + line 12
> + line 13
> + -long line 14
> + -long line 15
> + -long line 16
> + EOF
> + test_cmp expected actual
> +
Theres an empty line here. Not worth fixing if its the only issue
though.
--
Brandon Williams
nned through the series and it looks good. Again, thanks for
putting in the work to get this done. I'm looking forward to the end
product :)
--
Brandon Williams
uch easier to handle errors. This work needs
to continue if we want to improve the submodule experience in git.
Yes this might mean there are a few more conflicts when merging a series
(because of the scope of these refactorings) but it is well worth it
given what the end-state will look like.
--
Brandon Williams
fetched.
> ++
> +This option may be specified more than once; if so, Git will report
> +commits reachable from any of the given commits.
> ++
> +The argument to this option may be a glob on ref names, a ref, or the
> (possibly
> +abbreviated SHA-1 of a commit. Specifying a glob is equivalent to specifying
> +this option multiple times, one for each matching ref name.
I think you're missing a closing ')'
Aside from that nit this patch looks good, thanks!
--
Brandon Williams
fetch is a perfect example of supporting all three. I can do
git fetch origin SHA1
git fetch origin master
git fetch origin refs/heads/*:refs/heads/*
--
Brandon Williams
@@ static int fetch(struct transport *transport,
> transport, "filter",
> data->transport_options.filter_options.filter_spec);
>
> + if (data->transport_options.negotiation_tips)
> + warning("Ignoring --negotiation-tip because the protocol does
> not support it.");
> +
> if (data->fetch)
> return fetch_with_fetch(transport, nr_heads, to_fetch);
>
> diff --git a/transport.c b/transport.c
> index a32da30dee..9f10f8ad9f 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -318,6 +318,7 @@ static int fetch_refs_via_pack(struct transport
> *transport,
> args.filter_options = data->options.filter_options;
> args.stateless_rpc = transport->stateless_rpc;
> args.server_options = transport->server_options;
> + args.negotiation_tips = data->options.negotiation_tips;
>
> if (!data->got_remote_heads)
> refs_tmp = get_refs_via_connect(transport, 0, NULL);
> diff --git a/transport.h b/transport.h
> index 7792b08582..d31be5be63 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -25,6 +25,16 @@ struct git_transport_options {
> const char *receivepack;
> struct push_cas_option *cas;
> struct list_objects_filter_options filter_options;
> +
> + /*
> + * This is only used during fetch. See the documentation of
> + * negotiation_tips in struct fetch_pack_args.
> + *
> + * This field is only supported by transports that support connect or
> + * stateless_connect. Set this field directly instead of using
> + * transport_set_option().
> + */
> + struct oid_array *negotiation_tips;
> };
>
> enum transport_family {
> --
> 2.18.0.rc2.346.g013aa6912e-goog
>
--
Brandon Williams
-ref section. Also changed the docs around the
server's responsibility with the refs that are sent during this
section.
Brandon Williams (8):
test-pkt-line: add unpack-sideband subcommand
upload-pack: implement ref-in-want
upload-pack: test negotiation with changing repository
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them. This is in preparation
for allowing additional processing of the fetched refs before updating
the local ref store.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 23
he form of ref names
through a new "want-ref " parameter. At the conclusion of
negotiation, the server will send a list of all of the wanted references
(as provided by "want-ref" lines) in addition to the generated packfile.
Signed-off-by: Brandon Williams
---
Docum
(which will be introduced to the client in subsequent
commits), and ensures that subsequent commits do not change existing
behavior.
As part of this effort, a mechanism to substitute strings in a single
HTTP response is added.
Signed-off-by: Brandon Williams
---
t/lib-httpd.sh
sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.
Signed-off-by: Brandon Williams
---
builtin/clone.c | 4 ++--
builtin/fetch.c | 28
Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides
tightening scopes of variables in the code, this also prepares for
get_ref_map being able to be called multiple times within do_fetch.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 36
struct.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 52 -
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2fabfed0e..bda00e826 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
enable unpacking packet line data sent multiplexed using a sideband.
Signed-off-by: Brandon Williams
---
t/helper/test-pkt-line.c | 33 +
1 file changed, 33 insertions(+)
diff --git
and the value of that ref will be
sent at the termination of negotiation, just before a packfile is sent.
More information on the ref-in-want feature can be found in
Documentation/technical/protocol-v2.txt.
Signed-off-by: Brandon Williams
---
fetch-pack.c
On 06/27, Junio C Hamano wrote:
> Brandon Williams writes:
>
> >> > +* The server SHOULD NOT send any refs which were not requested
> >> > + using 'want-ref' lines and a client MUST ignore refs which
> >> > + weren
no want-ref
line is going to be sent because we're requesting an exact OID here, not
a ref. But I can add checks for want-ref in the tests that should be
sending want-ref.
>
> Other than that (and my other comments), this patch series looks good.
--
Brandon Williams
On 06/26, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > Expand the transport fetch method signature, by adding an output
> > parameter, to allow transports to return information about the refs they
> > have fetched. Then communicate shallow status information th
On 06/26, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
> > new file mode 100644
> > index 0..8a9a5aca0
> > --- /dev/null
> > +++ b/t/lib-httpd/one-time-sed.sh
On 06/26, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > +wanted-refs section
> > + * This section is only included if the client has requested a
> > + ref using a 'want-ref' line and if a packfile section is also
> > + included
Changes in v5:
* Added a comment explaining the one-time-sed.sh script
* Added a number of tests per reviewer feedback
* Fixed a typo in documentation
Brandon Williams (8):
test-pkt-line: add unpack-sideband subcommand
upload-pack: implement ref-in-want
upload-pack: test negotiation with
sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.
Signed-off-by: Brandon Williams
---
builtin/clone.c | 4 ++--
builtin/fetch.c | 28
and the value of that ref will be
sent at the termination of negotiation, just before a packfile is sent.
More information on the ref-in-want feature can be found in
Documentation/technical/protocol-v2.txt.
Signed-off-by: Brandon Williams
---
fetch-pack.c
struct.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 52 -
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2fabfed0e..bda00e826 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
(which will be introduced to the client in subsequent
commits), and ensures that subsequent commits do not change existing
behavior.
As part of this effort, a mechanism to substitute strings in a single
HTTP response is added.
Signed-off-by: Brandon Williams
---
t/lib-httpd.sh
he form of ref names
through a new "want-ref " parameter. At the conclusion of
negotiation, the server will send a list of all of the wanted references
(as provided by "want-ref" lines) in addition to the generated packfile.
Signed-off-by: Brandon Williams
---
Docum
Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides
tightening scopes of variables in the code, this also prepares for
get_ref_map being able to be called multiple times within do_fetch.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 36
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them. This is in preparation
for allowing additional processing of the fetched refs before updating
the local ref store.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 23
Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
enable unpacking packet line data sent multiplexed using a sideband.
Signed-off-by: Brandon Williams
---
t/helper/test-pkt-line.c | 33 +
1 file changed, 33 insertions(+)
diff --git
ace before an arrow operator in patch 2
> * Fix a typo in the commit message of patch 3: s/fetchobjs/fetchjobs
> * Add a note in the commit message of patch 6 about checking the
> worktree before loading .gitmodules
> * Drop patch 7, it was meant as a cleanup but resulted in parsing the
> .gitmodules file twice
Thanks for making these changes, this version looks good to me!
--
Brandon Williams
(which will be introduced to the client in subsequent
commits), and ensures that subsequent commits do not change existing
behavior.
As part of this effort, a mechanism to substitute strings in a single
HTTP response is added.
Signed-off-by: Brandon Williams
---
t/lib-httpd.sh
sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.
Signed-off-by: Brandon Williams
---
builtin/clone.c | 4 ++--
builtin/fetch.c | 28
and the value of that ref will be
sent at the termination of negotiation, just before a packfile is sent.
More information on the ref-in-want feature can be found in
Documentation/technical/protocol-v2.txt.
Signed-off-by: Brandon Williams
---
fetch-pack.c | 35 +++
struct.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 52 -
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2fabfed0e..bda00e826 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides
tightening scopes of variables in the code, this also prepares for
get_ref_map being able to be called multiple times within do_fetch.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 36
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them. This is in preparation
for allowing additional processing of the fetched refs before updating
the local ref store.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 23
he form of ref names
through a new "want-ref " parameter. At the conclusion of
negotiation, the server will send a list of all of the wanted references
(as provided by "want-ref" lines) in addition to the generated packfile.
Signed-off-by: Brandon Williams
---
Docum
Changes in v4 are fairly minor. There are a few documentation changes,
commit message updates, as well as a few small style tweaks based on
reviewer comments.
Brandon Williams (8):
test-pkt-line: add unpack-sideband subcommand
upload-pack: implement ref-in-want
upload-pack: test
Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
enable unpacking packet line data sent multiplexed using a sideband.
Signed-off-by: Brandon Williams
---
t/helper/test-pkt-line.c | 33 +
1 file changed, 33 insertions(+)
diff --git
tiation_state")
>for the struct, instead of "struct data"
> - squashed patch 8 into patch 7; this means that the comments are not
>moved verbatim, but for the reviewer, verbatim-ness of comments is
>probably not that important anyway
Thanks this series looks good now.
Reviewed-by: Brandon Williams
--
Brandon Williams
As a bonus, the API is also improved,
> and global state reduced.
I've reviewed this series and haven't found any issues.
Reviewed-by: Brandon Williams
--
Brandon Williams
oid;
> > + struct string_list_item *item;
> > + struct object *o;
> > +
> > + if (read_ref(arg, &oid))
> > + die("unknown ref %s", arg);
>
> One more thing - if you're planning to "die" here, als
ds);
> > + rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
> > + if (fetched_refs && nop_head) {
> > + *nop_tail = *fetched_refs;
> > + *fetched_refs = nop_head;
> > + }
> >
> > free(heads);
> > return rc;
>
> And sometimes, even if we are merely simulating the fetching of refs, we
> still need to report those refs in fetched_refs. This is correct.
>
> I also see that t5703 now passes.
>
> Besides enabling the writing of subsequent patches, I see that this also
> makes the API clearer in that the input refs to transport_fetch_refs()
> are not overloaded to output shallow information. Other than the " = 0"
> change above, this patch looks good to me.
Perfect, I'll just drop the " = 0" part (making the diff slightly
smaller)
--
Brandon Williams
1 - 100 of 2393 matches
Mail list logo