From: Stefan Beller <firstname.lastname@example.org> To: Jonathan Tan <email@example.com> Cc: Junio C Hamano <firstname.lastname@example.org>, git <email@example.com> Subject: Re: [PATCH 8/9] fetch: retry fetching submodules if needed objects were not fetched Date: Thu, 25 Oct 2018 14:42:15 -0700 Message-ID: <CAGZ79kY8bXR2OsdmKMyB+OJP67sWCF=N4KfUdUcpKy5ZQwcV9A@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Tue, Oct 23, 2018 at 4:38 PM Jonathan Tan <email@example.com> wrote: > > > > Another thing you need to clarify is what happens if the fetch-by-commit > > > fails. Right now, it seems that it will make the whole thing fail, which > > > might be a surprising change in behavior. > > > > But a positive surprise, I would assume? > > Whether positive or negative, I think that this needs to be mentioned in > the commit message. > > As for positive or negative, I tend to agree that it's positive - sure, > some previously successful fetches would now fail, but the results of > those fetches could not be recursively checked out anyway. > > > > The test stores the result in a normal branch, not a remote tracking > > > branch. Is storing in a normal branch required? > > > > In the test we fetch from another repository, such that in the > > repository-under-test this will show up in a remote tracking branch? I messed up there. Yes, we need to fetch into a normal branch such that the logic of check_for_new_submodule_commits triggers no matter where it is on the remote. Your experiment below shows that we cannot fetch into FETCH_HEAD: > If that were true, I would expect that when this line: > > > git fetch --recurse-submodules --recurse-submodules-default on-demand origin refs/changes/2:refs/heads/my_branch && > > is replaced by this line: > > > git fetch --recurse-submodules --recurse-submodules-default on-demand origin refs/changes/2 && > > then things would still work. The tests pass with the first line (after > I fixed a type mismatch) but not with the second. (Also I don't think a > remote-tracking branch is generated here - the output printed doesn't > indicate so, and refs/changes/2 is not a branch anyway.) > > > Also, do you know why this is required? A naive reading of the patch > > > leads me to believe that this should work even if merely fetching to > > > FETCH_HEAD. > > > > See the next patch, check_for_new_submodule_commits() is missing > > for FETCH_HEAD. > > I see in the next patch that there is an "if" branch in > store_updated_refs() without update_local_ref() in which > "check_for_new_submodule_commits(&rm->old_oid)" needs to be inserted. I > think this is a symptom that maybe check_for_new_submodule_commits() > needs to be extracted from update_local_ref() and put into > store_updated_refs() instead? In update_local_ref(), it is called on > ref->new_oid, which is actually the same as rm->old_oid anyway (there is > an oidcpy earlier). I'll look into that. > > > What is a "default" submodule and why would you need one? > > > > s/default/artificial/. Such a submodule is a submodule that has no > > config in the .gitmodules file and its name == path. > > We need to keep those around for historic reasons AFAICT, c.f. > > c68f837576 (implement fetching of moved submodules, 2017-10-16) > > Ah, OK. I would call it a fake submodule then, and copy over the "No > entry in .gitmodules?" comment. "fake submodule" sounds like http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb which is what I think of when hearing fake submodules.
next prev parent reply index Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-16 18:13 [PATCH 0/9] Resending sb/submodule-recursive-fetch-gets-the-tip Stefan Beller 2018-10-16 18:13 ` [PATCH 1/9] sha1-array: provide oid_array_filter Stefan Beller 2018-10-16 18:13 ` [PATCH 2/9] submodule.c: fix indentation Stefan Beller 2018-10-16 18:13 ` [PATCH 3/9] submodule.c: sort changed_submodule_names before searching it Stefan Beller 2018-10-17 21:21 ` Jonathan Tan 2018-10-16 18:13 ` [PATCH 4/9] submodule.c: move global changed_submodule_names into fetch submodule struct Stefan Beller 2018-10-17 21:26 ` Jonathan Tan 2018-10-18 19:09 ` Stefan Beller 2018-10-16 18:13 ` [PATCH 5/9] submodule.c: do not copy around submodule list Stefan Beller 2018-10-17 21:45 ` Jonathan Tan 2018-10-18 2:35 ` Junio C Hamano 2018-10-16 18:13 ` [PATCH 6/9] repository: repo_submodule_init to take a submodule struct Stefan Beller 2018-10-17 21:55 ` Jonathan Tan 2018-10-16 18:13 ` [PATCH 7/9] submodule: fetch in submodules git directory instead of in worktree Stefan Beller 2018-10-17 22:58 ` Jonathan Tan 2018-10-23 18:26 ` Stefan Beller 2018-10-23 22:55 ` Jonathan Tan 2018-10-23 23:01 ` Stefan Beller 2018-10-16 18:13 ` [PATCH 8/9] fetch: retry fetching submodules if needed objects were not fetched Stefan Beller 2018-10-18 0:39 ` Jonathan Tan 2018-10-23 22:37 ` Stefan Beller 2018-10-23 23:37 ` Jonathan Tan 2018-10-25 21:42 ` Stefan Beller [this message] 2018-10-16 18:13 ` [PATCH 9/9] builtin/fetch: check for submodule updates for non branch fetches Stefan Beller 2018-10-18 0:47 ` Jonathan Tan 2018-10-18 2:30 ` [PATCH 0/9] Resending sb/submodule-recursive-fetch-gets-the-tip Junio C Hamano 2018-10-18 7:30 ` Junio C Hamano 2018-10-18 18:00 ` Stefan Beller -- strict thread matches above, loose matches on Subject: below -- 2018-09-11 23:49 [PATCH 0/9] fetch: make sure submodule oids are fetched Stefan Beller 2018-09-17 21:35 ` [PATCHv2 " Stefan Beller 2018-09-17 21:35 ` [PATCH 8/9] fetch: retry fetching submodules if needed objects were not fetched Stefan Beller
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAGZ79kY8bXR2OsdmKMyB+OJP67sWCF=N4KfUdUcpKy5ZQwcV9A@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
firstname.lastname@example.org mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox