mptcp: MPTCP_KUNIT_TESTS should depend on MPTCP instead of selecting it
diff mbox series

Message ID 20201019113240.11516-1-geert@linux-m68k.org
State Accepted
Commit b142083b585c2c03af24cca4d274f797796a4064
Headers show
Series
  • mptcp: MPTCP_KUNIT_TESTS should depend on MPTCP instead of selecting it
Related show

Commit Message

Geert Uytterhoeven Oct. 19, 2020, 11:32 a.m. UTC
MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
user may not want to enable.  Fix this by making the test depend on
MPTCP instead.

Fixes: a00a582203dbc43e ("mptcp: move crypto test to KUNIT")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 net/mptcp/Kconfig | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Matthieu Baerts Oct. 19, 2020, 3:47 p.m. UTC | #1
Hi Geert,

Thank you for the patch!

On 19/10/2020 13:32, Geert Uytterhoeven wrote:
> MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
> user may not want to enable.  Fix this by making the test depend on
> MPTCP instead.

I think the initial intension was to select MPTCP to have an easy way to 
enable all KUnit tests. We imitated what was and is still done in 
fs/ext4/Kconfig.

But it probably makes sense to depend on MPTCP instead of selecting it. 
So that's fine for me. But then please also send a patch to ext4 
maintainer to do the same there.

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Cheers,
Matt
Geert Uytterhoeven Oct. 19, 2020, 8:38 p.m. UTC | #2
Hi Matthieu,

On Mon, Oct 19, 2020 at 5:47 PM Matthieu Baerts
<matthieu.baerts@tessares.net> wrote:
> On 19/10/2020 13:32, Geert Uytterhoeven wrote:
> > MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
> > user may not want to enable.  Fix this by making the test depend on
> > MPTCP instead.
>
> I think the initial intension was to select MPTCP to have an easy way to
> enable all KUnit tests. We imitated what was and is still done in
> fs/ext4/Kconfig.
>
> But it probably makes sense to depend on MPTCP instead of selecting it.
> So that's fine for me. But then please also send a patch to ext4
> maintainer to do the same there.

Thanks, good point.  I didn't notice, as I did have ext4 enabled anyway.
Will send a patch for ext4.  Looks like ext4 and MPTCP where the only
test modules selecting their dependencies.

> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Thanks!

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Oct. 20, 2020, 7:40 a.m. UTC | #3
On Mon, Oct 19, 2020 at 10:38 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Oct 19, 2020 at 5:47 PM Matthieu Baerts
> <matthieu.baerts@tessares.net> wrote:
> > On 19/10/2020 13:32, Geert Uytterhoeven wrote:
> > > MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
> > > user may not want to enable.  Fix this by making the test depend on
> > > MPTCP instead.
> >
> > I think the initial intension was to select MPTCP to have an easy way to
> > enable all KUnit tests. We imitated what was and is still done in
> > fs/ext4/Kconfig.
> >
> > But it probably makes sense to depend on MPTCP instead of selecting it.
> > So that's fine for me. But then please also send a patch to ext4
> > maintainer to do the same there.
>
> Thanks, good point.  I didn't notice, as I did have ext4 enabled anyway.
> Will send a patch for ext4.  Looks like ext4 and MPTCP where the only
> test modules selecting their dependencies.

FTR, "[PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead
of  selecting it"
https://lore.kernel.org/lkml/20201020073740.29081-1-geert@linux-m68k.org/


Gr{oetje,eeting}s,

                        Geert
Matthieu Baerts Oct. 20, 2020, 9:27 a.m. UTC | #4
Hi Geert,

On 20/10/2020 09:40, Geert Uytterhoeven wrote:
> On Mon, Oct 19, 2020 at 10:38 PM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Oct 19, 2020 at 5:47 PM Matthieu Baerts
>> <matthieu.baerts@tessares.net> wrote:
>>> On 19/10/2020 13:32, Geert Uytterhoeven wrote:
>>>> MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
>>>> user may not want to enable.  Fix this by making the test depend on
>>>> MPTCP instead.
>>>
>>> I think the initial intension was to select MPTCP to have an easy way to
>>> enable all KUnit tests. We imitated what was and is still done in
>>> fs/ext4/Kconfig.
>>>
>>> But it probably makes sense to depend on MPTCP instead of selecting it.
>>> So that's fine for me. But then please also send a patch to ext4
>>> maintainer to do the same there.
>>
>> Thanks, good point.  I didn't notice, as I did have ext4 enabled anyway.
>> Will send a patch for ext4.  Looks like ext4 and MPTCP where the only
>> test modules selecting their dependencies.
> 
> FTR, "[PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead
> of  selecting it"
> https://lore.kernel.org/lkml/20201020073740.29081-1-geert@linux-m68k.org/

Thank you for having sent this other patch and shared the link here!

Groetjes,
Matt
Jakub Kicinski Oct. 20, 2020, 11:55 p.m. UTC | #5
On Mon, 19 Oct 2020 17:47:20 +0200 Matthieu Baerts wrote:
> On 19/10/2020 13:32, Geert Uytterhoeven wrote:
> > MPTCP_KUNIT_TESTS selects MPTCP, thus enabling an optional feature the
> > user may not want to enable.  Fix this by making the test depend on
> > MPTCP instead.  
> 
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Applied, thanks!

Patch
diff mbox series

diff --git a/net/mptcp/Kconfig b/net/mptcp/Kconfig
index 698bc35251609755..abb0a992d4a0855a 100644
--- a/net/mptcp/Kconfig
+++ b/net/mptcp/Kconfig
@@ -22,11 +22,8 @@  config MPTCP_IPV6
 	select IPV6
 	default y
 
-endif
-
 config MPTCP_KUNIT_TESTS
 	tristate "This builds the MPTCP KUnit tests" if !KUNIT_ALL_TESTS
-	select MPTCP
 	depends on KUNIT
 	default KUNIT_ALL_TESTS
 	help
@@ -39,3 +36,4 @@  config MPTCP_KUNIT_TESTS
 
 	  If unsure, say N.
 
+endif