Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 685160 (colon-in-CFLAGS) - [TRACKER] ebuilds using colon (:) as a sed delimiter break on CFLAGS=-falign-functions=32:25:16
Summary: [TRACKER] ebuilds using colon (:) as a sed delimiter break on CFLAGS=-falign-...
Status: CONFIRMED
Alias: colon-in-CFLAGS
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All All
: Normal normal with 1 vote (vote)
Assignee: Gentoo Toolchain Maintainers
URL:
Whiteboard:
Keywords: Tracker
Depends on: 710332 710420 710428 710486 710606 710608 710896 710958 710968 711032 711156 711346 736988 755599 794763 795744 802291 806088 813849 903965 914274 928906 685166 685168 685170 710336 710338 710340 710342 710346 710348 710424 710430 710446 710456 710472 710494 710520 710530 710550 710554 710556 710562 710580 710592 710600 710626 710696 710700 710702 710706 710708 710710 710714 710716 710722 710798 710804 710806 710810 710812 710816 710818 710820 710822 710826 710876 710880 710886 710890 710894 710898 710916 710950 710952 710954 710956 710960 710964 710970 710972 710974 711034 711036 711048 711050 711088 711110 711246 711344 711354 711408 711854 711856 712024 712646 712986 713002 713436 713602 714666 715706 715738 717016 718648 726610 733160 739222 832165 872650 873988
Blocks:
  Show dependency tree
 
Reported: 2019-05-06 05:05 UTC by wolfwood
Modified: 2024-04-07 20:54 UTC (History)
7 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description wolfwood 2019-05-06 05:05:51 UTC
with gcc 9.1 one can now use -falign-functions=32:25:16 to say align to a 32 byte boundary, unless you move more than 24 bytes, then align to a 16 byte boundary. same for -falign-jumps, -falign-labels, -falign-loops.

adding that to CFLAGS, however, causes emerging a number of packages to fail during src_prepare because they have sed oneliners that use : to delimit substitution:

>>> Preparing source in /var/tmp/portage/sys-apps/iproute2-5.0.0/work/iproute2-5.0.0 ...
 * Applying iproute2-3.1.0-mtu.patch ...
 [ ok ]
 * Applying iproute2-4.20.0-configure-nomagic.patch ...
 [ ok ]
sed: -e expression #3, char 152: unknown option to `s'
 * ERROR: sys-apps/iproute2-5.0.0::gentoo failed (prepare phase):
 *   (no error message)
 * 
 * Call stack:
 *     ebuild.sh, line 124:  Called src_prepare
 *   environment, line 3506:  Called die
 * The specific snippet of code:
 *       sed -i -e '/^CC :\?=/d' -e "/^LIBDIR/s:=.*:=/$(get_libdir):" -e "s:-O2:${CFLAGS} ${CPPFLAGS}:" -e "/^HOSTCC/s:=.*:= $(tc-getBUILD_CC):" -e "/^DBM_INCLUDE/s:=.*:=${T}:" Makefile || die;


so far, I've found:

app-arch/p7zip-16.02-r4
sys-apps/iproute2-5.0.0
dev-qt/qtcore-5.12.3
dev-qt/qtnetwork-5.12.3
dev-qt/qtsql-5.12.3
dev-qt/qttest-5.12.3
dev-qt/qtdbus-5.12.3
dev-qt/qtconcurrent-5.12.3
dev-qt/qtgui-5.12.3-r1
dev-qt/qtwidgets-5.12.3                    
dev-qt/qtprintsupport-5.12.3
Comment 1 Ulrich Müller gentoo-dev 2019-05-06 07:09:36 UTC
Not a QA issue. Reassigning.
Comment 2 Sergei Trofimovich (RETIRED) gentoo-dev 2019-05-06 07:18:28 UTC
Looking at iproute2-5.0.0 I would say it's not a toolchain specific bug.

A problem is in the ebuild itself. Not how sed call does not escape ${CFLAGS}:

    src_prepare() {
        ....
        sed -i \
                -e '/^CC :\?=/d' \
                -e "/^LIBDIR/s:=.*:=/$(get_libdir):" \
                -e "s:-O2:${CFLAGS} ${CPPFLAGS}:" \
                -e "/^HOSTCC/s:=.*:= $(tc-getBUILD_CC):" \
                -e "/^DBM_INCLUDE/s:=.*:=${T}:" \
                Makefile || die
        ....
    }

ulm@, why it's not a QA issue?
Comment 3 Sergei Trofimovich (RETIRED) gentoo-dev 2019-05-06 07:47:11 UTC
> so far, I've found:
> 
> app-arch/p7zip-16.02-r4

Filed bug #685166

> sys-apps/iproute2-5.0.0

Filed bug #685168

