| Message ID | 20180928020816.11251-4-leobras.c@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Remove errors building drivers/DRIVERNAME | expand |
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
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
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 >
> 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
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. --
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
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
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
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(-)