Message ID | 20210121160115.4676-1-lukas.bulwahn@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | MAINTAINERS: adjust to clang-version.sh removal | expand |
On Thu, Jan 21, 2021 at 5:01 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > Kconfig") removed ./scripts/clang-version.sh and moved its content to > ./scripts/cc-version.sh. > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > warning: no file matches F: scripts/clang-version.sh > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > changes in ./scripts/clang-version.sh; as the file is removed, track > changes in ./scripts/cc-version.sh instead now. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Good catch, Lukas. As a tipp: Next time you can pass '--subject-prefix="PATCH next-YYYYMMDD"' when doing 'git format-patch ...' (or whatever you use to generate the patch). Cannot say if we can add a "Fixes:" tag with commit hash-id of "kbuild: check the minimum compiler version in Kconfig" - this missed that move. Maybe Masahiro does a respin and can fold this into above commit? That's not my decision. Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com> - Sedat - [1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=for-next&id=6c8ad4427f6ea306a1eee951d684a41f517b5986 > --- > applies cleanly on next-20210121 > > Masahiro-san, please pick this quick fix-up patch. > > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e5d7cf38ec82..aafbea806a82 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4355,8 +4355,8 @@ B: https://github.com/ClangBuiltLinux/linux/issues > C: irc://chat.freenode.net/clangbuiltlinux > F: Documentation/kbuild/llvm.rst > F: include/linux/compiler-clang.h > +F: scripts/cc-version.sh > F: scripts/clang-tools/ > -F: scripts/clang-version.sh > F: scripts/lld-version.sh > K: \b(?i:clang|llvm)\b > > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210121160115.4676-1-lukas.bulwahn%40gmail.com.
On Thu, Jan 21, 2021 at 05:01:15PM +0100, Lukas Bulwahn wrote: > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > Kconfig") removed ./scripts/clang-version.sh and moved its content to > ./scripts/cc-version.sh. > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > warning: no file matches F: scripts/clang-version.sh > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > changes in ./scripts/clang-version.sh; as the file is removed, track > changes in ./scripts/cc-version.sh instead now. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > --- > applies cleanly on next-20210121 > > Masahiro-san, please pick this quick fix-up patch. Masahiro cannot pick this up because the patch to add clang-version.sh to MAINTAINERS is in mmotm. I think the better solution is for Andrew to drop the current version of maintainers-add-a-couple-more-files-to-the-clang-llvm-section.patch and pick up the second one I sent, which allows us to deal with this: https://lore.kernel.org/lkml/20210114171629.592007-1-natechancellor@gmail.com/ I am not sure it is right for us to maintain cc-version.sh but I am open to it if Masahiro agrees. > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e5d7cf38ec82..aafbea806a82 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4355,8 +4355,8 @@ B: https://github.com/ClangBuiltLinux/linux/issues > C: irc://chat.freenode.net/clangbuiltlinux > F: Documentation/kbuild/llvm.rst > F: include/linux/compiler-clang.h > +F: scripts/cc-version.sh > F: scripts/clang-tools/ > -F: scripts/clang-version.sh > F: scripts/lld-version.sh > K: \b(?i:clang|llvm)\b > > -- > 2.17.1 >
On Thu, Jan 21, 2021 at 5:16 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 05:01:15PM +0100, Lukas Bulwahn wrote: > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > ./scripts/cc-version.sh. > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > warning: no file matches F: scripts/clang-version.sh > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > changes in ./scripts/clang-version.sh; as the file is removed, track > > changes in ./scripts/cc-version.sh instead now. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > --- > > applies cleanly on next-20210121 > > > > Masahiro-san, please pick this quick fix-up patch. > > Masahiro cannot pick this up because the patch to add clang-version.sh > to MAINTAINERS is in mmotm. > > I think the better solution is for Andrew to drop the current version of > > maintainers-add-a-couple-more-files-to-the-clang-llvm-section.patch > > and pick up the second one I sent, which allows us to deal with this: > > https://lore.kernel.org/lkml/20210114171629.592007-1-natechancellor@gmail.com/ > > I am not sure it is right for us to maintain cc-version.sh but I am open > to it if Masahiro agrees. > Sounds like a good idea to integrate both patches from Lukas and Nathan into a new version of "kbuild: check the minimum compiler version in Kconfig". ...up to the maintainers. - Sedat - > > MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index e5d7cf38ec82..aafbea806a82 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -4355,8 +4355,8 @@ B: https://github.com/ClangBuiltLinux/linux/issues > > C: irc://chat.freenode.net/clangbuiltlinux > > F: Documentation/kbuild/llvm.rst > > F: include/linux/compiler-clang.h > > +F: scripts/cc-version.sh > > F: scripts/clang-tools/ > > -F: scripts/clang-version.sh > > F: scripts/lld-version.sh > > K: \b(?i:clang|llvm)\b > > > > -- > > 2.17.1 > > > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210121161640.GA1101379%40ubuntu-m3-large-x86.
On Thu, Jan 21, 2021 at 5:16 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 5:01 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > ./scripts/cc-version.sh. > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > warning: no file matches F: scripts/clang-version.sh > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > changes in ./scripts/clang-version.sh; as the file is removed, track > > changes in ./scripts/cc-version.sh instead now. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > Good catch, Lukas. > > As a tipp: > Next time you can pass '--subject-prefix="PATCH next-YYYYMMDD"' when > doing 'git format-patch ...' (or whatever you use to generate the > patch). > Thanks for the hint. > Cannot say if we can add a "Fixes:" tag with commit hash-id of > "kbuild: check the minimum compiler version in Kconfig" - this missed > that move. > Maybe Masahiro does a respin and can fold this into above commit? > That's not my decision. > > Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com> > ...and thanks for the review. Lukas
On Thu, Jan 21, 2021 at 5:16 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 05:01:15PM +0100, Lukas Bulwahn wrote: > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > ./scripts/cc-version.sh. > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > warning: no file matches F: scripts/clang-version.sh > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > changes in ./scripts/clang-version.sh; as the file is removed, track > > changes in ./scripts/cc-version.sh instead now. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > --- > > applies cleanly on next-20210121 > > > > Masahiro-san, please pick this quick fix-up patch. > > Masahiro cannot pick this up because the patch to add clang-version.sh > to MAINTAINERS is in mmotm. > > I think the better solution is for Andrew to drop the current version of > > maintainers-add-a-couple-more-files-to-the-clang-llvm-section.patch > > and pick up the second one I sent, which allows us to deal with this: > > https://lore.kernel.org/lkml/20210114171629.592007-1-natechancellor@gmail.com/ > > I am not sure it is right for us to maintain cc-version.sh but I am open > to it if Masahiro agrees. > Okay, I did not see in linux-next that both changes are coming into linux-next through to different trees. Nathan, I guess if you send a follow-up patch to Andrew that is the best way to handle it, or we wait until both trees land in mainline, and then just provide a quick fix like this afterwards. So, Masahiro-san, no need to pick this patch here. Lukas
On Fri, Jan 22, 2021 at 1:16 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 05:01:15PM +0100, Lukas Bulwahn wrote: > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > ./scripts/cc-version.sh. > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > warning: no file matches F: scripts/clang-version.sh > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > changes in ./scripts/clang-version.sh; as the file is removed, track > > changes in ./scripts/cc-version.sh instead now. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > --- > > applies cleanly on next-20210121 > > > > Masahiro-san, please pick this quick fix-up patch. > > Masahiro cannot pick this up because the patch to add clang-version.sh > to MAINTAINERS is in mmotm. > > I think the better solution is for Andrew to drop the current version of > > maintainers-add-a-couple-more-files-to-the-clang-llvm-section.patch > > and pick up the second one I sent, which allows us to deal with this: > > https://lore.kernel.org/lkml/20210114171629.592007-1-natechancellor@gmail.com/ I agree. > I am not sure it is right for us to maintain cc-version.sh but I am open > to it if Masahiro agrees. I am not sure either. The part in cc-version.sh maintained by Clang folks will be this single line: clang_min_version=10.0.1 You can add cc-version.sh to the coverage if you want. Or, the following line in MAINTAINERS might be enough to catch the clang version change. K: \b(?i:clang|llvm)\b I will leave up to you guys. > > > MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index e5d7cf38ec82..aafbea806a82 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -4355,8 +4355,8 @@ B: https://github.com/ClangBuiltLinux/linux/issues > > C: irc://chat.freenode.net/clangbuiltlinux > > F: Documentation/kbuild/llvm.rst > > F: include/linux/compiler-clang.h > > +F: scripts/cc-version.sh > > F: scripts/clang-tools/ > > -F: scripts/clang-version.sh > > F: scripts/lld-version.sh > > K: \b(?i:clang|llvm)\b > > > > -- > > 2.17.1 > > > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210121161640.GA1101379%40ubuntu-m3-large-x86.
On Thu, Jan 21, 2021 at 05:15:56PM +0100, Sedat Dilek wrote: > On Thu, Jan 21, 2021 at 5:01 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > ./scripts/cc-version.sh. > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > warning: no file matches F: scripts/clang-version.sh > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > changes in ./scripts/clang-version.sh; as the file is removed, track > > changes in ./scripts/cc-version.sh instead now. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > Good catch, Lukas. > > As a tipp: > Next time you can pass '--subject-prefix="PATCH next-YYYYMMDD"' when > doing 'git format-patch ...' (or whatever you use to generate the > patch). I've never seen anyone use this prefix before. What does the date really help? In staging, we apply everything on top of staging-next and if it doesn't apply then we don't investigate, we just say "doesn't apply. resend if needed". We may as well just say [PATCH linux-next]. No one is ever going to look up the date if it doesn't apply to the latest linux-next. regards, dan carpenter
On Fri, Jan 22, 2021 at 1:34 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Thu, Jan 21, 2021 at 05:15:56PM +0100, Sedat Dilek wrote: > > On Thu, Jan 21, 2021 at 5:01 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > > > Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in > > > Kconfig") removed ./scripts/clang-version.sh and moved its content to > > > ./scripts/cc-version.sh. > > > > > > Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: > > > > > > warning: no file matches F: scripts/clang-version.sh > > > > > > The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track > > > changes in ./scripts/clang-version.sh; as the file is removed, track > > > changes in ./scripts/cc-version.sh instead now. > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > > Good catch, Lukas. > > > > As a tipp: > > Next time you can pass '--subject-prefix="PATCH next-YYYYMMDD"' when > > doing 'git format-patch ...' (or whatever you use to generate the > > patch). > > I've never seen anyone use this prefix before. > > What does the date really help? In staging, we apply everything on top > of staging-next and if it doesn't apply then we don't investigate, we > just say "doesn't apply. resend if needed". > > We may as well just say [PATCH linux-next]. No one is ever going to > look up the date if it doesn't apply to the latest linux-next. > Is there an official rule to label patches for Linux-next? Usually - when I was more active on Linux-next development - folks add a "PATCH -next" to the subject. Of course, this needs additionally a hint in the patch/commit message against which Linux-next release it is applicable. Linux-next releases are highly dynamic - a patch might be applicable to one single "-next" release. Git trees come and go - are resetted to an older version of a Git tree. As LKML is CCed - think of the hundreds and thousands of patches coming in daily. So a more meaningful subject can give a first orientation. That was my point. My €0,02. - Sedat -
In networking then they want you to say which tree it applies to, but it's not as simple as saying "net" vs "net-next". If it's a bugfix then you should write that against "net" but if it's a clean up or a fix to a recent change then it should be written against "net-next". Also linux-next is not necessarily the same thing as net-next. Networking patches should be written against either net or net-next, not linux-next. BPF tried to implement similar rules to they're not big enough to impose their own rules. It's quite a big headache to try to figure out which tree to use if you're like me and have no clue about bpf. Anyway, the point of the net vs net-next is that devs are supposed to figure out the exact tree and they're supposed to only write net-next if it doesn't apply to net. It's not clear to me the value of putting linux-next in the subject. Doesn't everyone develop against the latest devel tree? Certainly I can't imagine any maintainers doing extra work to try figure out the date of the linux-next release. Surely, they just say "Doesn't apply to foo-tree. Resend if necessary." That's the fastest and easiest response when patches don't apply. regards, dan carpente
diff --git a/MAINTAINERS b/MAINTAINERS index e5d7cf38ec82..aafbea806a82 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4355,8 +4355,8 @@ B: https://github.com/ClangBuiltLinux/linux/issues C: irc://chat.freenode.net/clangbuiltlinux F: Documentation/kbuild/llvm.rst F: include/linux/compiler-clang.h +F: scripts/cc-version.sh F: scripts/clang-tools/ -F: scripts/clang-version.sh F: scripts/lld-version.sh K: \b(?i:clang|llvm)\b
Commit 6c8ad4427f6e ("kbuild: check the minimum compiler version in Kconfig") removed ./scripts/clang-version.sh and moved its content to ./scripts/cc-version.sh. Since then, ./scripts/get_maintainer.pl --self-test=patterns complains: warning: no file matches F: scripts/clang-version.sh The CLANG/LLVM BUILD SUPPORT section in MAINTAINERS intends to track changes in ./scripts/clang-version.sh; as the file is removed, track changes in ./scripts/cc-version.sh instead now. Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> --- applies cleanly on next-20210121 Masahiro-san, please pick this quick fix-up patch. MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)