Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler and linker flags ignored when building probe runner #104

Closed
gregoa opened this issue Jan 25, 2019 · 13 comments

Comments

@gregoa
Copy link

commented Jan 25, 2019

While updating the Debian package for FFI-Platypus, I noticed that probe/bin/dlrun gets built without the compiler/linker flags we set for package builds in the environment.

I now came up with a patch which collects the *FLAGS and uses them in lib/FFI/Probe/Runner/Builder.pm. Not sure if this is the best/correct method but it seems to work.

Alas no pull request as this is against 0.74 and doesn't apply cleanly against git HEAD. - The patch is here:
https://salsa.debian.org/perl-team/modules/packages/libffi-platypus-perl/raw/master/debian/patches/cc_flags.patch

Cheers,
gregor

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

Awesome, thanks for spotting this and reporting it! NP, this is probably enough for me to apply similar fixes to HEAD.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 25, 2019

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 26, 2019

Hrm. I may need some help in understanding how this problem manifests itself. MakeMaker does not typically respond to those environment variables, (at least not directly, I think Configure does when Perl is built), it instead uses $Config{ccflags} and $Config{ldflags}/$Config{lddlflags}, these are the same variables that I am using in the probe builder, and I would expect them to have the customized flags that Debian uses.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 26, 2019

Yeah, it's all a bit complicated :) EUMM can be fed these variables, e.g. when building FFI-Platypus I get

perl Makefile.PL INSTALLDIRS=vendor "OPTIMIZE=-g -O2 -fdebug-prefix-map=/build/libffi-platypus-perl-0.74=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2" "LD=x86_64-linux-gnu-gcc -g -O2 -fdebug-prefix-map=/build/libffi-platypus-perl-0.74=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now"

