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

[bug] install method of conan.tools.meson.Meson calls meson setup --reconfigure before installing #11910

Closed
SpaceIm opened this issue Aug 18, 2022 · 5 comments
Assignees
Milestone

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 18, 2022

Environment Details (include every applicable attribute)

  • Operating System+version: macOS Monterey
  • Compiler+version: AppleClang 13
  • Conan version: 1.51.2
  • Python version: 3.10.6

Steps to reproduce (Include if Applicable)

During install step, an unnecessary reconfiguration is executed (it can be quite long depending on the project). So the behavior differs from CMake helper where no reconfiguration is done when install method is called.

Logs (Executed commands with output) (Include/Attach if Applicable)

https://c3i.jfrog.io/c3i/misc/logs/pr/12300/1-linux-gcc/dav1d/1.0.0//79ab1d7c838f7fd4d71114687478abc26e7bc7ef-build.txt

@franramirez688
Copy link
Contributor

Hi @SpaceIm

As you can read in this comment #11920 (comment), there is nothing to improve here because the package_folder can change (depending on the local flow), and it has to be passed to the install always, so that reconfigure is necessary...

(Suggestion/Question) Maybe, it could be better in these cases to use the copy() function instead to avoid that reconfiguration.

@eli-schwartz
Copy link

But according to the original report, the cmake helper does not need to do any such thing? Why not?

@jcar87
Copy link
Contributor

jcar87 commented Aug 22, 2022

But according to the original report, the cmake helper does not need to do any such thing? Why not?

CMake allows overriding the install prefix when calling the cmake --install command to perform the installation (docs here). This is what we use in the newer CMake integration.

With CMake-generated projects, one could also do make install/ninja install or cmake --build . --target install, and changing the CMAKE_INSTALL_PREFIX would require invoking CMake again to reconfigure and re-generate the project.

This is similar to defining the DESTDIR (docs here) environment variable when calling make install in GNU-compliant projects, so calling ./configure --prefix= is not needed.

One caveat that is worth mentioning, is that some projects take the value of the install prefix and embed it in compiled code - calling cmake --install . --prefix= or DESTDIR=xx make install will not cause the build tool to override what has already been compiled, whereas re-configuring the project with the new path would.

@eli-schwartz
Copy link

One caveat that is worth mentioning, is that some projects take the value of the install prefix and embed it in compiled code - calling cmake --install . --prefix= or DESTDIR=xx make install will not cause the build tool to override what has already been compiled, whereas re-configuring the project with the new path would.

That's exactly why DESTDIR exists, because prefix is semantically information that can be embedded in compiled code, and DESTDIR is, semantically, not allowed to be embedded in compiled code.

@franramirez688 franramirez688 added this to the 1.53 milestone Aug 30, 2022
@czoido czoido modified the milestones: 1.53, 1.54 Oct 4, 2022
@memsharded memsharded modified the milestones: 1.54, 1.55 Nov 2, 2022
@memsharded memsharded modified the milestones: 1.55, 1.56 Nov 28, 2022
@memsharded memsharded modified the milestones: 1.56, 1.57 Dec 20, 2022
@franramirez688 franramirez688 modified the milestones: 1.57, 1.58 Jan 11, 2023
@franramirez688
Copy link
Contributor

As far as I tested locally, adding the DESTDIR is not avoiding the reconfigure process, so I'm afraid we can't improve this now.

Note: another issue is to add that DESTDIR or the --destdir xxxx param (since Meson 0.57) in the Meson helper. That's a new issue and it could be done for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants