Build::SearchDep plugin appears to override compiler flags in share compilations

[shawnlaffan]

It seems that Alien::Build can override default compiler flags under autotools systems.

Relevant discussion is on the GDAL issue tracker: OSGeo/gdal#4706 (comment)

So far as I can tell the issue only occurs when Alien::Build::Plugin::SearchDep is used, as the behaviour is only evidenced on aliens that include it and which use autotools. I have observed it with Alien::gdal and Alien::proj, but not with Alien::libtiff or Alien::sqlite. Just as a data point, Alien::geos::af uses cmake and does not have this issue.

My current approach in the affected alienfiles is to add CFLAGS='-O2' CXXFLAGS='-O2' to the configure calls.

I am not sure what the best fix is, or even if there is such a thing given the default flags will vary by package. Probing the configure script might be feasible, but perhaps documenting this situation and the workaround is the simplest thing.

Possibly relevant code is:

Alien-Build/lib/Alien/Build/Plugin/Build/SearchDep.pm

Lines 73 to 115 in fdd350a

	$meta->around_hook(
	build => sub {
	my($orig, $build) = @_;
	local $ENV{CFLAGS} = $ENV{CFLAGS};
	local $ENV{CXXFLAGS} = $ENV{CXXFLAGS};
	local $ENV{LDFLAGS} = $ENV{LDFLAGS};
	tie my @CFLAGS, 'Env::ShellWords', 'CFLAGS';
	tie my @CXXFLAGS, 'Env::ShellWords', 'CXXFLAGS';
	tie my @LDFLAGS, 'Env::ShellWords', 'LDFLAGS';
	my $cflags = $build->install_prop->{plugin_build_searchdep_cflags} = [];
	my $ldflags = $build->install_prop->{plugin_build_searchdep_ldflags} = [];
	my $libs = $build->install_prop->{plugin_build_searchdep_libs} = [];
	foreach my $other (@aliens)
	{
	my $other_cflags;
	my $other_libs;
	if($other->install_type('share'))
	{
	$other_cflags = $other->cflags_static;
	$other_libs = $other->libs_static;
	}
	else
	{
	$other_cflags = $other->cflags;
	$other_libs = $other->libs;
	}
	unshift @$cflags, grep /^-I/, shellwords($other_cflags);
	unshift @$ldflags, grep /^-L/, shellwords($other_libs);
	unshift @$libs, grep /^-l/, shellwords($other_libs);
	}
	unshift @CFLAGS, @$cflags;
	unshift @CXXFLAGS, @$cflags;
	unshift @LDFLAGS, @$ldflags;
	$orig->($build);
	},
	);

[plicease]

I think unfortunately this is a problem with autotools. If a dep requires a library but doesn't use pkg-config, often the only way of adding it is often using CFLAGS etc., and setting those can override behaviors in the package. The Build::SearchDep plugin goes to pains to make sure that it is adding to existing environment variables, but there is no way of reliably knowing what the defaults are for the alienized package, and -O2 is probably fine for the most common platforms, but may not be right for some weird ones.

[shawnlaffan]

In the case of GDAL, it does not use any environment variables by default but the env vars do override the defaults when present. The -O2 is set somewhere in the build system at configure time (I cannot find exactly where at the moment).

A doc patch seems the simplest approach. How about something like this added to the POD? Possibly in the description section, but perhaps in a distinct caveats section.

Note that this will override any of the default flags for the alien being compiled, including optimisation flags.  
To ensure they are used you can add them to the configure call in your alienfile, for example:

    "%{configure} CFLAGS='-O2' CXXFLAGS='-O2'"

I can work up a PR if you'd like.

[plicease]

Yeah I was thinking a CAVEAT, but we can put it in the DESCRIPTION section. I think one thing that we need to be careful of is qualifying the issue. I'm thinking the language should be more like:

