Skip to content

Conversation

@Michael137
Copy link
Member

Got caught out by this because simply specifying DYLIB_CXX_SOURCES (without specifying DYLIB_NAME) resulted in linker errors because the dylib was never built (and linked). We should probably make that a Makefile error (though I haven't audited when exactly not specifying DYLIB_NAME is valid; looked like that can happen when we specify FRAMEWORK).

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Got caught out by this because simply specifying DYLIB_CXX_SOURCES (without specifying DYLIB_NAME) resulted in linker errors because the dylib was never built (and linked). We should probably make that a Makefile error (though I haven't audited when exactly not specifying DYLIB_NAME is valid; looked like that can happen when we specify FRAMEWORK).


Full diff: https://github.com/llvm/llvm-project/pull/112735.diff

1 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/make/Makefile.rules (+7)
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index f81db9bc06d8a8..a2a8ae504053c6 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -13,6 +13,13 @@
 # the building of the a.out executable program.  For example,
 # DYLIB_ONLY := YES
 #
+# When specifying one of the DYLIB_*_SOURCES variables, DYLIB_NAME
+# controls the name of the produced dylib. E.g., if set to "foo",
+# the generated dylib will be called "foo.<platform-specific-extension>",
+# which on Darwin will be "foo.dylib".
+#
+# DYLIB_NAME := foo
+#
 # Specifying FRAMEWORK and its variants has the effect of building a NeXT-style
 # framework.
 # FRAMEWORK := "Foo"

# When specifying one of the DYLIB_*_SOURCES variables, DYLIB_NAME
# controls the name of the produced dylib. E.g., if set to "foo",
# the generated dylib will be called "foo.<platform-specific-extension>",
# which on Darwin will be "foo.dylib".
Copy link
Member

Choose a reason for hiding this comment

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

This is interesting. LLDB makes an assumption that a module named foo will be turned onto libfoo.dylib on Darwin. See Platform::GetFullNameForDylib

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch! it was meant to be libfoo for the darwin example :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically, the "lib" part is platform specific as well. Windows does not have that convention. Libraries (at least, those that aren't very obviously unixy in origin) don't have it:

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, reworded to make it clear this is platform dependent (with an example of how it would look like on Darwin)

# When specifying one of the DYLIB_*_SOURCES variables, DYLIB_NAME
# controls the name of the produced dylib. E.g., if set to "foo",
# the generated dylib will be called "foo.<platform-specific-extension>",
# which on Darwin will be "foo.dylib".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically, the "lib" part is platform specific as well. Windows does not have that convention. Libraries (at least, those that aren't very obviously unixy in origin) don't have it:

# When specifying one of the DYLIB_*_SOURCES variables, DYLIB_NAME
# controls the (platform-dependent) name of the produced dylib. E.g.,
# on Darwin, if "DYLIB_NAME := foo", the generated dylib will be called
# "foo.dylib".
Copy link
Member

Choose a reason for hiding this comment

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

I think this is still missing the lib portion?

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, forgot about that, thanks

@Michael137 Michael137 merged commit faed85b into llvm:main Oct 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants