[PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Leonardo Brás]

Avoids building driver if 'make drivers/parisc/' is called and
CONFIG_PARISC is disabled.

Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
---
 drivers/parisc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
index 3cd5e6cb8478..80049d763aa0 100644
--- a/drivers/parisc/Makefile
+++ b/drivers/parisc/Makefile
@@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)		+= eisa.o eisa_enumerator.o eisa_eeprom.o
 obj-$(CONFIG_SUPERIO)		+= superio.o
 obj-$(CONFIG_CHASSIS_LCD_LED)	+= led.o
 obj-$(CONFIG_PDC_STABLE)	+= pdc_stable.o
-obj-y				+= power.o
+obj-$(CONFIG_PARISC)		+= power.o
 
-- 
2.19.0

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Leonardo Bras]

Hello Michael,

> That already works, doesn't it? So all that you'd need is an offline
> tool to precompute what drivers to actually build with a given config.
>
> 'make -n' with some suitable output mangling might do the job.
>
> There may well be other ways to achieve your stated goal, without any
> need to make changes to the kernel build process (which is the result of
> many years of evolution and tuning, BTW).

Thanks for the info, I will try to use it.

> > This change is not supposed to bother the usual way of building the kernel, and
>
> Enough people have voiced their concern to warrant that you should back
> up that claim, IMO. Have you verified that your patchset does not change
> current behaviour when building the entire set of default configurations
> for each supported architecture? Does it reduce or increase overall
> complexity of the build process?
>
I have tried in some ARCHs and it worked fine. Out of curiosity, I
will try on all
of them.

> > it is not even supposed to add overhead to kernel compilation. And it would,
> > at least, solve my problem with the 3h limit, and enable the tool
> > I am building on GiltabCI to help other developers.
>
> (Apropos of nothing: Am I the only one who thinks gitlab might take a
> rather dim view of your creativity in dealing with their limit?)
>

They make available 50k minutes a month for OSS projects. I don't believe they
care how it's spent if its used to build/deploy the project. They even
allow using
several 'jobs' in parallel in order to speed up the process.

Thanks for your help,

Leonardo Bras

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Michael Schmitz]

Am 05.10.2018 um 15:16 schrieb Leonardo Bras:
>> Well it's not really that persuasive.  Most people simply let the build
>> run to completion, but if you have a problem with a job control 3h
>> timelimit, then create a job that kills itself at 2:59 and then
>> resubmits itself.  That will produce a complete build in 3h chunks
>> without any need to call sub Makefiles.
>>
>
> Humm, I probably should have explained better how GitlabCI works.
> It works creating a docker container that have a limited lifespan of 3h max.
> After that the time is over, this container ceases to exist, leaving no build
> objects, only the console log. So there is no way of 'resuming' the building
> from where it stopped. I used the 'job' term because it's how they call it,
> and I understand it's easily confused with bash jobs.
>
>> All of our Makefiles are coded assuming the upper level can prevent
>> descent into the lower ones.  You're proposing to change that
>> assumption, requiring a fairly large patch set, which doesn't really
>> seem to provide a huge benefit.
>>
>> James
>
> I understand your viewpoint.
> But what I propose is not to change that assumption, but instead give some
> Makefiles the aditional ability to be called directly and still not build stuff
> if they were not enabled in .config.
>
> But, why these chosen Makefiles, and not all of them?
> Granularity.
> What I am trying to achieve with this patchset is the ability of building
> smaller sets of drivers without accidentally building what is not enabled
> on .config.
> And, in my viewpoint, building a single drivers/DRIVERNAME is small enough to
> be fast in most situations.

That already works, doesn't it? So all that you'd need is an offline 
tool to precompute what drivers to actually build with a given config.

'make -n' with some suitable output mangling might do the job.

There may well be other ways to achieve your stated goal, without any 
need to make changes to the kernel build process (which is the result of 
many years of evolution and tuning, BTW).

> This change is not supposed to bother the usual way of building the kernel, and

Enough people have voiced their concern to warrant that you should back 
up that claim, IMO. Have you verified that your patchset does not change 
current behaviour when building the entire set of default configurations 
for each supported architecture? Does it reduce or increase overall 
complexity of the build process?

