| Message ID | 20231205184503.79769-2-Nikolai.Kondrashov@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MAINTAINERS: Introduce V: entry for tests | expand |
On Tue, 2023-12-05 at 20:02 +0200, Nikolai Kondrashov wrote: > Do not die, but only warn when scripts/get_maintainer.pl is asked to > retrieve information about a missing file. > > This allows scripts/checkpatch.pl to query MAINTAINERS while processing > patches which are removing files. Why is this useful? Give a for-instance example please.
Hi Joe, On 12/5/23 20:55, Joe Perches wrote: > On Tue, 2023-12-05 at 20:02 +0200, Nikolai Kondrashov wrote: >> Do not die, but only warn when scripts/get_maintainer.pl is asked to >> retrieve information about a missing file. >> >> This allows scripts/checkpatch.pl to query MAINTAINERS while processing >> patches which are removing files. > > Why is this useful? > > Give a for-instance example please. Sure! Take the ebb0130dad751e88c28ab94c71e46e8ee65427c9 commit for example. It removes a file (and recreates it in another format, but that's besides the point) which belongs to four subsystems. This will work OK: scripts/get_maintainer.pl 0001-dt-bindings-mailbox-convert-bcm2835-mbox-bindings-to.patch But if we try to give the file being removed to get_maintainer.pl directly: scripts/get_maintainer.pl -f Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt It will abort with the following message: scripts/get_maintainer.pl: file 'Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt' not found Even though the state of the source tree we're running this on is *exactly* the same. The latter (using the -f option) is the way checkpatch.pl works to fetch the maintenance status (in is_maintained_obsolete()), and the way I implemented fetching the tests from MAINTAINERS (in get_file_proposed_tests()). This way seems to work better for checkpatch.pl, I suppose, because it can link the error message to a specific file. But in principle, it might be possible to just call get_maintainer.pl on every patch, if we really have to. However, I feel that conceptually it should be possible to query MAINTAINERS without actual *source* files being present in the tree (as opposed to patch files which it needs to read), or even the tree being there at all. I saw that we are printing a warning if the queried file is not there in the git repo (I guess it's helpful?), and thought perhaps we can do the same without the git repo too. Hope that helps! Nick
On 12/5/23 20:55, Joe Perches wrote: > On Tue, 2023-12-05 at 20:02 +0200, Nikolai Kondrashov wrote: >> Do not die, but only warn when scripts/get_maintainer.pl is asked to >> retrieve information about a missing file. >> >> This allows scripts/checkpatch.pl to query MAINTAINERS while processing >> patches which are removing files. > > Why is this useful? > > Give a for-instance example please. Does the example I provided make sense, Joe? Would my solution be adequate, or would you like to have another? Thank you. Nick
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 16d8ac6005b6f..37901c2298388 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -541,7 +541,11 @@ foreach my $file (@ARGV) { if ((-d $file)) { $file =~ s@([^/])$@$1/@; } elsif (!(-f $file)) { - die "$P: file '${file}' not found\n"; + if ($from_filename) { + warn "$P: file '${file}' not found\n"; + } else { + die "$P: file '${file}' not found\n"; + } } } if ($from_filename && (vcs_exists() && !vcs_file_exists($file))) {
Do not die, but only warn when scripts/get_maintainer.pl is asked to retrieve information about a missing file. This allows scripts/checkpatch.pl to query MAINTAINERS while processing patches which are removing files. Signed-off-by: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com> --- scripts/get_maintainer.pl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)