Re: [PATCH 7/9] submodule: fetch in submodules git directory instead of in worktree
- From
Stefan Beller <sbeller@google.com>
- Date
- Oct 23, 2018, 23:01 UTC
- Message-ID
- <CAGZ79kbNXD35ZwevjLZcrGsT=2hNcUPmVUWvP1RjsKSH0Gd3ww@mail.gmail.com>
- In-Reply-To
- <20181023225536.61508-1-jonathantanmy@google.com>
On Tue, Oct 23, 2018 at 3:55 PM Jonathan Tan <jonathantanmy@google.com> wrote:
Show 8 quoted lines
> > When adding the comment here, we'd also want to have > > the comment in prepare_submodule_repo_env, which > > could be its own preparation commit. > > I agree with the protection. As for the preparation commit, I don't > think it's always the code author's responsibility to tidy up the > surrounding code, but since you're adding an identical comment here, > it's probably worth it to add the comment there too.
Am I the only one who dislikes inconsistent files? ;-) (ie. clean in one place, not cleaned up in another) I can see your point. Will add a comment
Show 6 quoted lines
> > Thinking of that, maybe we need to announce that in get_next_submodule > > The consequence of getting caught changes, though. Currently, > spf->result is set to 1 whenever a child process fails. But in this > patch, some of these repositories would be entirely skipped, meaning > that no child process is run, and spf->result is never modified.
Right.
Show 7 quoted lines
> > If the working tree directory is empty for that submodule, it means > > it is likely not initialized. But why would we use that as a signal to > > skip the submodule? > > What I meant was: if empty, skip it completely. Otherwise, do the > repo_submodule_init() and repo_init() thing, and if they both fail, set > spf->result to 1, preserving existing behavior.
I did it the other way round:
If repo_[submodule_]init fails, see if we have a gitlink in tree and an empty dir in the FS, to decide if we need to signal failure.
I can switch it around again, but it seemed easier to write as that puts corner cases away into one else {} case.