Skip to content

Add option to set exact install name for LibExeObjStep #9783

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

Closed
wants to merge 2 commits into from

Conversation

dweiller
Copy link
Contributor

This should close #2231.

This change adds an optional exact_install_filename field to LibExeObjStep that is used to set the exact filename when installing. Andrew suggested using -femit-bin in #6325, but this seems to move the file outside the cache if you just append something like -femit-bin=my-exact-bin-name to the compiler args. Not setting -femit-bin means that the contents of the cache should be unaffected by this change (i.e. the name in the cache is the standard one) and only the install location (as given by the InstallArtifactStep) is changed.

If exact_install_filename is set for a versioned dynamic library, the InstallArtifactStep for it will not install the versioned symlinks. I thought this would be the preferred behaviour - you are wanting non-standard filenames so the standard versioned symlinks are probably wrong anyway. The versioned symlinks do appear in the cache.

This allows setting the exact installed filename for a LibExeObjStep.
Versioned symlinks will not be made for a shared dynamic library that
has exact_install_filename set.
@ifreund
Copy link
Member

ifreund commented Sep 16, 2021

For reference, I solved my use case for #2231 with a custom install step: https://github.com/ifreund/rundird/blob/8a97db7d6d7a07cd0ed24fe0e9604360c4c715e4/build.zig#L41-L63

@dweiller
Copy link
Contributor Author

At the moment there is some non-declarative/order dependent interactions in the build system. At the moment install() needs to be called on a LibExeObjStep after anything that affects the where the file should be installed because the installation paths are set in the InstallArtifactStep when install() is called. Perhaps install() could instead cause the make() function of LibExeObjStep configure the InstallArtifactStep, and have InstallArtifactStep be a union of a configured and not-configured state.

I remember reading in either the source code, or an issue here some time ago about some other order-dependent behavior, but I cannot find it anymore - are there other order-dependent parts of the build system that could be cleaned up that anyone knows of? Perhaps this should be spun out as a separate issue to clean up any unwanted order-dependence in the build system, but I could also remove this interaction of install() in this PR.

Instead of configuring the paths of a ArtifactInstallStep when it is
created, defer this until the make() call of the associated
LibExeObjStep. This means that any steps that depend on the paths of the
installed files (even if they do not need the files to be installed)
should depend on the LibExeObjStep and do any processing with the
installation paths after the LibExeObjStep has been made.
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

Thanks for the patch.

Unfortunately I think this is a tricky enough case that we need a test for it to verify the behavior. I'm also a little unsure about the solution to install() being non-declarative. I want to investigate that myself.

I do apologize for the late review. Given that this is old, and there are now merge conflicts, I'm going to close this PR. If you are interested in pursuing this change, please open a new one, and I will make sure to give it more attention. Otherwise, sit tight, the issue #2231 is still open (although it is not set to a high priority milestone).

@andrewrk andrewrk closed this May 10, 2022
@dweiller dweiller deleted the exact-install-name branch June 21, 2023 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Way to set shared library filename
3 participants