> it is not even supposed to add overhead to kernel compilation. And it would,
> at least, solve my problem with the 3h limit, and enable the tool
> I am building on GiltabCI to help other developers.

(Apropos of nothing: Am I the only one who thinks gitlab might take a 
rather dim view of your creativity in dealing with their limit?)

> Thanks for reading,
>
> Leonardo Bras

Thanks for listening!

Cheers,

	Michael

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Finn Thain]

On Thu, 4 Oct 2018, Leonardo Bras wrote:

> ... 
> But, why these chosen Makefiles, and not all of them?

I think that inconsistency is untenable. If nothing else, it means your 
changes will get broken by other people who also apply constraints 
inconsistently.

I think you need to consider what happens when you apply this regime 
tree-wide. Either that or explain why it would make sense to build e.g. 
drivers/s390 but not drivers/s390/net.

-- 

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Leonardo Bras]

> Well it's not really that persuasive.  Most people simply let the build
> run to completion, but if you have a problem with a job control 3h
> timelimit, then create a job that kills itself at 2:59 and then
> resubmits itself.  That will produce a complete build in 3h chunks
> without any need to call sub Makefiles.
>

Humm, I probably should have explained better how GitlabCI works.
It works creating a docker container that have a limited lifespan of 3h max.
After that the time is over, this container ceases to exist, leaving no build
objects, only the console log. So there is no way of 'resuming' the building
from where it stopped. I used the 'job' term because it's how they call it,
and I understand it's easily confused with bash jobs.

> All of our Makefiles are coded assuming the upper level can prevent
> descent into the lower ones.  You're proposing to change that
> assumption, requiring a fairly large patch set, which doesn't really
> seem to provide a huge benefit.
>
> James

I understand your viewpoint.
But what I propose is not to change that assumption, but instead give some
Makefiles the aditional ability to be called directly and still not build stuff
if they were not enabled in .config.

But, why these chosen Makefiles, and not all of them?
Granularity.
What I am trying to achieve with this patchset is the ability of building
smaller sets of drivers without accidentally building what is not enabled
on .config.
And, in my viewpoint, building a single drivers/DRIVERNAME is small enough to
be fast in most situations.

This change is not supposed to bother the usual way of building the kernel, and
it is not even supposed to add overhead to kernel compilation. And it would,
at least, solve my problem with the 3h limit, and enable the tool
I am building on GiltabCI to help other developers.

Thanks for reading,

Leonardo Bras

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[James Bottomley]

On Wed, 2018-10-03 at 21:31 -0300, Leonardo Bras wrote:
> On Fri, Sep 28, 2018 at 4:15 AM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > 
> > On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> > > Avoids building driver if 'make drivers/parisc/' is called and
> > > CONFIG_PARISC is disabled.
> > 
> > Is that really a problem? The drivers/Makefile has this:
> > 
> > obj-$(CONFIG_PARISC)            += parisc/
> > And you just overrode that by forcing the build.  It's not even
> > clear we should refuse the build in that case; how would we know
> > you don't have a legitimate reason for the override?
> > 
> 
> Sorry I did not explained my reasons earlier. I sent everybody
> involved an e-mail explaining the full reason of this change.
> (For reference it's here: https://lkml.org/lkml/2018/10/3/707)

Well it's not really that persuasive.  Most people simply let the build
run to completion, but if you have a problem with a job control 3h
timelimit, then create a job that kills itself at 2:59 and then
resubmits itself.  That will produce a complete build in 3h chunks
without any need to call sub Makefiles.

All of our Makefiles are coded assuming the upper level can prevent
descent into the lower ones.  You're proposing to change that
assumption, requiring a fairly large patch set, which doesn't really
seem to provide a huge benefit.

James


> > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > > ---
> > >  drivers/parisc/Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> > > index 3cd5e6cb8478..80049d763aa0 100644
> > > --- a/drivers/parisc/Makefile
> > > +++ b/drivers/parisc/Makefile
> > > @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)          += eisa.o
> > > eisa_enumerator.o eisa_eeprom.o
> > >  obj-$(CONFIG_SUPERIO)                += superio.o
> > >  obj-$(CONFIG_CHASSIS_LCD_LED)        += led.o
> > >  obj-$(CONFIG_PDC_STABLE)     += pdc_stable.o
> > > -obj-y                                += power.o
> > > +obj-$(CONFIG_PARISC)         += power.o
> > 
> > If we conclude the use case is legitimate, that's not enough: the
> > two
> > inner symbols are PARISC only but CONFIG_EISA isn't.
> 
> You are right.
> It worked for my needs because I am only building the drivers, and
> not linking them. But i believe doing something like I did in
> zorro/Makefile would fix this all. (For reference,
> https://lkml.org/lkml/2018/9/28/150 )
> 
> If you agree, I will send the next patchset with this change.
> 
> Thanks for your help!
> 
> Leonardo Bras
> 

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Leonardo Bras]

