| Message ID | 20210225111457.32540-1-david.marchand@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | devtools: catch symbol duplicates in version map | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-testing | success | Testing PASS |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-mellanox-Functional | success | Functional Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/github-robot | success | github build: passed |
| ci/travis-robot | fail | travis build: failed |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
25/02/2021 12:14, David Marchand: > Add a check on versioned symbol duplicates in map files. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > This check could be added to buildtools/check-symbols.sh so that > regular developers catch the issue when building their changes... > Opinions? In general I am against adding developer tools in the build process, because it could cause more issues for normal users.
On Thu, Feb 25, 2021 at 12:41:16PM +0100, Thomas Monjalon wrote: > 25/02/2021 12:14, David Marchand: > > Add a check on versioned symbol duplicates in map files. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > This check could be added to buildtools/check-symbols.sh so that > > regular developers catch the issue when building their changes... > > Opinions? > > In general I am against adding developer tools in the build process, > because it could cause more issues for normal users. > Is this really likely to cause issues for normal users? If we add this to the build process any issues will surely be caught be developers before merge.
25/02/2021 12:57, Bruce Richardson: > On Thu, Feb 25, 2021 at 12:41:16PM +0100, Thomas Monjalon wrote: > > 25/02/2021 12:14, David Marchand: > > > Add a check on versioned symbol duplicates in map files. > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > --- > > > This check could be added to buildtools/check-symbols.sh so that > > > regular developers catch the issue when building their changes... > > > Opinions? > > > > In general I am against adding developer tools in the build process, > > because it could cause more issues for normal users. > > > Is this really likely to cause issues for normal users? Yes because some users will have a different shell, or other weird setup we don't think about yet. > If we add this to > the build process any issues will surely be caught be developers before > merge. If we want the checks to be more popular, we should write a script to help running the right script at the right time, and ideally help in the contribution process.
On Thu, Feb 25, 2021 at 01:01:10PM +0100, Thomas Monjalon wrote: > 25/02/2021 12:57, Bruce Richardson: > > On Thu, Feb 25, 2021 at 12:41:16PM +0100, Thomas Monjalon wrote: > > > 25/02/2021 12:14, David Marchand: > > > > Add a check on versioned symbol duplicates in map files. > > > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > > --- > > > > This check could be added to buildtools/check-symbols.sh so that > > > > regular developers catch the issue when building their changes... > > > > Opinions? > > > > > > In general I am against adding developer tools in the build process, > > > because it could cause more issues for normal users. > > > > > Is this really likely to cause issues for normal users? > > Yes because some users will have a different shell, > or other weird setup we don't think about yet. > I think it unlikely, but ok. Now that meson (from version 0.53 onwards) has a "filesystem" module, with an "exists" function, we can perhaps look to introduce a "developer mode" build again, based off the presence of the .git folder. Alternatively, we could make "developer mode" a regular meson option rather than trying to be too smart about it. [Or combine both and have developer mode option with "enabled"/"disabled"/"auto-detect" values] /Bruce
25/02/2021 13:05, Bruce Richardson: > On Thu, Feb 25, 2021 at 01:01:10PM +0100, Thomas Monjalon wrote: > > 25/02/2021 12:57, Bruce Richardson: > > > On Thu, Feb 25, 2021 at 12:41:16PM +0100, Thomas Monjalon wrote: > > > > 25/02/2021 12:14, David Marchand: > > > > > Add a check on versioned symbol duplicates in map files. > > > > > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > > > --- > > > > > This check could be added to buildtools/check-symbols.sh so that > > > > > regular developers catch the issue when building their changes... > > > > > Opinions? > > > > > > > > In general I am against adding developer tools in the build process, > > > > because it could cause more issues for normal users. > > > > > > > Is this really likely to cause issues for normal users? > > > > Yes because some users will have a different shell, > > or other weird setup we don't think about yet. > > > I think it unlikely, but ok. You would be surprised. I'm going to send a fix for running buildtools/check-symbols.sh on busybox. > Now that meson (from version 0.53 onwards) has a "filesystem" module, with > an "exists" function, we can perhaps look to introduce a "developer mode" > build again, based off the presence of the .git folder. Alternatively, we > could make "developer mode" a regular meson option rather than trying to be > too smart about it. [Or combine both and have developer mode option with > "enabled"/"disabled"/"auto-detect" values] Yes, a developer mode is a good idea.
diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh index c3cbcaf720..a430de7f64 100755 --- a/devtools/check-symbol-maps.sh +++ b/devtools/check-symbol-maps.sh @@ -35,6 +35,21 @@ if [ -n "$orphan_symbols" ] ; then ret=1 fi +find_duplicate_symbols () +{ + for map in $(find lib drivers -name '*.map') ; do + ! buildtools/map-list-symbol.sh $map | \ + sort | uniq -c | grep -v " 1 $map" + done +} + +duplicate_symbols=$(find_duplicate_symbols) +if [ -n "$duplicate_symbols" ] ; then + echo "Found duplicates in symbol map file:" + echo "$duplicate_symbols" + ret=1 +fi + find_orphan_windows_symbols () { for def in $(find lib drivers -name '*_exports.def') ; do
Add a check on versioned symbol duplicates in map files. Signed-off-by: David Marchand <david.marchand@redhat.com> --- This check could be added to buildtools/check-symbols.sh so that regular developers catch the issue when building their changes... Opinions? --- devtools/check-symbol-maps.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+)