Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 685160 (colon-in-CFLAGS)

Summary: [TRACKER] ebuilds using colon (:) as a sed delimiter break on CFLAGS=-falign-functions=32:25:16
Product: Gentoo Linux Reporter: wolfwood <jamesbuyacar>
Component: Current packagesAssignee: Gentoo Toolchain Maintainers <toolchain>
Status: CONFIRMED ---    
Severity: normal CC: ivan, jstein, lssndrbarbieri, mgorny, mjo, sam, ulm
Priority: Normal Keywords: Tracker
Version: unspecified   
Hardware: All   
OS: All   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on: 710332, 710420, 710428, 710456, 710486, 710520, 710606, 710608, 710626, 710822, 710876, 710896, 710958, 710968, 710970, 711032, 711036, 711156, 711246, 711346, 711856, 717016, 718648, 736988, 755599, 794763, 795744, 802291, 806088, 813849, 685166, 685168, 685170, 710336, 710338, 710340, 710342, 710346, 710348, 710424, 710430, 710446, 710472, 710494, 710530, 710550, 710554, 710556, 710562, 710580, 710592, 710600, 710696, 710700, 710702, 710706, 710708, 710710, 710714, 710716, 710722, 710798, 710804, 710806, 710810, 710812, 710816, 710818, 710820, 710826, 710880, 710886, 710890, 710894, 710898, 710916, 710950, 710952, 710954, 710956, 710960, 710964, 710972, 710974, 711034, 711048, 711050, 711088, 711110, 711344, 711354, 711408, 711854, 712024, 712646, 712986, 713002, 713436, 713602, 714666, 715706, 715738, 726610, 733160, 739222    
Bug Blocks:    

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(-)