Note that this may override any of the default flags for the alien being compiled in some build systems (autoconf is particularly susceptible), including optimization flags.

"%{configure} CFLAGS='-O2' CXXFLAGS='-O2'"

Will this override local environment variables? ie, if a locally I have CFALGS set to -g will I get -g or -O2? I would want a solution that allows prefers the local users environment variables if they have set them.

One way is that we could provide a default value for these environment variables if they are not set. That way if you know -O2 is appropriate for this package then we can default to that but still allow user to set CFLAGS to -g or whatnot. I'm not sure that should be done in this plugin though as there may be other places that we want to modify these variables.

[plicease]

Basically we want to be able to say

$ENV{CFLAGS} = '-O3' unless defined $ENV{CFLAGS};

except just around the execution of the build.

[shawnlaffan]

If I read the plugin code correctly, it already appends to existing env vars. The issue with the gdal and proj builds is that they are set somewhere in the configure system and are overridden by any env vars.

I'm not familiar enough with autotools to know how passing CFLAGS and CXXFLAGS args to configure interacts with user defined env vars.

[plicease]

If I read the plugin code correctly, it already appends to existing env vars. The issue with the gdal and proj builds is that they are set somewhere in the configure system and are overridden by any env vars.

Right but if we set the value to what we expect the default should be for the package if (and only if) the variable is not set, then we effectively get the same default as the alienized package, but also allow the user to specify -g instead of -O2.

I'm not familiar enough with autotools to know how passing CFLAGS and CXXFLAGS args to configure interacts with user defined env vars.

Need to experiment :)

[plicease]

I'm not sure that should be done in this plugin though as there may be other places that we want to modify these variables.

The autoconf plugin might be the right place to do this actually.

[shawnlaffan]

Right but if we set the value to what we expect the default should be for the package if (and only if) the variable is not set, then we effectively get the same default as the alienized package, but also allow the user to specify -g instead of -O2.

Any such settings are probably best done in the relevant alienfile given the potential variability of default flags. Or I am misunderstanding this point.

Need to experiment :)

I was hoping to avoid that :) Will give it a poke.

[plicease]

Any such settings are probably best done in the relevant alienfile given the potential variability of default flags. Or I am misunderstanding this point.

No, that is exactly the point, I was musing on the best way for AB to do it once the alienfile tells it what the default should be. Does that make sense?

[shawnlaffan]

I've experimented with Alien::proj, and passing CFLAGS and CXXFLAGS args to the configure call does override any env vars.

Any alienfile is thus best to append or prepend these to the relevant env vars, as you suggested in #293 (comment)

Or more simply for the alienfile, they could be passed as arguments to the plugin.

There is of course the issue of the alienfile getting out of synch with the package being built, but that could be handled via a doc note for Alien maintainers.

[plicease]

I've experimented with Alien::proj, and passing CFLAGS and CXXFLAGS args to the configure call does override any env vars.

That reminds me we should document CXXFLAGS I think only CFLAGS and LIBS are mentioned in the documentation.

There is of course the issue of the alienfile getting out of synch with the package being built, but that could be handled via a doc note for Alien maintainers.

Yeah unfortunately unavoidable with the way that autoconf is structured.

[shawnlaffan]

Just a note that some Alien packages don't work with "%{configure} CFLAGS='-O2' CXXFLAGS='-O2'" (Alien::spatialite so far).

I ended up using an around hook for that one.

For others coming along later:

share {
  #  usual code
  ...

  meta->around_hook( build => \&set_compiler_flags );

  build [
    ...
  ]
}

sub set_compiler_flags {
  my ($orig, $build, @args) = @_;

  local $ENV{CFLAGS}   = '-O2 ' . ($ENV{CFLAGS} // '');
  local $ENV{CXXFLAGS} = '-O2 ' . ($ENV{CXXFLAGS} // '');

  $build->log ("Setting compiler flag env vars to -O2");

  $orig->($build, @args);
}