where parts are inserted by our build machinery (debhelper's perl_makemaker build system) from the environment. The environment looks something like

% dpkg-buildflags 
CFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong -Wformat -Werror=format-security
CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2
CXXFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong -Wformat -Werror=format-security
FCFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong
FFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong
GCJFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong
LDFLAGS=-Wl,-z,relro
OBJCFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong -Wformat -Werror=format-security
OBJCXXFLAGS=-g -O2 -fdebug-prefix-map=/home/gregoa/src/git-pkg-perl/meta/packages/libffi-platypus-perl=. -fstack-protector-strong -Wformat -Werror=format-security

and debhelper then does something like

push @flags, "OPTIMIZE=$ENV{CFLAGS} $ENV{CPPFLAGS}";
push @flags, "LD=$Config{ld} $ENV{CFLAGS} $ENV{LDFLAGS}";
doit("perl" … "Makefile.PL" … @flags);

The values from %Config are:

% perl -MConfig -E 'say $Config{ccflags}; say $Config{ldflags};'
-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
 -fstack-protector-strong -L/usr/local/lib

EUMM writes all kinds of things from %Config and from the invocation with OPTIMIZE and LD to the Makefile, and the actual compile and linking steps of the library look like

"/usr/bin/perl" "/usr/share/perl/5.28/ExtUtils/xsubpp"  -typemap '/usr/share/perl/5.28/ExtUtils/typemap' lib/FFI/Platypus.xs > lib/FFI/Platypus.xsc
x86_64-linux-gnu-gcc -c  -Iinclude -D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -g -O2 -fdebug-prefix-map=/build/libffi-platypus-perl-0.74=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/libffi-platypus-perl-0.74=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2   -DVERSION=\"0.74\" -DXS_VERSION=\"0.74\" -fPIC "-I/usr/lib/x86_64-linux-gnu/perl/5.28/CORE"   xs/closure.c -o xs/closure.o
…
x86_64-linux-gnu-gcc -g -O2 -fdebug-prefix-map=/build/libffi-platypus-perl-0.74=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now  -shared -L/usr/local/lib -fstack-protector-strong  lib/FFI/Platypus.o xs/closure.o xs/complex.o xs/custom.o xs/havepm.o xs/meta.o xs/names.o xs/perl_math_int64.o xs/record_opaque.o xs/record_simple.o xs/record_string.o xs/windl.o  -o blib/arch/auto/FFI/Platypus/Platypus.so  \
   -lffi   \

I don't know if there is a way to trick EUMM into writing a Makefile that builds "external" stuff like the probe runner in the same way as its "own" stuff, that's why I fiddled around with %ENV directly; maybe there are better ways …

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 26, 2019

Great thanks for the details. EUMM definitely ignores the environment variables, so we shouldn't be using that as the way to override, but your right it does allow you to override on the command line. I will dig a little further and see what I can figure out.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 26, 2019

Thanks!
I just remembered grep.metacpan.org and tried https://grep.metacpan.org/search?q=%5C%24ENV%5C%7B%5BA-Z%5D%2BFLAGS%5C%7D&qd=&qft=Makefile.PL
An interesting example is https://grep.metacpan.org/search?qci=&q=%5C%24ENV%5C%7B%5BA-Z%5D%2BFLAGS%5C%7D&qft=Makefile.PL&qd=CryptX&f=Makefile.PL with the extra_targetsat the bottom. Maybe some inspiration :)

@plicease plicease self-assigned this Jan 26, 2019

@plicease plicease added the 🐞Bug label Jan 26, 2019

plicease added a commit that referenced this issue Jan 26, 2019

@plicease plicease referenced this issue Jan 26, 2019
6 of 6 tasks complete
@plicease

This comment has been minimized.

Copy link
Member

commented Jan 28, 2019

@gregoa I believe #105 released as 0.77_01 should address this. If you can confirm or refute, please let me know.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 29, 2019

This looks very good, thank you! I see, with V=1, all flags in the cc calls.

There are just two minor (new) issues left: One is, to quote lintian (our package checker):

I: libffi-platypus-perl: file-references-package-build-path usr/lib/x86_64-linux-gnu/perl5/5.28/auto/share/dist/FFI-Platypus/probe/probe.pl
N: 
N:    The listed file or maintainer script appears to reference the build path
N:    used to build the package as specified in the Build-Path field of the
N:    .buildinfo file.
N:    
N:    This is likely to cause the package to be unreproducible, but it may
N:    also indicate that the package will not work correctly outside of the
N:    maintainer's own system.
N:    
N:    Refer to https://reproducible-builds.org/ and the dpkg-genbuildinfo(1)
N:    manual page for details.

And indeed, the generated probe.pl contains:

  'eumm' => {
    'cc' => [
      'x86_64-linux-gnu-gcc'
    ],
…
    'ld' => [
      'x86_64-linux-gnu-gcc',
      '-g',
      '-O2',
      '-fdebug-prefix-map=/build/libffi-platypus-perl-0.77.01=.',
      '-fstack-protector-strong',
      '-Wformat',
      '-Werror=format-security',
      '-Wl,-z,relro',
      '-Wl,-z,now'
    ],
…
    'optimize' => [
      '-g',
      '-O2',
      '-fdebug-prefix-map=/build/libffi-platypus-perl-0.77.01=.',
      '-fstack-protector-strong',
      '-Wformat',
      '-Werror=format-security',
      '-Wdate-time',
      '-D_FORTIFY_SOURCE=2'
    ]
  },

(i.e. /build/libffi-platypus-perl-0.77.01=. from ld and optimize). Not sure if this is a problem (probably not, as it's just documentation, and the build path is standardized in Debian), if this is necessary, if the probe.pl file is actually needed at runtime (or should we just not install it?).

The other point is autopkgtest, another Debian thing (sorry for all those idiosyncracies :)). In short these are tests against installed binary packages, and one test we do for perl modules is to simply run prove … t/ after copying t/ (and optionally other files) to a temporary directory (in order to make sure we're testing the installed modules). Now this was never easy with FFI-Platypus. In older versions, we had to setup the tests with

perl Build.PL
perl Build libtest

for 0.74 I've changed this to

perl Makefile.PL
make mymm_config
make mymm_test

Now the docs claim to use mymm-test but those targets don't exist anymore in the generated Makefile since b62dbe3 … (mm-{config,test} also doesn't work). -- Ah, I think I've found the new runes:

perl Makefile.PL
make config
make ffi-test

Ok, just a doc update then :)

Thanks again,
gregor

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

The stuff in the eumm key definitely won't be used at runtime, but some of the other keys have some things that I do intend on using in the future. I also have some items in config.pl that I'd like to move into probe.pl. (config.pl is intended for stuff that is used at runtime by Platypus and FFI modules that use it, and probe.pl will be useful for FFI modules using Platypus at their build time). When I refactor that I will keep what you say in mind see if we can install a paried down probe.pl which just has stuff needed at FFI module build time.

Fixed the doco here: 4d0416a, sorry for change, I think ffi-test is a less confusing.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 29, 2019

Thanks for the helpful explanation and the doc fix (and I agree that ffi-test is a better name).

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 30, 2019

I believe this to be resolved. I've opened #110 to track the planned refactor of config/probe.pl, including a note about the remaining unresolved issue you mentioned.

@plicease plicease closed this Jan 30, 2019

@plicease

This comment has been minimized.

Copy link
Member

commented Jan 30, 2019

This 404873d should let you run just make ffi-test without needing to (explicitly) run make config.

@gregoa

This comment has been minimized.

Copy link
Author

commented Jan 30, 2019

Nice, I thought that something like this should work but was too lazy to try :)
(And ack on closing this issue and keeping the remaining thought in a new one.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.