> dev-qt/qtcore-5.12.3
> dev-qt/qtnetwork-5.12.3
> dev-qt/qtsql-5.12.3
> dev-qt/qttest-5.12.3
> dev-qt/qtdbus-5.12.3
> dev-qt/qtconcurrent-5.12.3
> dev-qt/qtgui-5.12.3-r1
> dev-qt/qtwidgets-5.12.3                    
> dev-qt/qtprintsupport-5.12.3

Filed bug #685170

Please file new failures as individual bugs blocked against this bug.
Comment 4 Sergei Trofimovich (RETIRED) gentoo-dev 2019-05-06 08:03:50 UTC
For those who are here for a hint to fix it using '|' sounds like a safer delimiter.

Example iproute the fix would be:

@@ -64 +64 @@ src_prepare() {
-               -e "s:-O2:${CFLAGS} ${CPPFLAGS}:" \
+               -e "s|-O2|${CFLAGS} ${CPPFLAGS}|" \
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-05-06 08:53:16 UTC
In some cases a better fix would be to stop putting flags inline and instead adding variables to Makefiles.
Comment 6 Ulrich Müller gentoo-dev 2019-05-06 10:57:02 UTC
(In reply to Sergei Trofimovich from comment #2)
> ulm@, why it's not a QA issue?

This bug is blocking the gcc-9 tracker, so IMHO it should be assigned to toolchain. Also, using : as a separator in sed is not a QA issue per se.
Comment 7 Sergei Trofimovich (RETIRED) gentoo-dev 2019-05-06 16:20:34 UTC
(In reply to Ulrich Müller from comment #6)
> (In reply to Sergei Trofimovich from comment #2)
> > ulm@, why it's not a QA issue?
> 
> This bug is blocking the gcc-9 tracker, so IMHO it should be assigned to
> toolchain.

gcc-9 is not the first version that has options with colons. I dropped it from blocker.

> Also, using : as a separator in sed is not a QA issue per se.

The bug is about the ebuild failure in src_prepare() that is relatively easy to find. If qa@ does not plan to expand the tools or documentation to detect that kind of breakage it's fine by me.
Comment 8 Sergei Trofimovich (RETIRED) gentoo-dev 2019-11-24 23:24:11 UTC
If someone wants a few more commits into ::gentoo there are a lot more probken packages in the tree like that:
  $ git grep 's:' | fgrep CFLAGS | fgrep sed | wc -l
  130

A few selected examples:
  $ git grep 's:' | fgrep CFLAGS | fgrep sed

...
app-cdr/xbiso/xbiso-0.6.1-r2.ebuild:	sed -i -e 's:C) $(CFLAGS):C) $(LDFLAGS) $(CFLAGS):' Makefile.in || die #337769
app-cdr/xbiso/xbiso-0.6.1-r3.ebuild:	sed -i -e 's:C) $(CFLAGS):C) $(LDFLAGS) $(CFLAGS):' Makefile.in || die #337769
app-crypt/md4sum/md4sum-0.02.03-r2.ebuild:	sed -i -e "s:CFLAGS=:CFLAGS=${CFLAGS} :g" \
app-crypt/seahorse-sharing/seahorse-sharing-3.8.0_p20151117.ebuild:	sed -e 's:$CFLAGS -g -O0:$CFLAGS:' \
app-editors/elvis/elvis-2.2.0-r6.ebuild:	sed -i -e "s:gcc -O2:$(tc-getCC) ${CFLAGS}:" Makefile || die "sed 1 failed"
app-editors/elvis/elvis-2.2.0-r7.ebuild:	sed -i -e "s:gcc -O2:$(tc-getCC) ${CFLAGS}:" Makefile || die "sed 1 failed"
...
Comment 9 Michael Orlitzky gentoo-dev 2020-04-09 01:21:52 UTC
(In reply to Michał Górny from comment #5)
> In some cases a better fix would be to stop putting flags inline and instead
> adding variables to Makefiles.

In almost all cases this is the right thing to do. Instead of sed'ing the contents of the $CFLAGS variable into the target, the variable reference itself should be inserted into the target (if the target is shell, or a Makefile, or something that understands variables).

In other words, instead of creating a file that does

  $(CC) -march=native -O2 -pipe ...

we should be creating one that does

  $(CC) $(CFLAGS) ...
Comment 10 Larry the Git Cow gentoo-dev 2021-03-29 20:44:04 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/devmanual.git/commit/?id=0b00ff1312e734ae453a9f61f504bdc400b2b2df

commit 0b00ff1312e734ae453a9f61f504bdc400b2b2df
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2021-03-20 14:48:24 +0000
Commit:     Ulrich Müller <ulm@gentoo.org>
CommitDate: 2021-03-29 20:41:37 +0000

    tools-reference/sed: emphasise need for delimiter care
    
    Bug: https://bugs.gentoo.org/685160
    Signed-off-by: Sam James <sam@gentoo.org>
    Signed-off-by: Ulrich Müller <ulm@gentoo.org>

 tools-reference/sed/text.xml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)