You rely on a timestamp check to tell the users "suggest you rebuild
firmware". That timestamp check is inherently unreliable when files
are both generated and shipped.
>>* Rebuilding the firmware requires lex, yacc and libdb. Not everybody
>> has these installed.
>
>Then they shouldn't check the box "rebuild firmware".
See above. Users think they need to turn on the firmware build, then
complain when it breaks.
>>* The check for which libdb to use assumes that the presence of a db.h
>> is enough, but the overlap between glibc-devel and dbx-devel packages
>> means that finding a db.h is not enough, you have to confirm that the
>> corresponding libdb exists.
>
>Such is Linux. Those who understand what it means to rebuild the
>firmware will have the necessary tools, check the box in config,
>and have it work.
Wrong. Such is the way it _used_ to be. As the use of Linux expands,
more and more people are building their own kernels without knowing all
the internals. This is good, we get more users. But kernel build code
can no longer assume that anybody building a kernel is automatically an
expert.
>>* It checks if the firmware is up to date by comparing the timestamps
>> on aic7xxx_seq.h and aic7xxx_reg.h against aic7xxx.seq and
>> aic7xxx.reg. Alas, when a patch hits those files there is no
>> guarantee which order the files are listed in the patch so the final
>> timestamp order is unreliable. diff lists files in alphabetical
>> order but other source repository systems can generate patches in any
>> order. This is a problem for all generated files, not just aic7xxx.
>
>So you might get a harmless warning if you haven't checked the box. This
>is not fatal and I have yet to hear one complaint about it.
http://marc.theaimsgroup.com/?l=linux-kernel&m=99124017310488&w=2
was fatal, you even replied to it.
>>* Shipping files which are also overwritten by users causes problems
>> for source control systems and can cause spurious differences when
>> generating patches. This is a problem for all generated files, not
>> just aic7xxx.
>
>Those using revision control should know how to use revision control.
>The driver is developed under revision control and the current setup
>causes me no grief. Of course, I don't keep the generated files in
>revision control because there is no benefit in doing so.
Users take patches from Linus or Alan Cox which include the generated
patches and add the patches to local source repositories. That
includes the generated files. If it comes from Linus or AC it is a
"master" copy. End users do not have the luxury of excluding the
generated files from revision control because it is not their input.
And if they do exclude the files then their users are forced to
generate the firmware. Excluding the aic7xxx generated files from
source revision works for you because you always generate the firmware,
it does not work for anybody else.
>For those
>that decide to keep the generated files in revision control, they
>should pull any update to the generated files from the vendor (they
>are always provided in my patches) and *never check the box*.
Users must not be forced to go hunting for files from a vendor when the
rst of the code is in the kernel. Especially when that vendor is not
listed in MAINTAINERS and there is no contact data in the aic7xxx
directory.
>>The patch below fixes all of the above issues. It does not touch the
>>aic7xxx code nor sequencer input, just the generated files and the
>>kbuild related files. The patch is approx 100Kb but most of it is the
>>rename of aic7xxx_{seq,reg}.h to aic7xxx_{seq,reg}.h_shipped.
>
>I don't see this as an improvement.
I do, and I am the kernel build maintainer. I don't tell you how to
code aic7xxx drivers, but I can and will fix kbuild problems. The
current aic7xxx kbuild is a problem.
>>After applying this patch, normal users will not have to worry about
>>generating aic7xxx firmware.
>
>This is already true today.
Not true, the timestamp check produces spurious prompts.
>>In particular they will not have to
>>select "rebuild firmware" nor will they need lex, yacc or libdb.
>
>Already true today.
Wrong. See
http://marc.theaimsgroup.com/?l=linux-kernel&m=99323170127833&w=2
>>Only people who change one of these files
>
>Today, this only applies to those that *check the rebuild firmware*
>box.
Which the broken timestamp check encourages people to do.
>What again are you trying to fix? It looks to me like you are simply
>trying to make it harder for people actually working on the aic7xxx
>driver to have proper dependencies.
The patch still works for anybody changing the aic7xxx firmware or the
aicasm code. Any change to the generated files or the aicasm files now
forces a rebuild, the option is not required. Only people changing
aic7xxx firmware are affected, instead of everybody.
Bottom line: the current method relies on unreliable timestamps,
produces spurious warning messages and causes problems for everybody
using source control except for you. The new method is clean. And as
kbuild maintainer, that is the way I want it to be done.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/