On Fri, Sep 28, 2018 at 4:15 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> > Avoids building driver if 'make drivers/parisc/' is called and
> > CONFIG_PARISC is disabled.
>
> Is that really a problem? The drivers/Makefile has this:
>
> obj-$(CONFIG_PARISC)            += parisc/
> And you just overrode that by forcing the build.  It's not even clear
> we should refuse the build in that case; how would we know you don't
> have a legitimate reason for the override?
>

Sorry I did not explained my reasons earlier. I sent everybody involved an
e-mail explaining the full reason of this change.
(For reference it's here: https://lkml.org/lkml/2018/10/3/707)

> Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > ---
> >  drivers/parisc/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> > index 3cd5e6cb8478..80049d763aa0 100644
> > --- a/drivers/parisc/Makefile
> > +++ b/drivers/parisc/Makefile
> > @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)          += eisa.o
> > eisa_enumerator.o eisa_eeprom.o
> >  obj-$(CONFIG_SUPERIO)                += superio.o
> >  obj-$(CONFIG_CHASSIS_LCD_LED)        += led.o
> >  obj-$(CONFIG_PDC_STABLE)     += pdc_stable.o
> > -obj-y                                += power.o
> > +obj-$(CONFIG_PARISC)         += power.o
>
> If we conclude the use case is legitimate, that's not enough: the two
> inner symbols are PARISC only but CONFIG_EISA isn't.

You are right.
It worked for my needs because I am only building the drivers, and not
linking them. But i believe doing something like I did in zorro/Makefile
would fix this all. (For reference, https://lkml.org/lkml/2018/9/28/150 )

If you agree, I will send the next patchset with this change.

Thanks for your help!

Leonardo Bras

Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[James Bottomley]

On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> Avoids building driver if 'make drivers/parisc/' is called and
> CONFIG_PARISC is disabled.

Is that really a problem? The drivers/Makefile has this:

obj-$(CONFIG_PARISC)		+= parisc/ 
And you just overrode that by forcing the build.  It's not even clear
we should refuse the build in that case; how would we know you don't
have a legitimate reason for the override? 

Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> ---
>  drivers/parisc/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> index 3cd5e6cb8478..80049d763aa0 100644
> --- a/drivers/parisc/Makefile
> +++ b/drivers/parisc/Makefile
> @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)		+= eisa.o
> eisa_enumerator.o eisa_eeprom.o
>  obj-$(CONFIG_SUPERIO)		+= superio.o
>  obj-$(CONFIG_CHASSIS_LCD_LED)	+= led.o
>  obj-$(CONFIG_PDC_STABLE)	+= pdc_stable.o
> -obj-y				+= power.o
> +obj-$(CONFIG_PARISC)		+= power.o

If we conclude the use case is legitimate, that's not enough: the two
inner symbols are PARISC only but CONFIG_EISA isn't.

James

[PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

[Leonardo Brás]

Avoids building driver if 'make drivers/parisc/' is called and
CONFIG_PARISC is disabled.

Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
---
 drivers/parisc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
index 3cd5e6cb8478..80049d763aa0 100644
--- a/drivers/parisc/Makefile
+++ b/drivers/parisc/Makefile
@@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)		+= eisa.o eisa_enumerator.o eisa_eeprom.o
 obj-$(CONFIG_SUPERIO)		+= superio.o
 obj-$(CONFIG_CHASSIS_LCD_LED)	+= led.o
 obj-$(CONFIG_PDC_STABLE)	+= pdc_stable.o
-obj-y				+= power.o
+obj-$(CONFIG_PARISC)		+= power.o
 
-- 
2.19.0