First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 131779
Alias:
Product:
Component:
Status: RESOLVED
Resolution: FIXED
Assigned To: Portage team <dev-portage@gentoo.org>
Hardware:
OS:
Version:
Priority:
Severity:
Reporter: Kevin F. Quinn (RETIRED) <kevquinn@gentoo.org>
Add CC:
CC:
Remove selected CCs
URL:
Summary:
Status Whiteboard:
Keywords:

Filename Description Type Creator Created Size Actions
portage-2.1-qatextrels.patch Replace use of gawk with bash for loops patch Kevin F. Quinn (RETIRED) 2006-04-30 03:26 0000 1.13 KB Details | Diff
portage-2.0-qatextrels.patch Backport to 2.0.54-r1 patch Kevin F. Quinn (RETIRED) 2006-04-30 03:27 0000 1.44 KB Details | Diff
portage-2.1-qatextrels.patch Reworked patch for 2.1 supporting regexes, old style output and QA_EXECSTACK patch Kevin F. Quinn (RETIRED) 2006-05-02 03:05 0000 2.10 KB Details | Diff
portage-2.0-qatextrels.patch Reworked backport for 2.0 in line with patch #85978 patch Kevin F. Quinn (RETIRED) 2006-05-02 03:06 0000 3.10 KB Details | Diff
portage-2.1-qatextrels.patch rework 2.1 - regex, vertical list output, filenames with spaces patch Kevin F. Quinn (RETIRED) 2006-05-03 10:09 0000 2.71 KB Details | Diff
portage-2.0-qatextrels.patch rework 2.0 - regex. vertical list output, filenames with spaces patch Kevin F. Quinn (RETIRED) 2006-05-03 10:10 0000 3.48 KB Details | Diff
portage-svn-3316-qatextrels.patch 2.1 patch against svn 3316 patch Kevin F. Quinn (RETIRED) 2006-05-03 16:50 0000 3.36 KB Details | Diff
portage-svn-3316-qatextrels.patch 2.1 patch against svn 3316, with noglob fix. patch Kevin F. Quinn (RETIRED) 2006-05-04 00:30 0000 3.39 KB Details | Diff
portage-svn-3316-qatextrels.patch 2.1 against svn 3316 - regex fix (noglob) patch Kevin F. Quinn (RETIRED) 2006-05-06 03:31 0000 3.42 KB Details | Diff
portage-svn-2.0-3323-noglob.patch 2.0 - noglob fix against svn 3323 patch Kevin F. Quinn (RETIRED) 2006-05-06 03:48 0000 1.24 KB Details | Diff
portage-2.1-svn3323-simple.patch 2.1 simplified patch against svn 3323 patch Kevin F. Quinn (RETIRED) 2006-05-06 03:53 0000 3.37 KB Details | Diff
portage-2.0-svn3323-simple.patch 2.0 simplified patch against svn 3323 patch Kevin F. Quinn (RETIRED) 2006-05-06 03:54 0000 1.28 KB Details | Diff
pax-utils-qa.patch scanelf modification to do the qa stuff patch Kevin F. Quinn (RETIRED) 2006-05-09 04:09 0000 6.26 KB Details | Diff
pax-utils-qa.patch scanelf patch, whitespace separator patch Kevin F. Quinn (RETIRED) 2006-05-09 05:58 0000 6.51 KB Details | Diff
portage-2.1-svn-3341-qainscanelf.patch Remove processing from misc-functions.sh that's now done in scanelf (2.1) patch Kevin F. Quinn (RETIRED) 2006-05-12 12:59 0000 1.81 KB Details | Diff
portage-2.0-svn-3341-qainscanelf.patch Remove processing from ebuild.sh that's now done in scanelf (2.0) patch Kevin F. Quinn (RETIRED) 2006-05-12 13:00 0000 2.14 KB Details | Diff
pax-utils-qa_wx_load.patch Adds QA_WX_LOAD to scanelf for the PT_LOAD W|X check patch Kevin F. Quinn (RETIRED) 2006-05-13 06:04 0000 1.17 KB Details | Diff
pax-utils-qa_execstack.patch Alternative: make QA_EXECSTACK filter everything in scanelf -e patch Kevin F. Quinn (RETIRED) 2006-05-13 06:06 0000 1.04 KB Details | Diff
portage-2.0-svn-3341-qainscanelf-wx_load.patch Remove processing from ebuild.sh that's now done in scanelf (2.0), including QA_WX_LOAD support patch Kevin F. Quinn (RETIRED) 2006-05-13 06:07 0000 2.29 KB Details | Diff
portage-2.1-svn-3341-qainscanelf-wx_load.patch Remove processing from misc-functions.sh that's now done in scanelf (2.1), including QA_WX_LOAD support patch Kevin F. Quinn (RETIRED) 2006-05-13 06:08 0000 1.95 KB Details | Diff
portage-2.0-svn-3407-qainscanelf-wx_load.patch Remove processing from ebuild.sh that's now done in scanelf (2.0), including QA_WX_LOAD support patch Kevin F. Quinn (RETIRED) 2006-05-24 00:14 0000 2.28 KB Details | Diff
portage-2.1-svn-3407-qainscanelf-wx_load.patch Remove processing from misc-functions.sh that's now done in scanelf (2.1), including QA_WX_LOAD support patch Kevin F. Quinn (RETIRED) 2006-05-24 00:15 0000 1.95 KB Details | Diff
portage-svn-3409-strictwx-man.patch update man pages (and a little fix :/ ) patch Kevin F. Quinn (RETIRED) 2006-05-24 07:52 0000 3.78 KB Details | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 131779 depends on: Show dependency tree
Bug 131779 blocks: 115839
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments: (this is where you put emerge --info)


