[U-Boot] [STATUS] "Quality" of patches / testing.
Hi all,
the patches that have been submitted for this release turn out to of of shockingly bad quality. About every other batch of patches I apply will break building not only for a single board r a few boards, but for large numbers of boards, including whole processor families and even whole architectures.
Not to mention the huge number of patches that have to be rejected because they raise tons or errors and warnings from checkpatch.pl
The time I have available is limited even without such avoidable problems, and I really, really rather spend it on constructive work instead of continuously running git bisect to track down the culprits.
What's even worse is that it appears that I am the only person in this whole community who runs any build tests. Or how comes that it's always me who detects such build breakages?
Note # 3 at http://www.denx.de/wiki/U-Boot/Patches says:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
I cannot continue like that. We need to find ways to improve the quality of the submitted patches, and to distribute the work load for testing.
I need your help.
Thanks.
Wolfgang Denk
Hi Wolfgang,
On 10/18/2011 08:23 AM, Wolfgang Denk wrote: [SNIP]
Why is nobody doing this?
One of my big problems with this was that you not only have to run a MAKEALL with your own changes but also have to do that with the former state of the repo to identifie the differences.
So one suggestion I have is to do at least a reference build of each custodian repo and store the output somewhere so one can diff with these.
[SNIP]
Regards Simon
Dear Simon,
Am 18.10.2011 08:51, schrieb Simon Schwarz:
Hi Wolfgang,
On 10/18/2011 08:23 AM, Wolfgang Denk wrote: [SNIP]
Why is nobody doing this?
One of my big problems with this was that you not only have to run a MAKEALL with your own changes but also have to do that with the former state of the repo to identifie the differences.
We should first get a state of "all boards build clean" for a sort of toolchains (I think arm is now at this state after a lot of tumult in the last two releases).
So one suggestion I have is to do at least a reference build of each custodian repo and store the output somewhere so one can diff with these.
This is some kind of CI as suggested by Lukasz Majewski. I also favor CI for fast feedback to the submitter if his change breaks something.
There was already a suggestion by Graeme Russ to run checkpatch on every submitted patch. While this requires really low computing power it will produce a lot of mail overhead. The second suggestion by Graeme was to add an marker into the patch which states this patch as checkpatch-clean. If we would build some CI system with automatic build we will need even more computing power to build each patch with all/a subset of MAKEALL. Not to mention the mail overhead for clean/broken patches. Therefor some marker in the patch could do the job here again.
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
best regards
Andreas Bießmann
Dear Andreas,
In message 4E9D4552.5040506@gmail.com you wrote:
We should first get a state of "all boards build clean" for a sort of toolchains (I think arm is now at this state after a lot of tumult in the last two releases).
PowerPC has traditionally always been build-clean (all boards succeed to build, with very few [2...3] causing harmelss build warnings). Now ARM is close to that, too. MIPS has also always been building mostly fine.
This is some kind of CI as suggested by Lukasz Majewski. I also favor CI for fast feedback to the submitter if his change breaks something.
I'm not sure how to do that. To provide feedback to individual submitters, you would have to runn a full build cycle for each patch we apply. That would obviously be best, but I don't have machine power to do that.
What I do is testing batches - say, after applying 20...30 patches, or after major changes.
I have Jenkins running every night for a few selected CPU families, which already catches a number of issues, but even then it's directly pointing to a submitted patch.
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
Best regards,
Wolfgang Denk
Wolfgang,
On Tue, Oct 18, 2011 at 11:44:37AM +0200, Wolfgang Denk wrote:
Dear Andreas,
In message 4E9D4552.5040506@gmail.com you wrote:
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
I've been mulling this over, and here's my approach:
1.) modify git to add a hook in format-patch, output is dumped after the '---' and before the diff. We use this the run checkpatch.pl with our config. Our script includes a version tag of checkpatch.pl?
2.) Add a '--versioning' option to format-patch which will scan the output directory for previous versions of the patch. a.) Use the Message-Id of the first version as an In-Reply-To b.) Migrate patch changelog over from previous version, append '*** ADD CHANGELOG ENTRY HERE ***' c.) enforce [PATCH 1/7 V#] Subject line format.
I think this would be more flexible, and would help many projects, not just u-boot.
thx,
Jason.
Of course, I had another idea _after_ I hit send...
On Tue, Oct 18, 2011 at 09:05:07AM -0400, Jason wrote:
1.) modify git to add a hook in format-patch, output is dumped after the '---' and before the diff. We use this the run checkpatch.pl with our config. Our script includes a version tag of checkpatch.pl?
Add the ability to run a series of scripts, with the output appended as above. So,
00_checkpatch 10_makeall
Where 10_makeall would provide some concise output, eg (0 errors, 3 warnings) or similar.
thx,
Jason.
On 10/18/2011 03:05 PM, Jason wrote:
Wolfgang,
On Tue, Oct 18, 2011 at 11:44:37AM +0200, Wolfgang Denk wrote:
Dear Andreas,
In message4E9D4552.5040506@gmail.com you wrote:
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
I've been mulling this over, and here's my approach:
1.) modify git to add a hook in format-patch, output is dumped after the '---' and before the diff. We use this the run checkpatch.pl with our config. Our script includes a version tag of checkpatch.pl?
cool!
2.) Add a '--versioning' option to format-patch which will scan the output directory for previous versions of the patch. a.) Use the Message-Id of the first version as an In-Reply-To
- Where do you get the Message-Id from? Isn't the message-id assigned by the mail system? - I would prefer the last version
b.) Migrate patch changelog over from previous version, append '*** ADD CHANGELOG ENTRY HERE ***' c.) enforce [PATCH 1/7 V#] Subject line format.
Don't forget to remove "V#" for the first version.
I think this would be more flexible, and would help many projects, not just u-boot.
Totally agreed!
thx,
Jason.
Regards Simon
On Tue, Oct 18, 2011 at 03:13:45PM +0200, Simon Schwarz wrote:
On 10/18/2011 03:05 PM, Jason wrote:
2.) Add a '--versioning' option to format-patch which will scan the output directory for previous versions of the patch. a.) Use the Message-Id of the first version as an In-Reply-To
- Where do you get the Message-Id from? Isn't the message-id assigned
by the mail system?
'git format-patch' can create it, or 'git send-mail' will add it. Nothing prevents there from being more than one.
- I would prefer the last version
I suppose an example in threaded view might help (yes, not a practical series, but bear with me :-) ):
[PATCH 0/3] add xyz support. [PATCH 1/3] fix some junk [PATCH 2/3] add support for xyz board [PATCH 3/3] add xyz board to MAKEALL [PATCH 0/3 V2] add xyz support. [PATCH 1/3 V2] whitespace cleanup [PATCH 2/3 V2] add support for xyz board [PATCH 3/3 V2] add xyz board to MAKEALL [PATCH 0/2 V3] add xyz support. [PATCH 1/2 V3] whitespace cleanup [PATCH 2/2 V3] add support for xyz board [PATCH 1/1 V4] add xyz support. [PATCH 1/1 V5] add xyz support.
And the cooresponding changelog:
Changes from v1 to v2: - proper subject line for whitespace cleanup
Changes from v2 to v3: - collapse patch 3 into 2 advised by Prafulla
Changes from v3 to v4: - whitespace cleanup was merged
Changes from v4 to v5: - rebased against new tag, v2011.09
By always being In-Reply-To the original coverletter, it's imho, much easier to find the next version. Especially once comments and replies are in the thread. I suppose '--thread=shallow' could trigger my approach...
b.) Migrate patch changelog over from previous version, append '*** ADD CHANGELOG ENTRY HERE ***' c.) enforce [PATCH 1/7 V#] Subject line format.
Don't forget to remove "V#" for the first version.
Yes, see above.
thx,
Jason.
Let's try that example again...
[PATCH 0/3] add xyz support. |-[PATCH 1/3] fix some junk |-[PATCH 2/3] add support for xyz board |-[PATCH 3/3] add xyz board to MAKEALL |-[PATCH 0/3 V2] add xyz support. | |-[PATCH 1/3 V2] whitespace cleanup | |-[PATCH 2/3 V2] add support for xyz board | -[PATCH 3/3 V2] add xyz board to MAKEALL |-[PATCH 0/2 V3] add xyz support. | |-[PATCH 1/2 V3] whitespace cleanup | -[PATCH 2/2 V3] add support for xyz board |-[PATCH 1/1 V4] add xyz support. -[PATCH 1/1 V5] add xyz support.
Hopefully, that didn't get munged.
thx,
Jason.
On Tuesday 18 October 2011 09:13:45 Simon Schwarz wrote:
On 10/18/2011 03:05 PM, Jason wrote:
a.) Use the Message-Id of the first version as an In-Reply-To
- Where do you get the Message-Id from? Isn't the message-id assigned by
the mail system?
fairly certain the MTA never does that. it's on the client's head (e.g. git- send-email) to add message-id tags. -mike
Dear Jason,
Am 18.10.2011 15:05, schrieb Jason:
Wolfgang,
On Tue, Oct 18, 2011 at 11:44:37AM +0200, Wolfgang Denk wrote:
Dear Andreas,
In message 4E9D4552.5040506@gmail.com you wrote:
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
I've been mulling this over, and here's my approach:
1.) modify git to add a hook in format-patch, output is dumped after the '---' and before the diff. We use this the run checkpatch.pl with our config. Our script includes a version tag of checkpatch.pl?
I think of some script that utilizes git rather than changing git.
2.) Add a '--versioning' option to format-patch which will scan the output directory for previous versions of the patch.
nice, but ...
a.) Use the Message-Id of the first version as an In-Reply-To b.) Migrate patch changelog over from previous version, append '*** ADD CHANGELOG ENTRY HERE ***' c.) enforce [PATCH 1/7 V#] Subject line format.
I never hold old patches long, they are on the list, patchwork or in my tree. But feel free to discuss this on git ML. And I doubt this will help us in near future (remember the debian users ;)
Best regards
Andreas Bießmann
Andreas,
On Tue, Oct 18, 2011 at 03:36:21PM +0200, Andreas Bießmann wrote:
Dear Jason,
Am 18.10.2011 15:05, schrieb Jason:
...
I've been mulling this over, and here's my approach:
1.) modify git to add a hook in format-patch, output is dumped after the '---' and before the diff. We use this the run checkpatch.pl with our config. Our script includes a version tag of checkpatch.pl?
I think of some script that utilizes git rather than changing git.
As long as the end effect puts all of the versions of a patch series into one thread, and injects the output of checkpatch, a changelog and possibly the output of MAKEALL; then I'm all for it.
I'd prefer to create a generic solution so that other projects could take advantage of it and adapt it to their needs.
2.) Add a '--versioning' option to format-patch which will scan the output directory for previous versions of the patch.
nice, but ...
a.) Use the Message-Id of the first version as an In-Reply-To b.) Migrate patch changelog over from previous version, append '*** ADD CHANGELOG ENTRY HERE ***' c.) enforce [PATCH 1/7 V#] Subject line format.
I never hold old patches long, they are on the list, patchwork or in my tree.
Hmmm, if I can get a better handle on rfc 2822 [1], particularly section 3.6.4, then the threading would be maintained even when the old messages are deleted from your mail queue.
But feel free to discuss this on git ML. And I doubt this will help us in near future (remember the debian users ;)
Possibly, there's no reason why the wrapper script and the git hook can't both be pursued. Looks like Simon Glass already has a good start on the wrapper script.
thx,
Jason.
Hi,
On Tue, Oct 18, 2011 at 2:44 AM, Wolfgang Denk wd@denx.de wrote:
Dear Andreas,
In message 4E9D4552.5040506@gmail.com you wrote:
We should first get a state of "all boards build clean" for a sort of toolchains (I think arm is now at this state after a lot of tumult in the last two releases).
PowerPC has traditionally always been build-clean (all boards succeed to build, with very few [2...3] causing harmelss build warnings). Now ARM is close to that, too. MIPS has also always been building mostly fine.
This is some kind of CI as suggested by Lukasz Majewski. I also favor CI for fast feedback to the submitter if his change breaks something.
I'm not sure how to do that. To provide feedback to individual submitters, you would have to runn a full build cycle for each patch we apply. That would obviously be best, but I don't have machine power to do that.
What I do is testing batches - say, after applying 20...30 patches, or after major changes.
I have Jenkins running every night for a few selected CPU families, which already catches a number of issues, but even then it's directly pointing to a submitted patch.
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
I'm keen to take a look at this, as I have a and b already and a tool which munges patches. Also it something that causes me a bit of pain.
I have so far written a tool which automates patch creation based on tags in the commits - it works out how many patches are in your branch (or you can tell it), creates the patches with format-patch, runs patches through checkpatch, its own checks and does a 'git am' on each. Then it creates a cover letter and (if the patches are clean) optionally does a 'git send-email' on the series. Information is collected from tags in the commits (which are removed from the patch). Version changes are collected from each commit and added to the cover letter, the subject is set correctly, etc.
For example, here is my top commit for branch us-printf:
Series-to: u-boot Series-cc: wolfgang Cover-letter: Buffer overruns in printf The printf family of functions in U-Boot cannot deal with a situation where the caller provides a buffer which turns out to be too small for the format string. This can result in buffer overflows, stack overflows and other bad behavior.
This patch series tidies this up in the common vsprintf.c code.
END
Series-version: 3
Series-changes: 2 - Use sizeof(printbuffer) instead of CONFIG_SYS_PBSIZE - Drop patch which changes network code to use snprintf()
and another commit has:
Series-changes: 3 - Move prototypes from common.h to vsprintf.h
Other tags are Series-prefix (for 'RFC' for example) and Series-notes for notes which should appear at the end of the cover letter. It also looks for tags like 'arm:' and 'net:' in the in the subject field and CCs the maintainer.
It does enforce a certain workflow, but allows you to easily change branches, make a change and create some new patches. It does not handle In-reply-to yet.
Anyway, with this and Mike's little script for calling MAKEALL it doesn't seem like a huge job to add building to this script. and then append the results into the patch.
I have already been wondering how to send this out for review - perhaps as something in tools/scripts?
Regads, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Drun'? 'm not drun'! You woudn' dare call m' drun' if I was sober! - Terry Pratchett, _Men at Arms_ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tue, Oct 18, 2011 at 7:05 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, Oct 18, 2011 at 2:44 AM, Wolfgang Denk wd@denx.de wrote:
Dear Andreas,
In message 4E9D4552.5040506@gmail.com you wrote:
We should first get a state of "all boards build clean" for a sort of toolchains (I think arm is now at this state after a lot of tumult in the last two releases).
PowerPC has traditionally always been build-clean (all boards succeed to build, with very few [2...3] causing harmelss build warnings). Now ARM is close to that, too. MIPS has also always been building mostly fine.
This is some kind of CI as suggested by Lukasz Majewski. I also favor CI for fast feedback to the submitter if his change breaks something.
I'm not sure how to do that. To provide feedback to individual submitters, you would have to runn a full build cycle for each patch we apply. That would obviously be best, but I don't have machine power to do that.
What I do is testing batches - say, after applying 20...30 patches, or after major changes.
I have Jenkins running every night for a few selected CPU families, which already catches a number of issues, but even then it's directly pointing to a submitted patch.
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch c) (configurable subset of) MAKEALL on some clean tree with that patch applied d) append the results to the patch
This tool would implement some kind of CI but utilize the computing power of submitter.
Sounds good. Any takers?
I'm keen to take a look at this, as I have a and b already and a tool which munges patches. Also it something that causes me a bit of pain.
I have so far written a tool which automates patch creation based on tags in the commits - it works out how many patches are in your branch (or you can tell it), creates the patches with format-patch, runs patches through checkpatch, its own checks and does a 'git am' on each. Then it creates a cover letter and (if the patches are clean) optionally does a 'git send-email' on the series. Information is collected from tags in the commits (which are removed from the patch). Version changes are collected from each commit and added to the cover letter, the subject is set correctly, etc.
For example, here is my top commit for branch us-printf:
Series-to: u-boot Series-cc: wolfgang Cover-letter: Buffer overruns in printf The printf family of functions in U-Boot cannot deal with a situation where the caller provides a buffer which turns out to be too small for the format string. This can result in buffer overflows, stack overflows and other bad behavior.
This patch series tidies this up in the common vsprintf.c code.
END
Series-version: 3
Series-changes: 2 - Use sizeof(printbuffer) instead of CONFIG_SYS_PBSIZE - Drop patch which changes network code to use snprintf()
and another commit has:
Series-changes: 3 - Move prototypes from common.h to vsprintf.h
Other tags are Series-prefix (for 'RFC' for example) and Series-notes for notes which should appear at the end of the cover letter. It also looks for tags like 'arm:' and 'net:' in the in the subject field and CCs the maintainer.
It does enforce a certain workflow, but allows you to easily change branches, make a change and create some new patches. It does not handle In-reply-to yet.
Anyway, with this and Mike's little script for calling MAKEALL it doesn't seem like a huge job to add building to this script. and then append the results into the patch.
I have already been wondering how to send this out for review - perhaps as something in tools/scripts?
Regads, Simon
Amusingly (since Simon and I work a few feet apart from each other), I have something similar that I've written and am using to automate generation of patches and running of checkpatch. I'll sync up with him today and see if it makes sense to merge our efforts and post the result.
-Anton
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Drun'? 'm not drun'! You woudn' dare call m' drun' if I was sober! - Terry Pratchett, _Men at Arms_ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Simon Glass,
In message CAPnjgZ1SniUd1pBgfyX4apA62inC3k9FFmKuG-opzwFJDY=Gcg@mail.gmail.com you wrote:
I have already been wondering how to send this out for review - perhaps as something in tools/scripts?
Yes, please.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tue, Oct 18, 2011 at 1:23 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message CAPnjgZ1SniUd1pBgfyX4apA62inC3k9FFmKuG-opzwFJDY=Gcg@mail.gmail.com you wrote:
I have already been wondering how to send this out for review - perhaps as something in tools/scripts?
Yes, please.
OK I have done this. It's a little rough but at least a starting point. I hope you really like Python because there is plenty of it :-)
No MAKEALL integration yet - will watch and see how that discussion comes along. That would be *really* nice.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "Unix is simple, but it takes a genius to understand the simplicity." - Dennis Ritchie
Dear Simon Glass,
In message CAPnjgZ2TB6UCXgqkekBkJGxiscxLQcA1trrQqO2UunRLeSv6uw@mail.gmail.com you wrote:
OK I have done this. It's a little rough but at least a starting
Thanks!
point. I hope you really like Python because there is plenty of it :-)
Actually I don't (didn't really learn it yet), but then I don;t have to if I can just use the stuff :-)
No MAKEALL integration yet - will watch and see how that discussion comes along. That would be *really* nice.
I really appreciate all these activities. I have to admit that I didn't expect such a lot of positive, constructive responses.
Best regards,
Wolfgang Denk
Hi,
I guess it will be doable to have some scripts/prepare-patch which runs a) git format-patch b) checkpatch on the patch
I'm a bit confused.
Joe Hershberger has prepared a following patch:
http://patchwork.ozlabs.org/patch/119083/
Is this THE ONE checkpatch version which we shall use (including its config file)?
I've used it for my patch sets and I can say that warnings like:
WARNING: min() should probably be min_t(unsigned int, amount_left, mod_data.buflen)
and
WARNING: consider using kstrto* in preference to simple_strtoul #4358:
are still present.
Those are Linux kernel specific.
Shall I not care about them and submit patches with non zero warnings output from checkpatch?
Dear Lukasz Majewski,
In message 20111018122416.483205d6@lmajewski.digital.local you wrote:
Joe Hershberger has prepared a following patch:
http://patchwork.ozlabs.org/patch/119083/
Is this THE ONE checkpatch version which we shall use (including its config file)?
It shall become it, yes.
I've used it for my patch sets and I can say that warnings like:
WARNING: min() should probably be min_t(unsigned int, amount_left, mod_data.buflen)
and
WARNING: consider using kstrto* in preference to simple_strtoul #4358:
are still present.
Those are Linux kernel specific.
Shall I not care about them and submit patches with non zero warnings output from checkpatch?
Neither one nore the other. Help adapting the checkpatch config file to our needs, so that things like simple_strtoul() don't throw such warnings in U-Boot context.
Then submit zero warning patches...
Best regards,
Wolfgang Denk
Dear Simon Schwarz,
In message 4E9D21F8.40100@gmail.com you wrote:
One of my big problems with this was that you not only have to run a MAKEALL with your own changes but also have to do that with the former state of the repo to identifie the differences.
So one suggestion I have is to do at least a reference build of each custodian repo and store the output somewhere so one can diff with these.
That would be a task for all custodians, then?
Best regards,
Wolfgang Denk
On 10/18/2011 11:34 AM, Wolfgang Denk wrote:
Dear Simon Schwarz,
In message4E9D21F8.40100@gmail.com you wrote:
One of my big problems with this was that you not only have to run a MAKEALL with your own changes but also have to do that with the former state of the repo to identifie the differences.
So one suggestion I have is to do at least a reference build of each custodian repo and store the output somewhere so one can diff with these.
That would be a task for all custodians, then?
Yes. Or a script which does this automatically.
Best regards,
Wolfgang Denk
Regards Simon
On Tue, 18 Oct 2011 08:23:10 +0200 Wolfgang Denk wd@denx.de wrote:
I need your help.
I'd propose to sent result from a night build (for a respective architecture/the whole u-boot) to the u-boot mailing list.
Then, in the morning (well, depends on the part of the world) a custodian or culprit of the error would know that his stuff needs to be fixed.
On Tuesday 18 October 2011 02:23:10 Wolfgang Denk wrote:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently. -mike
Hi,
On Tue, Oct 18, 2011 at 10:01 AM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 18 October 2011 02:23:10 Wolfgang Denk wrote:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
Yes I think this is along the right track. I tried MAKEALL a long time ago and it didn't work (just got an error about incorrect architecture) so I assumed it was broken. Apart from full docs, it would be good to have a place to download all the toolchains needed for MAKEALL.
Regards, Simon
-mike _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tue, Oct 18, 2011 at 10:01 AM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 18 October 2011 02:23:10 Wolfgang Denk wrote:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
If we're going to throw out feature requests, I'd like more than one SoC family support. I've got a wrapper around MAKEALL right now to build omap3 then omap4 then davinci.
On Tuesday 18 October 2011 13:58:22 Tom Rini wrote:
On Tue, Oct 18, 2011 at 10:01 AM, Mike Frysinger wrote:
On Tuesday 18 October 2011 02:23:10 Wolfgang Denk wrote:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch. It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run: $ BUILD_DIR=/tmp/u-boot-build ./MAKEALLWhy is nobody doing this?
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
If we're going to throw out feature requests, I'd like more than one SoC family support. I've got a wrapper around MAKEALL right now to build omap3 then omap4 then davinci.
well, i think yours has a much easier chance of being implemented. and should be trivial to do with boards_by_soc. although i think i have a better idea. i'll post a patch. -mike
On Tuesday 18 October 2011 13:58:22 Tom Rini wrote:
If we're going to throw out feature requests, I'd like more than one SoC family support. I've got a wrapper around MAKEALL right now to build omap3 then omap4 then davinci.
hmm, actually doesn't the newish -s flag do what you want ? ./MAKEALL -s omap3 arm -mike
On Tue, Oct 18, 2011 at 11:31 AM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 18 October 2011 13:58:22 Tom Rini wrote:
If we're going to throw out feature requests, I'd like more than one SoC family support. I've got a wrapper around MAKEALL right now to build omap3 then omap4 then davinci.
hmm, actually doesn't the newish -s flag do what you want ? ./MAKEALL -s omap3 arm
So, -s omap3 -s omap4 looks like it does what I want, -s omap3,omap4 (what I first guessed) didn't. So just another vote for more / better docs that I of course should help to provide. Maybe it's time for at least a doc/README.MAKEALL?
On Tuesday 18 October 2011 14:54:09 Tom Rini wrote:
On Tue, Oct 18, 2011 at 11:31 AM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 18 October 2011 13:58:22 Tom Rini wrote:
If we're going to throw out feature requests, I'd like more than one SoC family support. I've got a wrapper around MAKEALL right now to build omap3 then omap4 then davinci.
hmm, actually doesn't the newish -s flag do what you want ? ./MAKEALL -s omap3 arm
So, -s omap3 -s omap4 looks like it does what I want, -s omap3,omap4 (what I first guessed) didn't. So just another vote for more / better docs that I of course should help to provide. Maybe it's time for at least a doc/README.MAKEALL?
./MAKEALL -h -mike
Dear Mike Frysinger,
In message 201110181301.57390.vapier@gentoo.org you wrote:
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
How is this supposed to work? Assume I have a number of different tool chains, say I want to use the tool chain in /opt/eldk-5.1/armv5te for all ARM9 systems, that in /opt/eldk-5.1/armv7a for all OMAP based boards, that in /opt/eldk-5.1/armv6 for Kirkwood processors and yet another one for the (bix endian) PXA boards. In all cases we have ARCH=arm and CROSS_COMPILE=arm-linux-gnueabi-
And then, for compatibility testings, I want to compile all this with ELDK 4.2. Or ELDK 4.1. Or CodeSourcery xxx. Or...
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
Best regards,
Wolfgang Denk
On Tuesday 18 October 2011 16:07:38 Wolfgang Denk wrote:
Mike Frysinger wrote:
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
How is this supposed to work? Assume I have a number of different tool chains, say I want to use the tool chain in /opt/eldk-5.1/armv5te for all ARM9 systems, that in /opt/eldk-5.1/armv7a for all OMAP based boards, that in /opt/eldk-5.1/armv6 for Kirkwood processors and yet another one for the (bix endian) PXA boards. In all cases we have ARCH=arm and CROSS_COMPILE=arm-linux-gnueabi-
And then, for compatibility testings, I want to compile all this with ELDK 4.2. Or ELDK 4.1. Or CodeSourcery xxx. Or...
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
my proposal is only for the default behavior, and it only searches $PATH. if the auto-lookup isn't what the user wants, they still can set CROSS_COMPILE= themselves. so all existing usage is unchanged.
to add a further bit of flexibility, i might also propose that MAKEALL check the variable CROSS_COMPILE_<arch> and automatically set CROSS_COMPILE to that before running `make`. this way people can do CROSS_COMPILE_arm=... CROSS_COMPILE_powerpc=... ./MAKEALL arm powerpc. -mike
Dear Mike Frysinger,
In message 201110181614.46289.vapier@gentoo.org you wrote:
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
my proposal is only for the default behavior, and it only searches $PATH. if the auto-lookup isn't what the user wants, they still can set CROSS_COMPILE= themselves. so all existing usage is unchanged.
While we are at it I would like to fix the known (to me) problems of the current usage - that is for example that ARCH=arm includes for example both little and big endian systems, which usually require different tool chains to be used.
to add a further bit of flexibility, i might also propose that MAKEALL check the variable CROSS_COMPILE_<arch> and automatically set CROSS_COMPILE to that before running `make`. this way people can do CROSS_COMPILE_arm=... CROSS_COMPILE_powerpc=... ./MAKEALL arm powerpc.
That would still be to coarse for above issue. Also, you might want to use different ARM tool chains for ARMv5te systemd than for ARMv6 and yet other ones for ARMv7a, etc.
Best regards,
Wolfgang Denk
On Tuesday 18 October 2011 16:47:12 Wolfgang Denk wrote:
Mike Frysinger wrote:
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
my proposal is only for the default behavior, and it only searches $PATH. if the auto-lookup isn't what the user wants, they still can set CROSS_COMPILE= themselves. so all existing usage is unchanged.
While we are at it I would like to fix the known (to me) problems of the current usage - that is for example that ARCH=arm includes for example both little and big endian systems, which usually require different tool chains to be used.
at least from code sorcery (who have been defacto arm providers), their single toolchain includes support for both endians in one package. the right output/libraries are selected with -m{big,little}-endian.
in terms of compiling all the arm in a single run, i haven't had a problem with my one toolchain (which defaults to little endian). but i've been using the private libgcc due to the many issues (including this) that comes with trying to use the one provided by the toolchain.
i see that some targets do add -EB/-EL/-mbig-endian to their compiler flags in the respective config.mk files ...
to add a further bit of flexibility, i might also propose that MAKEALL check the variable CROSS_COMPILE_<arch> and automatically set CROSS_COMPILE to that before running `make`. this way people can do CROSS_COMPILE_arm=... CROSS_COMPILE_powerpc=... ./MAKEALL arm powerpc.
That would still be to coarse for above issue. Also, you might want to use different ARM tool chains for ARMv5te systemd than for ARMv6 and yet other ones for ARMv7a, etc.
the idea is easy to extend to CROSS_COMPILE_<soc|cpu> and perhaps even CROSS_COMPILE_<vendor|board>
if you're not against the concept, i can post a patch and we can go from there. but i can say that the limited MAKEALL behavior is the single reason for my limited build testing in the past. i wrote the buildall script after trying to do tree-wide changes in the last few months because running ./MAKEALL simply does not scale. -mike
Hi,
On Tue, Oct 18, 2011 at 1:55 PM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 18 October 2011 16:47:12 Wolfgang Denk wrote:
Mike Frysinger wrote:
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
my proposal is only for the default behavior, and it only searches $PATH. if the auto-lookup isn't what the user wants, they still can set CROSS_COMPILE= themselves. so all existing usage is unchanged.
While we are at it I would like to fix the known (to me) problems of the current usage - that is for example that ARCH=arm includes for example both little and big endian systems, which usually require different tool chains to be used.
at least from code sorcery (who have been defacto arm providers), their single toolchain includes support for both endians in one package. the right output/libraries are selected with -m{big,little}-endian.
in terms of compiling all the arm in a single run, i haven't had a problem with my one toolchain (which defaults to little endian). but i've been using the private libgcc due to the many issues (including this) that comes with trying to use the one provided by the toolchain.
i see that some targets do add -EB/-EL/-mbig-endian to their compiler flags in the respective config.mk files ...
to add a further bit of flexibility, i might also propose that MAKEALL check the variable CROSS_COMPILE_<arch> and automatically set CROSS_COMPILE to that before running `make`. this way people can do CROSS_COMPILE_arm=... CROSS_COMPILE_powerpc=... ./MAKEALL arm powerpc.
That would still be to coarse for above issue. Also, you might want to use different ARM tool chains for ARMv5te systemd than for ARMv6 and yet other ones for ARMv7a, etc.
the idea is easy to extend to CROSS_COMPILE_<soc|cpu> and perhaps even CROSS_COMPILE_<vendor|board>
if you're not against the concept, i can post a patch and we can go from there. but i can say that the limited MAKEALL behavior is the single reason for my limited build testing in the past. i wrote the buildall script after trying to do tree-wide changes in the last few months because running ./MAKEALL simply does not scale. -mike
With Mike's script I was able to get something running.
Boards compiled: 247 Boards with warnings or errors: 175
ohci-hcd.c: In function 'submit_control_msg': ohci-hcd.c:1300: warning: dereferencing pointer 'data_buf.80' does break strict-aliasing rules ohci-hcd.c:1300: note: initialized from here ohci-hcd.c:1303: warning: dereferencing pointer 'data_buf.80' does break strict-aliasing rules ohci-hcd.c:1303: note: initialized from here ohci-hcd.c:1306: warning: dereferencing pointer 'data_buf.80' does break strict-aliasing rules ohci-hcd.c:1306: note: initialized from here
These seem to be fixed by Marek's commit in usb:
commit 3563664f6f5799cad08127f6fe3a63e64bfe2715 Author: Marek Vasut marek.vasut@gmail.com Date: Fri Oct 7 02:00:13 2011 +0200
USB: Fix complaints about strict aliasing in OHCI-HCD
arm-none-linux-gnueabi-ld: stubs.o: compiled for a big endian system and target is little endian arm-none-linux-gnueabi-ld: failed to merge target specific data of file stubs.o
I guess this is the endian error is the one you talk about here which I suppose can be fixed with a -m flag. How should this be added in? Should the ARM arch have a -mlittle-endian default?
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tuesday 18 October 2011 17:30:30 Simon Glass wrote:
arm-none-linux-gnueabi-ld: stubs.o: compiled for a big endian system and target is little endian arm-none-linux-gnueabi-ld: failed to merge target specific data of file stubs.o
I guess this is the endian error is the one you talk about here which I suppose can be fixed with a -m flag. How should this be added in? Should the ARM arch have a -mlittle-endian default?
i fear that would break some boards where the maintainer has always used a big endian toolchain. i don't think there are any CONFIG_xxx knobs for boards to say "i am big endian" or "i am little endian". -mike
Le 19/10/2011 00:21, Mike Frysinger a écrit :
On Tuesday 18 October 2011 17:30:30 Simon Glass wrote:
arm-none-linux-gnueabi-ld: stubs.o: compiled for a big endian system and target is little endian arm-none-linux-gnueabi-ld: failed to merge target specific data of file stubs.o
I guess this is the endian error is the one you talk about here which I suppose can be fixed with a -m flag. How should this be added in? Should the ARM arch have a -mlittle-endian default?
i fear that would break some boards where the maintainer has always used a big endian toolchain. i don't think there are any CONFIG_xxx knobs for boards to say "i am big endian" or "i am little endian". -mike
Actually the issue is not with the compiler -- it does build big-endian (example taken with scpu):
arm-linux-gcc -g -Os -fno-common -ffixed-r8 -msoft-float -mbig-endian -ffunction-sections -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0x50000000 -I/home/uboot/src/u-boot-arm/include -fno-builtin -ffreestanding -nostdinc -isystem /home/uboot/eldk42/usr/bin/../lib/gcc/arm-linux-gnueabi/4.2.2/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -mbig-endian -march=armv5te -mtune=strongarm1100 -Wall -Wstrict-prototypes -fno-stack-protector -Wno-format-nonliteral -Wno-format-security -fno-toplevel-reorder -o stubs.o stubs.c -c arm-linux-ld -r -o libstubs.o stubs.o
Notice the -mbig-endian compiler switch.
The issue is with the linker:
arm-linux-ld -r -o libstubs.o stubs.o
This fails because the linker does not specify -EB and thus links in little, not big, endian.
Note that further issues may prevent a big-endian build such as the absence of big-endian run-time libs.
Amicalement,
On Wednesday 19 October 2011 07:36:15 Albert ARIBAUD wrote:
Le 19/10/2011 00:21, Mike Frysinger a écrit :
On Tuesday 18 October 2011 17:30:30 Simon Glass wrote:
arm-none-linux-gnueabi-ld: stubs.o: compiled for a big endian system and target is little endian arm-none-linux-gnueabi-ld: failed to merge target specific data of file stubs.o
I guess this is the endian error is the one you talk about here which I suppose can be fixed with a -m flag. How should this be added in? Should the ARM arch have a -mlittle-endian default?
i fear that would break some boards where the maintainer has always used a big endian toolchain. i don't think there are any CONFIG_xxx knobs for boards to say "i am big endian" or "i am little endian".
Actually the issue is not with the compiler -- it does build big-endian (example taken with scpu):
that's because the target you picked has a config.mk which forces -mbig-endian in arch/arm/cpu/ixp/config.mk. that is the only arm soc that does. i'm pretty sure many of the other arm soc's support either big or little endian and it's just a matter of what the board wants to do.
The issue is with the linker:
arm-linux-ld -r -o libstubs.o stubs.o
This fails because the linker does not specify -EB and thus links in little, not big, endian.
that's a failing on the part of ixp/config.mk. it should be adding -EB to the linker flags when it forces -mbig-endian.
Note that further issues may prevent a big-endian build such as the absence of big-endian run-time libs.
there is no run-time libs. only libgcc.a is taken external of u-boot, and that issue is resolved by using the private libgcc. -mike
Dear Mike Frysinger,
In message 201110191025.02227.vapier@gentoo.org you wrote:
there is no run-time libs. only libgcc.a is taken external of u-boot, and that issue is resolved by using the private libgcc.
No. This is not a solution, it is only a workaround for what I call a broken tool chain.
If the compiler builds BE code, it must also provie the needed BE libgcc functions.
Best regards,
Wolfgang Denk
Dear Mike Frysinger,
In message 201110181655.29507.vapier@gentoo.org you wrote:
That would still be to coarse for above issue. Also, you might want to use different ARM tool chains for ARMv5te systemd than for ARMv6 and yet other ones for ARMv7a, etc.
the idea is easy to extend to CROSS_COMPILE_<soc|cpu> and perhaps even CROSS_COMPILE_<vendor|board>
The problem is that this does not differentiate the tool chains.
CROSS_COMPILE would be arm-linux-gnueabi- in all these cases - it's PATH that needs to be different in my case. Eventually we should not try to catch all situations directly, but allow for a soc|cpu|vendor|board|whatever specific file name to be sourced by the script, which then would be responsible to set up the environment as needed?
if you're not against the concept, i can post a patch and we can go from there. but i can say that the limited MAKEALL behavior is the single reason for my limited build testing in the past. i wrote the buildall script after trying to do tree-wide changes in the last few months because running ./MAKEALL simply does not scale.
I'm all for improving the current situation. Thanks!
Best regards,
Wolfgang Denk
On Tuesday 18 October 2011 17:50:52 Wolfgang Denk wrote:
Mike Frysinger wrote:
That would still be to coarse for above issue. Also, you might want to use different ARM tool chains for ARMv5te systemd than for ARMv6 and yet other ones for ARMv7a, etc.
the idea is easy to extend to CROSS_COMPILE_<soc|cpu> and perhaps even CROSS_COMPILE_<vendor|board>
The problem is that this does not differentiate the tool chains.
CROSS_COMPILE would be arm-linux-gnueabi- in all these cases - it's PATH that needs to be different in my case.
or a full path to the toolchain base: CROSS_COMPILE=/path/to/foo/arm-linux-gnueabi-
Eventually we should not try to catch all situations directly, but allow for a soc|cpu|vendor|board|whatever specific file name to be sourced by the script, which then would be responsible to set up the environment as needed?
toolchains tend to be site-specific. i use compilers i created myself, or found via kernel.org, or i've gotten directly from SoC vendors. all of which have different names. so while i think it'd be fine for maintainers to specify a preference, it would merely be that. -mike
Hi Wolfgang,
On Wed, Oct 19, 2011 at 7:07 AM, Wolfgang Denk wd@denx.de wrote:
Dear Mike Frysinger,
In message 201110181301.57390.vapier@gentoo.org you wrote:
because MAKEALL is a pita to use. it has no automatic CROSS_COMPILE support, and the current logic only allows one-CROSS_COMPILE-setting-per-run. so you have to run MAKEALL by hand once per arch.
the documentation you quote only shows running MAKEALL for powerpc (since that's the default), so even the docs are a bit unclear.
ideally, MAKEALL should be intelligent and automatically find an appropriate toolchain if one isn't setup in the env. much like the buildall script i posted recently.
How is this supposed to work? Assume I have a number of different tool chains, say I want to use the tool chain in /opt/eldk-5.1/armv5te for all ARM9 systems, that in /opt/eldk-5.1/armv7a for all OMAP based boards, that in /opt/eldk-5.1/armv6 for Kirkwood processors and yet another one for the (bix endian) PXA boards. In all cases we have ARCH=arm and CROSS_COMPILE=arm-linux-gnueabi-
And then, for compatibility testings, I want to compile all this with ELDK 4.2. Or ELDK 4.1. Or CodeSourcery xxx. Or...
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
IMHO, for running MAKEALL, I see no problem with this. If we had a 'toolchains.cfg' file which could be a format like:
#ARCH SOC BOARD TOOLCHAIN x86 sc520 - /path/to/gcc
This would give new developers a head-up as to what the defacto toolchains are
We can then have 'toolchains.cfg.local' which is added to .gitignore so individual users can override the toolchain. But all patch submissions must pass MAKEALL without using toolchains.cfg.local (something like 'MAKEALL --no-custom-toolchains'. The first thing MAKEALL should do is scan toolchains.cfg (and toolchains.cfg.local if required) for each selected arch and check that each toolchain is available and spit out 'toolchain not available' warnings.
All we need to do then is setup our build machines to do an automated git-pull and MAKEALL
Regards,
Graeme
Dear Graeme Russ,
Am 19.10.2011 00:33, schrieb Graeme Russ:
Hi Wolfgang,
On Wed, Oct 19, 2011 at 7:07 AM, Wolfgang Denk wd@denx.de wrote:
Dear Mike Frysinger,
In message 201110181301.57390.vapier@gentoo.org you wrote:
<snip>
And then, for compatibility testings, I want to compile all this with ELDK 4.2. Or ELDK 4.1. Or CodeSourcery xxx. Or...
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
IMHO, for running MAKEALL, I see no problem with this. If we had a 'toolchains.cfg' file which could be a format like:
#ARCH SOC BOARD TOOLCHAIN x86 sc520 - /path/to/gcc
This would give new developers a head-up as to what the defacto toolchains are
That is OK.
We can then have 'toolchains.cfg.local' which is added to .gitignore so individual users can override the toolchain. But all patch submissions must pass MAKEALL without using toolchains.cfg.local (something like 'MAKEALL --no-custom-toolchains'. The first thing MAKEALL should do is scan toolchains.cfg (and toolchains.cfg.local if required) for each selected arch and check that each toolchain is available and spit out 'toolchain not available' warnings.
But I don't like to force the users to have _all_ toolchains installed on their work station. I think the current procedure to MAKEALL _at least_ two different arches is enough. Furthermore I don't like to force the users to have a specific toolchain for submitting a patch to the list. I think it is a benefit to have a lot of different toolchains on different host systems building the code, but one should see the build-environment in MAKEALL output to be able to distinguish between error from patch or error from toolchain.
All we need to do then is setup our build machines to do an automated git-pull and MAKEALL
It is a good idea for some automated build process which runs in the backyard and spit out some error/warning messages if one patch does break the build unattended (i.e. the two arches MAKEALL did fail).
best regards
Andreas Bießmann
Dear Graeme Russ,
In message CALButC+_q+bfZuMJyXjn-GbC1XFWjvGeD-go85LVUvp4S16WnA@mail.gmail.com you wrote:
I see no clean way to implement this - ok, we could provide an external tool / data base that maps boards or SoC names to CROSS_COMPILE/ARCH/PATH settings, which each user has to configure for his own set of tool chain settings.
IMHO, for running MAKEALL, I see no problem with this. If we had a 'toolchains.cfg' file which could be a format like:
#ARCH SOC BOARD TOOLCHAIN x86 sc520 - /path/to/gcc
This would give new developers a head-up as to what the defacto toolchains are
This is IMO not flexible enough. The longer I think the more I like the idea of providing a hook (script file name) that gets passed all relevant parameters as arguments (target name, arch, cpu, board name, vendor, soc, options) and that gets sourced by MAKEALL, so it is able to modify any environment variables (PATH, ARCH, CROSS_COMPILE, eventually more) as needed.
We can then have 'toolchains.cfg.local' which is added to .gitignore so individual users can override the toolchain. But all patch submissions must pass MAKEALL without using toolchains.cfg.local (something like 'MAKEALL --no-custom-toolchains'. The first thing MAKEALL should do is scan toolchains.cfg (and toolchains.cfg.local if required) for each selected arch and check that each toolchain is available and spit out 'toolchain not available' warnings.
As mentioned before: a decision based on ARCH alone is not sufficient; see for example the issues with the big endian ARM boards (PXA).
All we need to do then is setup our build machines to do an automated git-pull and MAKEALL
:-)
Best regards,
Wolfgang Denk
On Mon, Oct 17, 2011 at 11:23 PM, Wolfgang Denk wd@denx.de wrote:
Hi all,
the patches that have been submitted for this release turn out to of of shockingly bad quality. About every other batch of patches I apply will break building not only for a single board r a few boards, but for large numbers of boards, including whole processor families and even whole architectures.
Not to mention the huge number of patches that have to be rejected because they raise tons or errors and warnings from checkpatch.pl
The time I have available is limited even without such avoidable problems, and I really, really rather spend it on constructive work instead of continuously running git bisect to track down the culprits.
What's even worse is that it appears that I am the only person in this whole community who runs any build tests. Or how comes that it's always me who detects such build breakages?
Note # 3 at http://www.denx.de/wiki/U-Boot/Patches says:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
I would like to start a thread addressing this question. I don't think the people submitting are running MAKEALL because it has a very high barrier to entry. In particular I spent a few days trying to get as many architecture toolchains up and running as I could so that I could run MAKEALL to fully test my builds. The result was frustrating. I was only able to get ARM to build (Our local toolchain works, but I also got the ELDK toolchain to work for me). I was not able to get the ELDK toolchain for PowerPC to work however.
-Anton
Ahh, looks like Mike agrees...
I cannot continue like that. We need to find ways to improve the quality of the submitted patches, and to distribute the work load for testing.
I need your help.
Thanks.
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de There are no data that cannot be plotted on a straight line if the axis are chosen correctly. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Le 18/10/2011 19:16, Anton Staaf a écrit :
I would like to start a thread addressing this question. I don't think the people submitting are running MAKEALL because it has a very high barrier to entry. In particular I spent a few days trying to get as many architecture toolchains up and running as I could so that I could run MAKEALL to fully test my builds. The result was frustrating. I was only able to get ARM to build (Our local toolchain works, but I also got the ELDK toolchain to work for me). I was not able to get the ELDK toolchain for PowerPC to work however.
-Anton
Ahh, looks like Mike agrees...
As far as I am concerned, I do run ./MAKEALL arm (or a subset) usually before requesting pulls and before applying pull requests, and I have ELD and CS ARM toolchains in place, but I never run MAKEALL on other arches than ARM.
Amicalement,
On Tue, Oct 18, 2011 at 10:44 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Le 18/10/2011 19:16, Anton Staaf a écrit :
I would like to start a thread addressing this question. I don't think the people submitting are running MAKEALL because it has a very high barrier to entry. In particular I spent a few days trying to get as many architecture toolchains up and running as I could so that I could run MAKEALL to fully test my builds. The result was frustrating. I was only able to get ARM to build (Our local toolchain works, but I also got the ELDK toolchain to work for me). I was not able to get the ELDK toolchain for PowerPC to work however.
-Anton
Ahh, looks like Mike agrees...
As far as I am concerned, I do run ./MAKEALL arm (or a subset) usually before requesting pulls and before applying pull requests, and I have ELD and CS ARM toolchains in place, but I never run MAKEALL on other arches than ARM.
Simon pointed me to another thread where Mike published his buildall script and a repository of toolchains. I'm going to try and set that up and see if it works for me. I would love to be able to do a full MAKEALL for all architectures.
Especially since I plan to do more architecture independent work in U-Boot.
Thanks, Anton
Amicalement,
Albert.
Hi Wolfgang,
[...]
Note # 3 at http://www.denx.de/wiki/U-Boot/Patches says:
Before sending the patch, you must run the MAKEALL script on your patched source tree and make sure that no errors or warnings are reported for any of the boards. Well, at least not any more warnings than without your patch.
It is strongly recommended to verify that out-of-tree building (with "-O" make option resp. "BUILD_DIR" environment variable) is still working. For example, run:
$ BUILD_DIR=/tmp/u-boot-build ./MAKEALL
Why is nobody doing this?
I'd like to raise awareness looking at the root of the problems, not how to fix the fallout.
Why is it that we regularly have cases where ("local") U-Boot changes break ("distant") board configurations? Isn't this a sign that our code base isn't as orhtogonal as it should be? Is such an orthogonality not possible for us?
Has anyone got a good explanation why it is this way - and even better ideas on how to improve this?
Thanks Detlev
participants (13)
-
Albert ARIBAUD -
Andreas Bießmann -
Anton Staaf -
Anton Staaf -
Detlev Zundel -
Graeme Russ -
Jason -
Lukasz Majewski -
Mike Frysinger -
Simon Glass -
Simon Schwarz -
Tom Rini -
Wolfgang Denk