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

Management of GNU installation vars in CMake and Autotools #3599

Merged
merged 15 commits into from Oct 8, 2018
@@ -9,6 +9,7 @@
build_type_flags, libcxx_flag, build_type_define,
libcxx_define, pic_flag, rpath_flags)
from conans.client.build.cppstd_flags import cppstd_flag
from conans.model.build_info import DEFAULT_BIN, DEFAULT_LIB, DEFAULT_INCLUDE, DEFAULT_RES
from conans.client.tools.oss import OSInfo
from conans.client.tools.win import unix_path
from conans.tools import (environment_append, args_to_string, cpu_count, cross_building,
@@ -95,7 +96,7 @@ def _get_host_build_target_flags(self):
return build, host, None

def configure(self, configure_dir=None, args=None, build=None, host=None, target=None,
pkg_config_paths=None, vars=None):
pkg_config_paths=None, vars=None, default_install_dirs=True):

This comment has been minimized.

Copy link
@jgsogo

jgsogo Oct 8, 2018

Member

default_install_dirs > use_default_install_dirs, I think it is more explicit if it is a True/False parameter.

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 8, 2018

Author Member

Thought about it but I think it is too long, however the boolean type could be more easily inferred

"""
:param pkg_config_paths: Optional paths to locate the *.pc files
:param configure_dir: Absolute or relative path to the configure script
@@ -106,10 +107,10 @@ def configure(self, configure_dir=None, args=None, build=None, host=None, target
"False" skips the --target flag
When the tool chain generates executable program, in which target system
the program will run.
:return: None
http://jingfenghanmax.blogspot.com.es/2010/09/configure-with-host-target-and-build.html
https://gcc.gnu.org/onlinedocs/gccint/Configure-Terms.html
:param default_install_dirs: Use or not the defaulted installation dirs
"""
if not self._conanfile.should_configure:
@@ -146,11 +147,20 @@ def configure(self, configure_dir=None, args=None, build=None, host=None, target
if self._conanfile.package_folder is not None:
if not args:
args = ["--prefix=%s" % self._conanfile.package_folder.replace("\\", "/")]
elif not any(["--prefix=" in arg for arg in args]):
elif not self._is_flag_in_args("prefix", args):
args.append("--prefix=%s" % self._conanfile.package_folder.replace("\\", "/"))

if not any(["--libdir=" in arg for arg in args]):
args.append("--libdir=${prefix}/lib")
if default_install_dirs:
for varname in ["bindir", "sbin", "libexec"]:
if not self._is_flag_in_args(varname, args):
args.append("--%s=${prefix}/%s" % (varname, DEFAULT_BIN))
if not self._is_flag_in_args("libdir", args):
args.append("--libdir=${prefix}/%s" % DEFAULT_LIB)
for varname in ["includedir", "oldincludedir"]:
if not self._is_flag_in_args(varname, args):
args.append("--%s=${prefix}/%s" % (varname, DEFAULT_INCLUDE))
if not self._is_flag_in_args("datarootdir", args):
args.append("--datarootdir=${prefix}/%s" % DEFAULT_RES)

with environment_append(pkg_env):
with environment_append(vars or self.vars):
@@ -167,6 +177,11 @@ def _adjust_path(self, path):
path = unix_path(path, path_flavor=self.subsystem)
return '"%s"' % path if " " in path else path

@staticmethod
def _is_flag_in_args(varname, args):
flag = "--%s=" % varname
return any([flag in arg for arg in args])

def make(self, args="", make_program=None, target=None, vars=None):
if not self._conanfile.should_build:
return
@@ -6,6 +6,7 @@
from conans.client.build.cppstd_flags import cppstd_flag
from conans.client.tools import cross_building
from conans.client.tools.oss import get_cross_building_settings
from conans.model.build_info import DEFAULT_BIN, DEFAULT_LIB, DEFAULT_INCLUDE, DEFAULT_RES
from conans.errors import ConanException
from conans.util.env_reader import get_env
from conans.util.log import logger
@@ -277,6 +278,13 @@ def get_definitions(self):
try:
if self._conanfile.package_folder:
ret["CMAKE_INSTALL_PREFIX"] = self._conanfile.package_folder
ret["CMAKE_INSTALL_BINDIR"] = DEFAULT_BIN
ret["CMAKE_INSTALL_SBINDIR"] = DEFAULT_BIN
ret["CMAKE_INSTALL_LIBEXECDIR"] = DEFAULT_BIN
ret["CMAKE_INSTALL_LIBDIR"] = DEFAULT_LIB
ret["CMAKE_INSTALL_INCLUDEDIR"] = DEFAULT_INCLUDE
ret["CMAKE_INSTALL_OLDINCLUDEDIR"] = DEFAULT_INCLUDE
ret["CMAKE_INSTALL_DATAROOTDIR"] = DEFAULT_RES
except AttributeError:
pass

@@ -6,7 +6,7 @@
from conans import tools
from conans.client.build.autotools_environment import AutoToolsBuildEnvironment
from conans.client.tools.oss import cpu_count
from conans.model.ref import ConanFileReference
from conans.model.ref import ConanFileReference, PackageReference
from conans.model.settings import Settings
from conans.paths import CONANFILE
from conans.test.build_helpers.cmake_test import ConanFileMock
@@ -514,43 +514,58 @@ def test_make_targets_install(self):
ab.install()
self.assertEquals(runner.command_called, "make install -j%s" % cpu_count())

def autotools_prefix_libdir_test(self):
def autotools_install_dirs_test(self):
runner = RunnerMock()
conanfile = MockConanfile(MockSettings({}), None, runner)
# Package folder is not defined
ab = AutoToolsBuildEnvironment(conanfile)
ab.configure()
self.assertNotIn("--prefix", runner.command_called)
self.assertNotIn("--bindir", runner.command_called)
self.assertNotIn("--libdir", runner.command_called)
self.assertNotIn("--includedir", runner.command_called)
self.assertNotIn("--dataroot", runner.command_called)
# package folder defined
conanfile.package_folder = "/package_folder"
ab.configure()
if platform.system() == "Windows":
self.assertIn("./configure --prefix=/package_folder --libdir=${prefix}/lib",
runner.command_called)
self.assertIn("./configure --prefix=/package_folder --bindir=${prefix}/bin "
"--sbin=${prefix}/bin --libexec=${prefix}/bin --libdir=${prefix}/lib "
"--includedir=${prefix}/include --oldincludedir=${prefix}/include "
"--datarootdir=${prefix}/res", runner.command_called)
else:
self.assertIn("./configure '--prefix=/package_folder' '--libdir=${prefix}/lib'",
runner.command_called)
self.assertIn("./configure '--prefix=/package_folder' '--bindir=${prefix}/bin' "
"'--sbin=${prefix}/bin' '--libexec=${prefix}/bin' '--libdir=${prefix}/lib' "
"'--includedir=${prefix}/include' '--oldincludedir=${prefix}/include' "
"'--datarootdir=${prefix}/res'", runner.command_called)
# --prefix already used in args
ab.configure(args=["--prefix=/my_package_folder"])
self.assertIn("--prefix=/my_package_folder", runner.command_called)
self.assertNotIn("--prefix=/package_folder", runner.command_called)
# --bindir, --libdir, --includedir already used in args
ab.configure(args=["--bindir=/pf/superbindir", "--libdir=/pf/superlibdir",
"--includedir=/pf/superincludedir"])
self.assertNotIn("--bindir=${prefix}/bin", runner.command_called)
self.assertNotIn("--libdir=${prefix}/lib", runner.command_called)
self.assertNotIn("--includedir=${prefix}/lib", runner.command_called)
if platform.system() == "Windows":
self.assertIn("./configure --prefix=/my_package_folder --libdir=${prefix}/lib",
self.assertIn("./configure --bindir=/pf/superbindir --libdir=/pf/superlibdir "
"--includedir=/pf/superincludedir --prefix=/package_folder "
"--sbin=${prefix}/bin --libexec=${prefix}/bin "
"--oldincludedir=${prefix}/include --datarootdir=${prefix}/res",
runner.command_called)
self.assertNotIn("--prefix=/package_folder", runner.command_called)
else:
self.assertIn("./configure '--prefix=/my_package_folder' '--libdir=${prefix}/lib'",
self.assertIn("./configure '--bindir=/pf/superbindir' '--libdir=/pf/superlibdir' "
"'--includedir=/pf/superincludedir' '--prefix=/package_folder' "
"'--sbin=${prefix}/bin' '--libexec=${prefix}/bin' "
"'--oldincludedir=${prefix}/include' '--datarootdir=${prefix}/res'",
runner.command_called)
self.assertNotIn("'--prefix=/package_folder'", runner.command_called)
# --libdir already used in args
ab.configure(args=["--libdir=/my_package_folder/superlibdir"])
if platform.system() == "Windows":
self.assertIn("./configure --libdir=/my_package_folder/superlibdir "
"--prefix=/package_folder", runner.command_called)
self.assertNotIn("--libdir=${prefix}/lib", runner.command_called)
else:
self.assertIn("./configure '--libdir=/my_package_folder/superlibdir' "
"'--prefix=/package_folder'", runner.command_called)
self.assertNotIn("'--libdir=${prefix}/lib'", runner.command_called)
# opt-out from default installation dirs
ab.configure(default_install_dirs=False)
self.assertIn("--prefix=/package_folder", runner.command_called)
self.assertNotIn("--bindir=${prefix}/bin", runner.command_called)
self.assertNotIn("--libdir=${prefix}/lib", runner.command_called)
self.assertNotIn("--includedir=${prefix}/lib", runner.command_called)

def autotools_configure_vars_test(self):
from mock import patch
@@ -621,3 +636,86 @@ def autotools_fpic_test(self):
self.assertFalse(ab.fpic)
ab.fpic = True
self.assertIn("-fPIC", ab.vars["CXXFLAGS"])

@unittest.skipUnless(platform.system() == "Linux", "Requires make")
def autotools_real_install_dirs_test(self):
body = r"""#include "hello.h"
#include <iostream>
using namespace std;
void hello()
{
cout << "Hola Mundo!";
}
"""
header = """
#pragma once
void hello();
"""
main = """
#include "hello.h"
int main()
{
hello();
return 0;
}
"""
conanfile = """
from conans import ConanFile, AutoToolsBuildEnvironment, tools
class TestConan(ConanFile):
name = "test"
version = "1.0"
settings = "os", "compiler", "arch", "build_type"
exports_sources = "*"
def build(self):
makefile_am = '''
bin_PROGRAMS = main
lib_LIBRARIES = libhello.a
libhello_a_SOURCES = hello.cpp
main_SOURCES = main.cpp
main_LDADD = libhello.a
'''
configure_ac = '''
AC_INIT([main], [1.0], [luism@jfrog.com])
AM_INIT_AUTOMAKE([-Wall -Werror foreign])
AC_PROG_CXX
AC_PROG_RANLIB
AM_PROG_AR
AC_CONFIG_FILES([Makefile])
AC_OUTPUT
'''
tools.save("Makefile.am", makefile_am)
tools.save("configure.ac", configure_ac)
self.run("aclocal")
self.run("autoconf")
self.run("automake --add-missing --foreign")
autotools = AutoToolsBuildEnvironment(self)
autotools.configure()
autotools.make()
autotools.install()
def package_id(self):
# easier to have same package_id for the test
self.info.header_only()
"""
client = TestClient()
client.save({"conanfile.py": conanfile,
"main.cpp": main,
"hello.h": header,
"hello.cpp": body})
client.run("create . danimtb/testing")
pkg_path = client.client_cache.package(
PackageReference.loads(
"test/1.0@danimtb/testing:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9"))

[self.assertIn(folder, os.listdir(pkg_path)) for folder in ["lib", "bin"]]

new_conanfile = conanfile.replace("autotools.configure()",
"autotools.configure(args=['--bindir=${prefix}/superbindir', '--libdir=${prefix}/superlibdir'])")
client.save({"conanfile.py": new_conanfile})
client.run("create . danimtb/testing")
[self.assertIn(folder, os.listdir(pkg_path)) for folder in ["superlibdir", "superbindir"]]
[self.assertNotIn(folder, os.listdir(pkg_path)) for folder in ["lib", "bin"]]
@@ -1003,6 +1003,32 @@ def test_cmake_system_version_android(self):
cmake = CMake(conan_file)
self.assertEquals(cmake.definitions["CMAKE_SYSTEM_VERSION"], "32")

def install_definitions_test(self):
conanfile = ConanFileMock()
conanfile.package_folder = None
conanfile.settings = Settings.loads(default_settings_yml)
install_defintions = {"CMAKE_INSTALL_PREFIX": conanfile.package_folder,
"CMAKE_INSTALL_BINDIR": "bin",
"CMAKE_INSTALL_SBINDIR": "bin",
"CMAKE_INSTALL_SBINDIR": "bin",
"CMAKE_INSTALL_LIBEXECDIR": "bin",
"CMAKE_INSTALL_LIBDIR": "lib",
"CMAKE_INSTALL_INCLUDEDIR": "include",
"CMAKE_INSTALL_OLDINCLUDEDIR": "include",
"CMAKE_INSTALL_DATAROOTDIR": "res"}

This comment has been minimized.

Copy link
@SSE4

SSE4 Oct 3, 2018

Contributor

any reason to use "res" instead of "share" for datarootdir? I would expect it will require lots of programs to change location then

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 7, 2018

Author Member

"res" is the default folder name for resources and data files in self.cpp_info https://docs.conan.io/en/latest/reference/conanfile/methods.html#cpp-info

This comment has been minimized.

Copy link
@SSE4

SSE4 Oct 8, 2018

Contributor

in this case, may be it's better to add "shared" as a secondary default in addition to the existing "res"? "res" sounds to be too foreign for autotools-based installations.

This comment has been minimized.

Copy link
@wittmeie

wittmeie Oct 9, 2018

Often CMake package-config files are installed to /share
install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config.cmake ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config-version.cmake DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/cmake/${PROJECT_NAME} )

With this change, they are installed to /res which is not a default search path of the CMake find_package command: https://cmake.org/cmake/help/v3.12/command/find_package.html?highlight=find_package

Hence, packages are not found anymore if included via conan_basic_setup()!!!

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 10, 2018

Author Member

You were right, I am proposing a fix for this in #3705

This comment has been minimized.

Copy link
@wittmeie

wittmeie Oct 10, 2018

Great - many thanks!


# Without package_folder
cmake = CMake(conanfile)
for key, value in cmake.definitions.items():
self.assertNotIn(key, install_defintions.keys())

# With package_folder
conanfile.package_folder = "my_package_folder"
install_defintions["CMAKE_INSTALL_PREFIX"] = conanfile.package_folder
cmake = CMake(conanfile)
for key, value in install_defintions.items():
self.assertEquals(cmake.definitions[key], value)

@mock.patch('platform.system', return_value="Macos")
def test_cmake_system_version_osx(self, _):
settings = Settings.loads(default_settings_yml)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.