Not eligible to see or edit group visibility for this bug.






View Bug Activity   |   Format For Printing   |   XML   |   Clone This Bug


Description:   Opened: 2006-04-30 03:24 0000
Two things really:

First, I'd like to get the QA_TEXTRELS filtering into the 2.0 stable branch
since people have started RESTRICTing "stricter", which we'd like to
discourage.

Second, the QA_TEXTRELS stuff in the 2.1 branch currently fails if the
QA_TEXTRELS string is written on more than one line (gawk bails).

To be attached - patches against 2.0.54-r1 and 2.1_pre9-r5.

------- Comment #1 From Kevin F. Quinn (RETIRED) 2006-04-30 03:26:54 0000 -------
Created an attachment (id=85819) [details]
Replace use of gawk with bash for loops

Use of bash 'for' loops rather than gawk to process QA_TEXTRELS against the
scanelf output allows QA_TEXTRELS to be specified on more than one line.

------- Comment #2 From Kevin F. Quinn (RETIRED) 2006-04-30 03:27:50 0000 -------
Created an attachment (id=85820) [details]
Backport to 2.0.54-r1

Exact same code, but in ebuild.sh rather than misc-functions.sh.

------- Comment #3 From solar 2006-04-30 07:18:49 0000 -------
It's in SVN both branches right now. thanks for the updates.

------- Comment #4 From SpanKY 2006-04-30 08:25:28 0000 -------
i dont like this patch at all ... issues:
 - you dont support regexes
 - your version makes the output ugly and really unreadable

if your only gripe with the current version is that the QA vars cant be
declared on one line, then i can fix that

also, if you're going to change/backport QA_TEXTRELS to stable, you should have
also done QA_EXECSTACKS since the code is pretty much exactly the same

------- Comment #5 From solar 2006-04-30 09:13:01 0000 -------
(In reply to comment #4)
> i dont like this patch at all ... issues:
>  - you dont support regexes
>  - your version makes the output ugly and really unreadable

ha. I and I just had copied the portage-patches-2.0.54-2 to the 
distfiles-local and was about 2mins away from commiting. I logged in 
and deleted the files before it had a chance to be mirrored I think.

> then i can fix that
..
>  QA_EXECSTACKS since the code is pretty much exactly the same

SpanKY: Can you fix that like soonish?

------- Comment #6 From Kevin F. Quinn (RETIRED) 2006-04-30 13:32:21 0000 -------
(In reply to comment #4)
> i dont like this patch at all ... issues:
>  - you dont support regexes

I hadn't noticed regex support was there, as it hadn't occurred to me it would
be necessary.  It's easy enough though - replace:

[[ ${t} == ${s} ]] && continue 2

with

[[ ${s} =~ ${t} ]] && continue 2

>  - your version makes the output ugly and really unreadable

I think that's putting it a little strongly, but writing:

for t in ${f[@]}; do
  echo "TEXTREL ${t}"
done

is straightforward.

> if your only gripe with the current version is that the QA vars cant be
> declared on one line, then i can fix that

Whatever happens, I think that needs to be fixed.  It's normal for portage vars
to support multiple lines, and if such support is missing it gets ugly trying
to maintain sensible line lengths if there are several entries in the variable.

> also, if you're going to change/backport QA_TEXTRELS to stable, you should have
> also done QA_EXECSTACKS since the code is pretty much exactly the same

Fairy snuff.

Let me know if you want me to provide updated patches; otherwise I'll assume
you're going to do it your way.  Either is fine by me, as long as it gets into
stable and supports multi-line values.

------- Comment #7 From Kevin F. Quinn (RETIRED) 2006-05-02 03:05:11 0000 -------
Created an attachment (id=85978) [details]
Reworked patch for 2.1 supporting regexes, old style output and QA_EXECSTACK

Now supports regexes, multi-line values, displays the output as provided by
scanelf, includes same for QA_EXECSTACK.

------- Comment #8 From Kevin F. Quinn (RETIRED) 2006-05-02 03:06:41 0000 -------
Created an attachment (id=85979) [details]
Reworked backport for 2.0 in line with patch #85978

------- Comment #9 From solar 2006-05-02 04:39:24 0000 -------
SpanKY.. Waiting on you now.. please review the follow patches

------- Comment #10 From SpanKY 2006-05-02 20:48:11 0000 -------
the array stuff is unnecessary, just use '#t%p' for the scanelf format

you can use '<<-EOF' and then indent the code with tabs so you dont break
formatting

as for the while/read/EOF stuff, i dislike that ;)

------- Comment #11 From Kevin F. Quinn (RETIRED) 2006-05-03 02:06:07 0000 -------
(In reply to comment #10)
> the array stuff is unnecessary, just use '#t%p' for the scanelf format

ok; if you don't want the previous scanelf format retained, it's a lot easier.

> you can use '<<-EOF' and then indent the code with tabs so you dont break
> formatting

useful to know, thanks.

> as for the while/read/EOF stuff, i dislike that ;)

heh - thought you wouldn't, you gawk fiend ;)
That was the only way I could retain the output from scanelf unchanged; doing:

  for s in $(scanelf -qyRF '"%t %p"' ${D}); do...

or

  s=( $(scanelf -qyRF '"%t %p"' ${D}) )

splits '"TEXTREL fred"' into two tokens, '"TEXTREL' and 'fred"', which is not
what I had expected; I had hoped it would do the same as if the output of
scanelf was pasted in place of the command, i.e. tokenising as "TEXTREL fred",
"TEXTREL jim" etc.

If you're happy with just a list of files rather than the "%t %p" and "%e %p"
formats, the while loop can be dropped for a simpler for loop.

------- Comment #12 From Kevin F. Quinn (RETIRED) 2006-05-03 10:09:16 0000 -------
Created an attachment (id=86077) [details]
rework 2.1 - regex, vertical list output, filenames with spaces

Here we go.  No arrays, no while read loops :)

