search

conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.11k stars 963 forks source link

AutotoolsToolchain: support custom configure command #14551

Open fpelliccioni opened 1 year ago

fpelliccioni commented 1 year ago

What is your suggestion?

Currently, AutotoolsToolchain helper has method configure(), which always invokes hard-coded configure command. it works for 99% of projects, however, there are some exceptions:

I suggest to add new parameter configure_command, with default configure (as currently).

Related with: https://github.com/conan-io/conan/issues/4915

Have you read the CONTRIBUTING guide?

memsharded commented 1 year ago

Hi @fpelliccioni

Thanks for the suggestion. I see some problems here, this is the full implementation:

 script_folder = os.path.join(self._conanfile.source_folder, build_script_folder) \
            if build_script_folder else self._conanfile.source_folder

        configure_args = []
        configure_args.extend(args or [])

        self._configure_args = "{} {}".format(self._configure_args, cmd_args_to_string(configure_args))

        configure_cmd = "{}/configure".format(script_folder)
        subsystem = deduce_subsystem(self._conanfile, scope="build")
        configure_cmd = subsystem_path(subsystem, configure_cmd)
        cmd = '"{}" {}'.format(configure_cmd, self._configure_args)
        self._conanfile.output.info("Calling:\n > %s" % cmd)
        self._conanfile.run(cmd)

The openssl case actually runs perl Configure. This will not work with just defining that argument, because it will result in:

/path/to/script/perl Configure

Which doesn't make sense

Then also the configure_args are very likely that are pure autotools args, are we sure they will work with the Tensorflow configure.py? I see from https://www.tensorflow.org/install/source that both configure and configure.py are provided, not clear why one is better than the other, but also it seems that it is using Bazel and not autotools? Are we sure that the Autotools integration is the best for this package?

I am also curious why the ICU recipe in ConanCenter is not using the runConfigureICU apparently only:

 autotools = Autotools(self)
autotools.configure(build_script_folder=os.path.join(self.source_folder, "source"))

Taking everything into account, it seems that for these special cases it is better that recipes use a bare self.run(), because there can be as many corner cases to this as special cases out there, and we cannot go down the rabbit hole inside the Autotools.configure() for every one of them.

fpelliccioni commented 1 year ago

Well, I just needed for the Emscripten case, I copied the other cases from the original issue.