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.

Sign up for GitHub

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
+172 −25
Merged

Management of GNU installation vars in CMake and Autotools #3599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
83f1bea
Added --libdir parameter to autotools configure
danimtb Aug 22, 2018
5393688
Merge branch 'develop' into feature/3369_autotools_libdir
danimtb Aug 22, 2018
0e4c06f
Added test
danimtb Aug 22, 2018
31cd7c7
Merge branch 'feature/3369_autotools_libdir' into feature/3594
danimtb Sep 21, 2018
caff5ed
Added GNU installation vars in cmake and atools
danimtb Sep 21, 2018
c7c2e72
Added tests for atools
danimtb Sep 21, 2018
722deb1
added cmake install definitions test
danimtb Sep 21, 2018
2f0aea8
Merge branch 'develop' into feature/3594
danimtb Sep 24, 2018
04ef814
Included real autotools test
danimtb Sep 27, 2018
6ea7d4e
fix skipunless
danimtb Sep 27, 2018
cb36599
same package ID in test
danimtb Sep 27, 2018
432acba
remove repeted libdir adjustment as per review
danimtb Oct 1, 2018
f0008ed
Merge branch 'develop' into feature/3594
danimtb Oct 1, 2018
be0f168
opt-out for default installation dirs in autotools
danimtb Oct 8, 2018
13019bd
renamed to use_default_install_dirs
danimtb Oct 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions conans/client/build/autotools_environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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, use_default_install_dirs=True):
"""
:param pkg_config_paths: Optional paths to locate the *.pc files
:param configure_dir: Absolute or relative path to the configure script
Expand All @@ -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 use_default_install_dirs: Use or not the defaulted installation dirs

"""
if not self._conanfile.should_configure:
Expand Down Expand Up @@ -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 use_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):
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions conans/client/build/cmake_flags.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
138 changes: 118 additions & 20 deletions conans/test/build_helpers/autotools_configure_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(use_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
Expand Down Expand Up @@ -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"]]
26 changes: 26 additions & 0 deletions conans/test/build_helpers/cmake_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@danimtb danimtb Oct 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

@wittmeie wittmeie Oct 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Expand Down