Found a way to get the shell to split the output of scanelf '"#t%p"' to support
filenames with spaces, using eval to expand the variable before the for loop is
parsed.

Note; to solve the newline problem (which is where I started), it's enough to
do:

QA_TEXTRELS=$(echo ${QA_TEXTRELS})

prior to the call to gawk; so if you don't care about filenames with spaces you
could just do that.  I couldn't see an easy way to get gawk to parse names with
spaces as it doesn't treat quotes in any special way.

------- Comment #13 From Kevin F. Quinn (RETIRED) 2006-05-03 10:10:12 0000 -------
Created an attachment (id=86079) [details]
rework 2.0 - regex. vertical list output, filenames with spaces

------- Comment #14 From solar 2006-05-03 15:41:29 0000 -------
2.0.54 (svn revision 3314) updated with "rework 2.0 - regex." attachment #86079 [details] 

------- Comment #15 From solar 2006-05-03 15:51:23 0000 -------
Kevin, can you please svn up and repost the 2.1 patch.

------- Comment #16 From Kevin F. Quinn (RETIRED) 2006-05-03 16:50:37 0000 -------
Created an attachment (id=86099) [details]
2.1 patch against svn 3316

Added vprintf() to isolated-functions.sh, to provide support for printing
strings containing formatting, in the style of the vecho().

I've tested vprintf() on its own, but haven't tested svn portage as such.  Rest
of the patch tested as part of 2.1-pre9-r5, but it's self-contained so should
be ok.

------- Comment #17 From SpanKY 2006-05-03 18:19:21 0000 -------
yeah this just keeps getting worse

what about using original gawk script but with this change:
-    BEGIN { split("'"${QA_TEXTRELS}"'", ignore); }
+    BEGIN { split("'$(set -o noglob;echo ${QA_TEXTRELS})'", ignore); }

------- Comment #18 From SpanKY 2006-05-03 18:31:20 0000 -------
actually, that's hackish ... what about this:
-gawk '
-    BEGIN { split("'"${QA_TEXTRELS}"'", ignore); }
+gawk -v QA_TEXTRELS="${QA_TEXTRELS}" '
+    BEGIN { split(QA_TEXTRELS, ignore); }

------- Comment #19 From Kevin F. Quinn (RETIRED) 2006-05-04 00:27:37 0000 -------
(In reply to comment #17)
> yeah this just keeps getting worse

If you say that sort of thing, you should say _why_ you think it's worse.

The reasons why I think your gawk solution is worse:
1) Doesn't deal with filenames containing spaces
2) Involves a second language (gawk) when it does not provide anything that
can't be done easily and effectively in the first (bash)

From where I'm standing, my code is simple and clear enough - with comments
explaining the two bits whose presence isn't self-documenting (the echo to
remove newlines, and the eval to get bash to process quoted tokens in the
variable correctly).  I'll update it with the 'set -o noglob' to avoid
expansion in the echo.

