From: Jonathan Tan <firstname.lastname@example.org> To: Stefan Beller <email@example.com> Cc: git <firstname.lastname@example.org> Subject: Re: [PATCH] submodule: do not pass null OID to setup_revisions Date: Thu, 24 May 2018 16:30:46 -0700 Message-ID: <email@example.com> (raw) In-Reply-To: <CAGZ79kbB-8dbO5QjpG-G+9uRpa1m-_oypfw=kRFu__Ws7FV0Xg@mail.gmail.com> On Thu, 24 May 2018 16:07:49 -0700 Stefan Beller <firstname.lastname@example.org> wrote: > Hi Jonathan, > > On Thu, May 24, 2018 at 1:47 PM, Jonathan Tan <email@example.com> wrote: > > If "git pull --recurse-submodules --rebase" is invoked when the current > > branch and its corresponding remote-tracking branch have no merge base, > > a "bad object" fatal error occurs. This issue was introduced with commit > > a6d7eb2c7a ("pull: optionally rebase submodules (remote submodule > > changes only)", 2017-06-23), which also introduced this feature. > > Ok, what should happen instead? Just for there not to be this "bad object" error. > > This is because cmd_pull() in builtin/pull.c thus invokes > > submodule_touches_in_range() with a null OID as the first parameter. > > Ensure that this case works, and document what happens in this case. > > By documenting you mean adding a test, i.e. documenting it for the > developers, not the users. I also updated the submodule.h file, but yes, it is for the developers. I'll change the commit message to make this more clear if I need a reroll. > I inserted a test_pause here and inspect child: > * the submodule is the same as in parent, so this patch is > just testing it works with submodules the same? > * No, the submodule is not cloned into the child > at all. So we do not know what do do with the submodule. Yes, this test doesn't do much. I just wanted to make sure that submodule_touches_in_range() could be called without encountering this unrelated error. (Incidentally, we might want to add tests for the "cannot rebase with locally recorded submodule modifications", but I haven't looked into that.) > However this patch is about making sure the superproject > works out well, without this patch we'd have > $ git -C child pull --recurse-submodules --rebase > fatal: bad object 0000000000000000000000000000000000000000 > which is to be avoided. > > Yes I think this is the best way to fix the issue, I thought for some time that > could first check if submodules are initialzed or active, but these > are checks are done afterwards, so this is ok. > > Reviewed-by: Stefan Beller <firstname.lastname@example.org> Thanks!
prev parent reply index Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-24 20:47 Jonathan Tan 2018-05-24 23:07 ` Stefan Beller 2018-05-24 23:30 ` Jonathan Tan [this message]
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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
email@example.com list mirror (unofficial, 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/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox