On Mon, Sep 18, 2023 at 5:56 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > > This is to be expected, in my opinion. In terms of revision.c and > > setting the `revs->ignore_missing_links` bit, the traversal will > > go throw all objects (commits and otherwise) and call > > `show_commit` or `show_object` on them. > > Yes. And the user can choose how to handle such an object here by > telling finish_object__ma() with the --missing=<how> option, so > letting them do so, instead of robbing the choice from them, would > be a more flexible design here, right? > > > if a commit is > > missing, git-rev-list(1) will still barf an error, but this error > > OK, yeah, I do see the need for setting the ignore-missing-links bit > for what you are doing, and --missing and --ignore-missing-links are > orthogonal options. Getting rid of the hardcoded skipping of > finish_object__ma() would make sense from this angle, too. Well. The only problem is that setting `ignore_missing_links` bit never calls `show_commit` for missing commits (since commits are pre-parsed in revision.c). So to keep that behavior consistent for non-commit objects, I hardcoded the skipping of `finish_object__ma()` in `show_object`. If I remove the hardcoding, it would mean that `--ignore-missing-links` would skip missing commits but for non-commits objects, the user would have to pass `--missing=allow-any` else rev-list would still error out with a missing object error. Don't you think this would be confusing for the user? I'm happy to send a revised version removing this hardcoding if you still think otherwise :)