Further, using bash instead of gawk means the reader only has to know bash, not
gawk.  In my opinion, using languages other than bash in shell scripts should
be restricted to cases where the other language provides a clear benefit. 
There's no benefit to using gawk here.

As I said in comment #12, if you don't care about spaces in filenames then the
echo to remove the newlines is enough.  However if you want to support spaces
in filenames you need more.

------- Comment #20 From Kevin F. Quinn (RETIRED) 2006-05-04 00:30:19 0000 -------
Created an attachment (id=86124) [details]
2.1 patch against svn 3316, with noglob fix.

------- Comment #21 From Kevin F. Quinn (RETIRED) 2006-05-04 00:33:29 0000 -------
(From update of attachment 86124 [details])
Actually the noglob isn't necessary on the scanelf results; reverting.

------- Comment #22 From SpanKY 2006-05-05 20:01:47 0000 -------
> 1) Doesn't deal with filenames containing spaces

and ?  neither does the proposed bash version, and really it isnt doable unless
we specify an unintuitive separator for use in the QA variables ... then again,
the gawk would be able to handle the separator just fine with '-F' whereas a
bash solution would involve saving/restoring IFS

> 2) Involves a second language (gawk) when it does not provide anything that
> can't be done easily and effectively in the first (bash)

we disagree on the "easily and effectively" part

the current bash code is more complicated than needs be and imo the gawk is
simpler ... plus the bash code needs to make sure the stuff in the QA var is
not shell expanded whereas no such concern exists with the gawk version

> Actually the noglob isn't necessary on the scanelf results; reverting.

noglob is needed, otherwise you can get wrong results

------- Comment #23 From Kevin F. Quinn (RETIRED) 2006-05-06 03:28:16 0000 -------
(In reply to comment #22)
> > 1) Doesn't deal with filenames containing spaces
> 
> and ?  neither does the proposed bash version,

Yes it does.  What did you try?  It works for me -

QA_TEXTRELS='fred "jim bob"'

or

QA_TEXTRELS="fred \"jim bob\""

works fine.

> and really it isnt doable unless
> we specify an unintuitive separator for use in the QA variables ... then again,
> the gawk would be able to handle the separator just fine with '-F' whereas a
> bash solution would involve saving/restoring IFS

As you can see above, none of that is necessary.  Simply double-quote the
filename that contains spaces - as you have to do when using such filenames
anyway.

You can't parse that easily in gawk; it'll split on the separator but won't
respect the quotes, so you'd have to change the separator as you say - ':'
would be a sensible choice if you go that route, as used in path variables (and
what we did for the GCC_SPECS variable).

Where bash is concerned, it doesn't respect the quotes if you write:

for file in ${QA_TEXTRELS}; do...

but it does if you do:

eval "for file in ${QA_TEXTRELS}; do..."

which is why I used the eval.

> > 2) Involves a second language (gawk) when it does not provide anything that
> > can't be done easily and effectively in the first (bash)
> 
> we disagree on the "easily and effectively" part

Obviously.  "effectively" was a reference to spaces in filenames.  "easily" is
more subjective, but I certainly found it easy enough to do it in bash.

> the current bash code is more complicated than needs be and imo the gawk is
> simpler ... 

As far as complexity goes, there's nothing in it.  Whether one is clearer than
another is somewhat subjective, but I still think it's less effort to grok the
double for loop than it is to grok the pipe and gawk for loop.

> plus the bash code needs to make sure the stuff in the QA var is
> not shell expanded whereas no such concern exists with the gawk version
> 
> > Actually the noglob isn't necessary on the scanelf results; reverting.
> 
> noglob is needed, otherwise you can get wrong results

scanelf reports a list of files, not a pattern match, so noglob is unnecessary
on the scanelf output, which is what I did before.  For some inexplicable
reason I thought the eval wouldn't need it either, but of course it does.  I'll
update my patch and attach it.

------- Comment #24 From Kevin F. Quinn (RETIRED) 2006-05-06 03:31:46 0000 -------
Created an attachment (id=86246) [details]
2.1 against svn 3316 - regex fix (noglob)

------- Comment #25 From Kevin F. Quinn (RETIRED) 2006-05-06 03:48:16 0000 -------
Created an attachment (id=86247) [details]
2.0 - noglob fix against svn 3323

Not sure this is a real problem anyway - without this change, when QA_TEXTRELS
is expanded by the shell the current working directory is ${D} anyway.  So the
only difference is that the inner loop has more iterations.

Incidentally, this means my version could be simplified by removing the
subshell, the "set -o noglob" and reverting the comparison to just

[[ \${s} == \${t} ]]

------- Comment #26 From Kevin F. Quinn (RETIRED) 2006-05-06 03:53:30 0000 -------
Created an attachment (id=86248) [details]
2.1 simplified patch against svn 3323

