-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
build: Support 'rename' in configure_file, build_target and custom_target #13960
base: master
Are you sure you want to change the base?
Conversation
a165c27
to
6ff146a
Compare
@@ -3482,6 +3488,11 @@ def build_target(self, node: mparser.BaseNode, args: T.Tuple[str, SourcesVarargs | |||
target = targetclass(name, self.subdir, self.subproject, for_machine, srcs, struct, objs, | |||
self.environment, self.compilers[for_machine], kwargs) | |||
|
|||
if len(kwargs['rename']) not in {0, 1, len(target.outputs)}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still have the soname issue that was mentioned in earlier attempts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. I don't need that case to work for my projects, and we have had zero traction in getting build subdirectory support merged in meson, so I'm trying a simpler patch to see if I can at least get my use case working without thousands (literally) of warnings. To fix the soname issue, it might be easier to fix up the existing -soname
code paths to permit a custom name for that too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that my alternative plan, #14002, does not suffer from this problem.
b525553
to
1b8f0e5
Compare
f51c06d
to
fc55a87
Compare
In getting the tests working, I discovered that shared libraries can generate additional files, like the Windows import library, which aren't listed in the outputs array. As you can see from this test.json file, those get installed using the original names, without any rename support. I could:
Other than this issue, I think this MR is ready to go; it's passing all of the tests (save for a couple which seem to just have CI issues?). |
I missed a case that I need -- configure_file. I've added another commit which fixes that. |
I believe that the additional windows files the |
If so, then just taking the |
Hmmm... IIUC, then we would just need to install the .lib and .pdb with the same name but .pdb, though maybe we have someone with more Windows expertise? skipping Windows might be a possibility if getting it working is too hard, but I'd like to at least try to get it working. It would also need to exclude Vala because there are implicit outputs from that too. |
1f2d77c
to
7046e5b
Compare
"it's complicated". Cygwin uses a 'cyg' prefix rather than 'lib', while still using '.dll.a' for various reasons.
Instead of attempting to automate this renaming, I've just added two new keyword args, 'import_rename' and 'debug_rename' that allow the user to explicitly specify the install names for those two files. This ensures that the
Vala uses the existing 'rename' mechanism as it extends the 'output' array with the new files. That means the user will A couple of options I considered:
|
68b3693
to
b152a17
Compare
Can you rebase, hoping that will improve CI results a bit? |
b152a17
to
c3afcc8
Compare
This allows built results to be renamed during installation, just as with install_data. This feature is motivated by the need to install multiple files from a single source directory into different install directories but with the same basename. In my case, I build dozens of 'libc.a' files which need to be installed into the toolchain's multilib heirarchy correctly. The rename parameter must either be empty, indicating that renaming is disabled, or have length equal to the number of outputs generated. Using an empty list to disable renaming matches the behavior of install_data. Signed-off-by: Keith Packard <[email protected]>
This allows generated configuration files to be renamed during installation, just as with install_data. The rename parameter must either be an empty list, indicating that renaming is disabled, or a single string. Signed-off-by: Keith Packard <[email protected]>
Executables, shared libraries and shared modules may all install two additional files, an import library and a debuginfo file. Provide new keyword arguments, 'import_rename', and 'debug_rename' so that these can be installed with names matching the output filename when using the 'rename' keyword argument. Signed-off-by: Keith Packard <[email protected]>
c3afcc8
to
d4cece6
Compare
This allows built results from any build target, custom target or configure file to be renamed during installation, just as with install_data.
This feature is motivated by the need to install multiple files from a single source directory into different install directories but with the same basename. In my case, I build dozens of 'libc.a' files which need to be installed into the toolchain's multilib heirarchy correctly.
This replaces #11954 and I can use this instead of #4037.