------- Comment #27 From Kevin F. Quinn (RETIRED) 2006-05-06 03:54:00 0000 -------
Created an attachment (id=86249) [details]
2.0 simplified patch against svn 3323

------- Comment #28 From Kevin F. Quinn (RETIRED) 2006-05-06 04:13:34 0000 -------
So now, our discussion centers around which is better:

        s=$(scanelf -qyRF '"#t%p"' "${D}" | grep -v 'usr/lib/debug/')
        s=$(echo ${s}) # strip newlines
        f=$(eval "
        for s in ${s}; do
            for t in ${QA_TEXTRELS}; do
                [[ \${s} == \${t} ]] && continue 2
            done
            printf \"\${s}\n\"
        done")

with the double-quote method of coping with spaces, or

        f=$(scanelf -qyRF '%t %p' "${D}" | grep -v ' usr/lib/debug/' | \
                gawk -v QA_TEXTRELS="${QA_TEXTRELS}" '
                BEGIN { split("'"${QA_TEXTRELS}"'", ignore, ":"); }
                {       for (idx in ignore)
                                if ($NF ~ "^ *"ignore[idx]"$")
                                next;
                        print;
                }')

with the ':'-separator method (doesn't cope with filenames that start with
spaces, but although that's valid it's pathological).

------- Comment #29 From SpanKY 2006-05-07 00:20:04 0000 -------
you didnt include my changes ... revised gawk would be:
        f=$(scanelf -qyRF '#t%p' "${D}" | grep -v ' usr/lib/debug/' | \
                gawk -v QA_TEXTRELS="${QA_TEXTRELS}" '
                BEGIN { split(QA_TEXTRELS, ignore, ":"); }
                {       for (idx in ignore)
                            if ($0 ~ "^"ignore[idx]"$")
                                next;
                        print "TEXTREL " $0;
                }')

once you start getting into eval and embedded quotes, things get to be a pita
to maintain ... there is no question of what does and doesnt need to be escaped
when using gawk

------- Comment #30 From Kevin F. Quinn (RETIRED) 2006-05-07 04:46:28 0000 -------
(In reply to comment #29)
> you didnt include my changes ... revised gawk would be:

I missed a couple of the changes; sorry about that.

I tried the gawk version, but it still doesn't work.

Firstly:

  gawk -v QA_TEXTRELS="${QA_TEXTRELS}" '...

doesn't remove newlines so the matches fail (although it does stop gawk
bailing), and if it's replaced with:

  gawk -v QA_TEXTRELS="$(set -o noglob; echo ${QA_TEXTRELS})" '...

the newlines become spaces so

    if ($0 ~ "^"ignore[idx]"$")

doesn't match because $0 has a leading space (my earlier suggestion of

    if ($0 ~ "^ *"ignore[idx]"$")

is garbage as well, fwiw).

I can get it to work by doing:

        q=$(printf "${QA_TEXTRELS} | sed -e 's:^[ \t]*::')
        q=$(printf "${q//
}")
        f=$(scanelf -qyRF '#t%p' "${D}" | grep -v ' usr/lib/debug/' | \
                gawk -v QA_TEXTRELS="${q}" '
                BEGIN { split(QA_TEXTRELS, ignore, ":"); }
                {       for (idx in ignore)
                            if ($0 ~ "^"ignore[idx]"$")
                                next;
                        print "TEXTREL " $0;
                }')

to allow the definition of QA_TEXTRELS to be:

QA_TEXTRELS="usr/bin/fred*:
             usr/bin/bob:
             usr/bin/j i m:
             usr/lib/harry.so"

although people are bound to forget the ':'s.  My version accepts:

QA_TEXTRELS='usr/bin/fred*
             usr/bin/bob
             "usr/bin/j i m"
             usr/bin/harry.so'
or
QA_TEXTRELS="usr/bin/fred*
             usr/bin/bob
             \"usr/bin/j i m\"
             usr/bin/harry.so"

which I think is more natural for portage variables.

> once you start getting into eval and embedded quotes, things get to be a pita
> to maintain ... there is no question of what does and doesnt need to be
> escaped when using gawk

I still feel that when you compare the two methods, my code is clearer (and
hence easier to maintain), and as for embedded quotes in QA_TEXTRELS, that
seems perfectly natural to me.

However, you do whichever way you think is best - you have authority
legitimately derived from the volume of work you do.  I really only care that
the variable definition should support a sensible way of avoiding overly-long
lines.

If you go the ':'-separator route, it might be reasonable to forbid extra
whitespace for formatting (including newlines), and require the ebuild author
to write things like:

QA_TEXTRELS="usr/bin/fred*:usr/bin/bob:usr/bin/j i m"
QA_TEXTRELS="${QA_TEXTRELS}:usr/bin/harry.so"

if they wish to split stuff over more than one line - the reason being that the
no-extra-whitespace rule applies to other ':'-separated variables like PATH
(and GCC_SPECS, for example), so it would be consistent. Then

        f=$(scanelf -qyRF '#t%p' "${D}" | grep -v ' usr/lib/debug/' | \
                gawk -v QA_TEXTRELS="${QA_TEXTRELS}" '
                BEGIN { split(QA_TEXTRELS, ignore, ":"); }
                {       for (idx in ignore)
                            if ($0 ~ "^"ignore[idx]"$")
                                next;
                        print "TEXTREL " $0;
                }')

is sufficient.  We'll have to bump the 2.0.54 series again to do that, of
course.

------- Comment #31 From solar 2006-05-07 05:13:10 0000 -------
this seems like it's becoming over complex. 

Spanky/Kevin: 
How about we shift the logic into scanelf itself and have it ignore internally
using fnmatch() ?

------- Comment #32 From Kevin F. Quinn (RETIRED) 2006-05-07 07:15:23 0000 -------
Certainly could do.  We would still need to decide how to pass the relevant
information from the ebuild to the invocation of scanelf, of course.

Other tools do this sort of thing via options; the tar '--exclude PATTERN'
option seems sensible, which can be specified multiple times.  The
'--exclude-from FILE' option might be worth considering; it has the advantage
that it doesn't have to worry about separators or spaces, although the
disadvantage is that it necessitates creating more files (e.g.
"${FILESDIR}/${PF}-textrels.qa").

BTW after thinking about c#30 some more, I'm more convinced that using the
PATH-style format for the environment variable makes most sense in that
approach.  It's de-facto standard for specifying a list of filesystem entities,
and whatever can't be supported in that style will fail other stuff as well
anyway.

The colon-separated environment variable could be used equally well by scanelf,
if you want to go that way rather than adding yet another option.  Actually, a
bit of both would be good, in that invocation would become:

scanelf -qyRF "%t %p" "${D}" --exclude="${QA_TEXTRELS}"

(i.e. such that scanelf wouldn't read the environment variable itself)

------- Comment #33 From solar 2006-05-07 07:36:54 0000 -------
(In reply to comment #32)
> Certainly could do.  We would still need to decide how to pass the relevant
> information from the ebuild to the invocation of scanelf, of course.

Heh.. To keep it real simple I was thinking the env variable QA_TEXTRELS :)

------- Comment #34 From solar 2006-05-07 07:55:06 0000 -------
Pseudo Example:

Index: scanelf.c
===================================================================
RCS file: /var/cvsroot/gentoo-projects/pax-utils/scanelf.c,v
retrieving revision 1.141
diff -u -b -B -w -p -r1.141 scanelf.c
--- scanelf.c   23 Apr 2006 15:24:38 -0000      1.141
+++ scanelf.c   7 May 2006 14:49:52 -0000
@@ -276,8 +276,18 @@ static const char *scanelf_file_textrel(
        static const char *ret = "TEXTREL";
        unsigned long i;

+       char *env = getenv("QA_TEXTRELS");
+
        if (!show_textrel && !show_textrels) return NULL;

+       if (env != NULL) {
+               char e[__PAX_UTILS_PATH_MAX] = "";
+               FOREACH(e, env) {
+                       if (fnmatch(e, elf->filename, 0) == 0)
+                               return NULL;
+               }
+       }
+
        if (elf->phdr) {
 #define SHOW_TEXTREL(B) \
        if (elf->elf_class == ELFCLASS ## B) { \

------- Comment #35 From SpanKY 2006-05-07 17:12:51 0000 -------
> I tried the gawk version, but it still doesn't work.

that's because you're accepting garbage from the QA_TEXTREL input variable

it's either whitespace delimited or : delimited, mixing and matching just
complicates things

and going with a : delimited variable doesnt add enough benefit to make it
worth it

> I still feel that when you compare the two methods, my code is clearer

as i said, using eval and trying to escape everything properly just makes
things a pita, no two ways about it

------- Comment #36 From Kevin F. Quinn (RETIRED) 2006-05-07 23:25:48 0000 -------
(In reply to comment #35)
> > I tried the gawk version, but it still doesn't work.
> 
> that's because you're accepting garbage from the QA_TEXTREL input variable
> 
> it's either whitespace delimited or : delimited, mixing and matching just
> complicates things
>
> and going with a : delimited variable doesnt add enough benefit to make it
> worth it

So if we stick with whitespace delimited, how is your gawk going to work?  As
it stands it only supports single-line definitions, as I explained in my first
point on c#30.

> > I still feel that when you compare the two methods, my code is clearer
> 
> as i said, using eval and trying to escape everything properly just makes
> things a pita, no two ways about it

Using arrays is the natural way of expressing sets of strings in bash:

QA_TEXTRELS=( "usr/bin/fred*" usr/bin/bob "usr/bin/j i m"
              usr/lib/harry.so )

for q in "${QA_TEXTELS[@]}"; do
...

so how about that?

------- Comment #37 From Kevin F. Quinn (RETIRED) 2006-05-09 04:09:18 0000 -------
Created an attachment (id=86470) [details]
scanelf modification to do the qa stuff

(re c#34)

Splits by ":" - if you want whitespace separator, just replace ":" with " \t\n"
in the calls to strtok_r (obviously won't support spaces in filenames, fwiw).

Prepends search_path when doing fnmatch (file_matches_list()), so that the
match uses the same text that scanelf reports.  Tweaked scanelf_dir to stop it
generating '//' in the middle of paths when concatenating the path and entry.
Maybe file_matches_list() should check whether the filename starts with '/'
before prepending search_path, but I'm not sure.

Does the execstack filtering only on the PT_GNU_STACK check - not sure if
that's sensible (depends what QA_EXECSTACK means), but it shows the filtering
can be more specific if we want.

The reason for moving the utility functions was to allow the inline to work; I
got warnings that it couldn't be inlined because the body wasn't available
(might have been with -ggdb3 only, though).

------- Comment #38 From solar 2006-05-09 04:59:49 0000 -------
Kevin,SpanKY:
Is whitespace in the QA_TEXTREL expression so vitual?
I'd really rather use the expression string of 
QA_TEXTREL="usr/lib/*.so next_item last_item" to follow suit with the existing 
INSTALL_MASK and STRIP_MASK's. So can we replace the ":" with " "?

------- Comment #39 From Kevin F. Quinn (RETIRED) 2006-05-09 05:58:47 0000 -------
Created an attachment (id=86474) [details]
scanelf patch, whitespace separator

(In reply to comment #38)
> So can we replace the ":" with " "?

With " \t\n", yes (so people can format stuff nicely in ebuilds).  Since those
_MASK variables don't cater for spaces in filenames, there's no point worrying
about them here.  Need to twiddle things a bit; revised patch attached (I did
think about switching from strtok_r to strsep as that's used elsewhere, but
strsep returns empty strings for every pair of separator characters, which is
not what we want).

------- Comment #40 From solar 2006-05-10 14:22:18 0000 -------
(In reply to comment #39)
> Created an attachment (id=86474) [edit] [details]
> scanelf patch, whitespace separator

I like this patch. Feel free to commit it to pax-utils cvs Kevin.

------- Comment #41 From Kevin F. Quinn (RETIRED) 2006-05-10 15:46:34 0000 -------
(In reply to comment #40)
> I like this patch. Feel free to commit it to pax-utils cvs Kevin.

Done.

------- Comment #42 From solar 2006-05-11 15:19:40 0000 -------
(In reply to comment #41)
> (In reply to comment #40)
> > I like this patch. Feel free to commit it to pax-utils cvs Kevin.
> 
> Done.

Great. 
Now feel free to attach patches here to ebuild.sh that can drop all the 
gawk, eval etc. I'm guessing you/we will need to export QA_FOO if set.

------- Comment #43 From Kevin F. Quinn (RETIRED) 2006-05-12 12:59:20 0000 -------
Created an attachment (id=86667) [details]
Remove processing from misc-functions.sh that's now done in scanelf

(In reply to comment #42)
> Now feel free to attach patches here to ebuild.sh that can drop all the 
> gawk, eval etc. I'm guessing you/we will need to export QA_FOO if set.

Here we are :)  export is indeed necessary - it's unconditional since I don't
think it matters if it's set or not prior to the export.

QA_*_${ARCH} => QA_*, and QA_STRICT_* handling is left to the scripts.

------- Comment #44 From Kevin F. Quinn (RETIRED) 2006-05-12 13:00:13 0000 -------
Created an attachment (id=86668) [details]
Remove processing from ebuild.sh that's now done in scanelf (2.0)

------- Comment #45 From Kevin F. Quinn (RETIRED) 2006-05-13 06:02:43 0000 -------
One more issue arose; the -e option rejects both W|X PT_GUN_STACK headers and
W|X PT_LOAD headers.  The patches above only deal with the first.

There are two ways to deal with this; either:
1) Widen QA_EXECSTACK back to what it was before (i.e. skip the whole -e check)
2) Add QA_WX_LOAD to filter the W|X PT_LOAD headers

To follow, patches for both possibilities.

------- Comment #46 From Kevin F. Quinn (RETIRED) 2006-05-13 06:04:08 0000 -------
Created an attachment (id=86696) [details]
Adds QA_WX_LOAD to scanelf for the PT_LOAD W|X check

------- Comment #47 From Kevin F. Quinn (RETIRED) 2006-05-13 06:06:05 0000 -------
Created an attachment (id=86697) [details]
Alternative: make QA_EXECSTACK filter everything in scanelf -e

------- Comment #48 From Kevin F. Quinn (RETIRED) 2006-05-13 06:07:52 0000 -------
Created an attachment (id=86698) [details]
Remove processing from ebuild.sh that's now done in scanelf (2.0), including
QA_WX_LOAD support

------- Comment #49 From Kevin F. Quinn (RETIRED) 2006-05-13 06:08:28 0000 -------
Created an attachment (id=86699) [details]
Remove processing from misc-functions.sh that's now done in scanelf (2.1),
including QA_WX_LOAD support

------- Comment #50 From solar 2006-05-23 11:35:29 0000 -------
(In reply to comment #49)
> Created an attachment (id=86699) [edit] [details]
> Remove processing from misc-functions.sh that's now done in scanelf (2.1),
> including QA_WX_LOAD support


Kevin,
The line that reads this does not match up with what you added to scanelf.
+ export QA_EXECSTACK QA_WXLOAD

scanelf.c:      qa_wx_load = get_split_env("QA_WX_LOAD");
scanelf.c:           "\t- 1 per QA_TEXTRELS/QA_EXECSTACK/QA_WX_LOAD");

------- Comment #51 From Kevin F. Quinn (RETIRED) 2006-05-24 00:14:43 0000 -------
Created an attachment (id=87360) [details]
Remove processing from ebuild.sh that's now done in scanelf (2.0), including
QA_WX_LOAD support

Sorry about that - also the two previous lines had WX_LOAD instead of
QA_WX_LOAD.

------- Comment #52 From Kevin F. Quinn (RETIRED) 2006-05-24 00:15:35 0000 -------
Created an attachment (id=87361) [details]
Remove processing from misc-functions.sh that's now done in scanelf (2.1),
including QA_WX_LOAD support

Same for 2.1

------- Comment #53 From solar 2006-05-24 03:44:25 0000 -------
(In reply to comment #52)
> Created an attachment (id=87361) [edit] [details]
> Remove processing from misc-functions.sh that's now done in scanelf (2.1),
> including QA_WX_LOAD support

commited to misc-functions.sh (hopefully last commit before 2.1 final)

With regards to 2.0.x we might as well leave it alone now that 2.1 in is the rc
phases

------- Comment #54 From solar 2006-05-24 06:28:05 0000 -------
Kevin,
Care to provide a man entry for expected usage? Maybe set some ground rules on
when it should and should not be used (to avoid abuse by lazy devs)

------- Comment #55 From Kevin F. Quinn (RETIRED) 2006-05-24 07:52:30 0000 -------
Created an attachment (id=87405) [details]
update man pages (and a little fix :/ )

How's this?  Additions to make.conf(5) and ebuild(5).  Adds documentation of
the 'stricter' FEATURE.

Also includes a little fix QA_STRICT_EXECSTACK => QA_STRICT_WX_LOAD, that I
noticed while writing the docs.

------- Comment #56 From Zac Medico 2006-05-24 14:55:26 0000 -------
It looks like solar committed this in svn r3409 and r3410.

------- Comment #57 From Zac Medico 2006-05-25 07:09:37 0000 -------
This has been released in 2.1_pre2-r3.

------- Comment #58 From Joël 2006-06-03 16:42:31 0000 -------
I don't understand...

With portage 2.1_rc4 and pax-utils 0.1.13, my QA_TEXTRELS statements seem to
get ignored. I made an example ebuild, which only needs fglrx_dri.so (taken
from ati-drivers) copied into files/:

=============================
# Copyright 1999-2006 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $

DESCRIPTION=""
HOMEPAGE=""
SRC_URI=""

LICENSE=""
SLOT="0"
KEYWORDS="x86"
IUSE=""

DEPEND=""
RDEPEND=""

QA_TEXTRELS="/opt/test/fglrx_dri.so"
QA_TEXTREL_x86="/opt/test/fglrx_dri.so"

RESTRICT="nostrip"

src_install()
{
        insinto /opt/test
        doexe "${FILESDIR}/fglrx_dri.so"
}
========================

I still get: TEXTREL fglrx_dri.so

------- Comment #59 From Joël 2006-06-03 17:23:26 0000 -------
Ok, it seems I took something incorrect as a "reference ebuild", sorry.

As I see now, ati-drivers-8.25.18 uses pathnames in "QA_" variables, and that
doesn't work.

------- Comment #60 From solar 2006-06-03 17:25:13 0000 -------
Drop the leading / The path is relative to the image directory. 
Also don't define both QA_FOO and QA_FOO_${ARCH}

------- Comment #61 From Joël 2006-06-03 17:43:57 0000 -------
Yes, I'm aware that QA_FOO_${ARCH} just overrides QA_FOO... but I was trying
various things for a moment :-)

These are working:

QA_TEXTRELS="fglrx_dri.so"
QA_TEXTRELS="*"

But really, this doesn't work:

QA_TEXTRELS="opt/test/fglrx_dri.so"

First Last Prev Next    No search results available      Search page      Enter new bug