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.

Sign up for GitHub

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

Jump to bottom

Support external customisation of top-level namespace #1292

Open
feltech opened this issue Apr 11, 2024 · 1 comment
Open

Support external customisation of top-level namespace #1292

feltech opened this issue Apr 11, 2024 · 1 comment
Labels
needs discussion
Milestone
v1.x.x

Comments

@feltech
Copy link
Member

feltech commented Apr 11, 2024 •
edited
Loading

What

Support build system customisable top-level namespace(s) of all OpenAssetIO symbols.

Why

There are use cases where multiple (incompatible) copies of OpenAssetIO may coexist in a single application process.

For example, an OpenAssetIO Host application may have its own plugin system, and an application-specific plugin may wish to (privately) make use of a custom build of OpenAssetIO as well as the version exposed by the host application.

Acceptance Criteria

Delete as appropriate based on requirements gathering:

  • Expose the existing inline namespace macro OPENASSETIO_CORE_ABI_VERSION as a CMake cache variable.
  • Add a new inline namespace wrapper around all symbols, and expose as a new CMake cache variable.
  • Add a CMake cache variable to enable renaming of the top-level namespace openassetio.
  • Encode the customisable namespace(s) in the file names of the binary shared libraries.
  • Mirror the customisable namespace(s) in Python (see Python namespace packages)

Notes

Needs requirements gathering to narrow the focus.
@feltech feltech added this to the v1.0.0-NiceToHave milestone Apr 15, 2024
@elliotcmorris
Copy link
Contributor

We can and probably should endeavour to do this without a break.

feltech added a commit to feltech/OpenAssetIO that referenced this issue Jan 27, 2025
@feltech
[Lint] Remove TODOs in sources
c163238 
Closes OpenAssetIO#1091.

Issues created for TODOs that are still relevant:

- MacOS equivalent of `-Wl,--exclude-libs,ALL` OpenAssetIO#1439
- Making use of CMake's `FILE_SET` functionality OpenAssetIO#547.
- Customising the library name and namespace OpenAssetIO#1292.
- pylint scanning of extension modules OpenAssetIO#1136.
- Windows build improvements OpenAssetIO#538.
- Bump pybind11 version and remove LSan suppressions and bindings
  workarounds OpenAssetIO#1440.

TODO comments simply removed:

CI
- Linting of TraitGen isn't relevant to OpenAssetIO integration tests.

CMake
- There's not much we can/will do to resolve innocuous errors printed
  due to `MACOSX_RPATH TRUE`. Left a note explaining the issue.
- Nothing we can do about libabigail suppression `std::` regex bug, and
  using `--no-default-suppression` hasn't caused any problems so far.
  Left a note explaining the issue.
- We rejected an issue to investigate ASan on platforms other than Linux
  (OpenAssetIO#381), so no point leaving a TODO in the codebase to fix it for
  MacOS.
- Using `BUILD_SHARED_LIBS` is idiomatic, it's not clear what we'd do
  instead, and the TODO comment was written in the earliest days of
  OpenAssetIO whilst the CMake structure was still forming.
- The CMake `configure_package_config_file()`'s `PATH_VARS` argument is
  a way to notify CMake which variables are paths that need translating
  to fit the install tree. We don't make use of any such variables.
- Tested pybind11 version(s) is documented elsewhere, it is up to the
  consumer to choose an appropriate version. This is the same as for
  other dependencies. No need to pin the version in CMake.
- The location of `OPENASSETIO_CORE_ABI_VERSION` alongside other export
  related macros has turned out to be convenient, so no need to change
  this. There is an issue to extract macros to their own header (OpenAssetIO#1437),
  which may reconsider this.
- Changing the name of OpenAssetIO library file name(s) would be a
  breaking change, and the current names work just fine.
- Supporting pkg-config might be useful, but many other projects don't
  bother, and noone has requested this.

Conan
- More idiomatic/modern conanfile.py can/should be done as part of Conan
  2 adoption (OpenAssetIO#1313).
- libfmt latest versions no longer have the erroneous symbol export, so
  we're free to upgrade like any other library. However, we must
  consider downstream build pipelines before doing so (e.g. the latest
  libfmt version moves some symbols around).

FileUrlPathConverter
- There are no plans to diverge from compatibility with swift-url, so
  remove the TODOs on error priority. Leave explanatory notes.
- Ada supporting `percent_encode<true>` is off the table - left note in
  code comment to upstream issue.
- Optimisation of path parsing is unlikely to happen with the current
  implementation. The implementation is set to change by removing the
  PCRE2 dependency (OpenAssetIO#1289).

C API
- In general the C API is due a major re-think, so most TODOs just seem
  overly specific.
- There is no record of what "@exception messages" is supposed to mean.
- Granting access to tests to private sources is a common practice
  across the codebase.
- We're never likely to simulate and test a `std::bad_alloc` condition.

Python bindings/tests
- `isEntityReferenceString` tests now have a test for
  `kInfoKey_EntityReferencesMatchPrefix` so the TODO is defunct.
- There isn't much we can do to test/solve catastrophic `PyRetainingPtr`
  destruction corner cases. Leave notes explaining the potential issues.
- `__str__`/`__repr__` is now implemented, and tested in
  `test_printable.py`. The TODO re. "debug tricks" is ancient (predates
  the git repo), it's unclear what it means, but is almost definitely
  defunct.
- The pylint suppression of `abstract-method` is irrelevant since we
  migrated to C++, plus the mentioned issue OpenAssetIO#163 has been closed, so the
  TODO is defunct anyway.
- Suppressing the pylint `no-name-in-module` check for a couple of
  imports of `TraitsData` seems superfluous, and the comment(s) is
  ancient and seems to be regarding support for pylint in `--editable`
  installs, which won't be supported until OpenAssetIO#1309.
- Ancient TODO to assert that `TraitsData.traitSet` returns a `set()`
  that doesn't reference internals is unclear and/or seems superfluous,
  given that is how the pybind11 bindings work and is relied on in
  various other tests.

SimpleResolver
- The partial string test is sufficient. We could add an issue to update
  the test when OpenAssetIO/OpenAssetIO-Manager-BAL#127 is done, but
  it doesn't seem worth the effort.

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jan 27, 2025
@feltech
[Lint] Remove TODOs in sources
9ae07aa 
Closes OpenAssetIO#1091.

Issues created for TODOs that are still relevant:

- MacOS equivalent of `-Wl,--exclude-libs,ALL` OpenAssetIO#1439
- Making use of CMake's `FILE_SET` functionality OpenAssetIO#547.
- Customising the library name and namespace OpenAssetIO#1292.
- pylint scanning of extension modules OpenAssetIO#1136.
- Windows build improvements OpenAssetIO#538.
- Bump pybind11 version and remove LSan suppressions and bindings
  workarounds OpenAssetIO#1440.

TODO comments simply removed:

CI
- Linting of TraitGen isn't relevant to OpenAssetIO integration tests.

CMake
- There's not much we can/will do to resolve innocuous errors printed
  due to `MACOSX_RPATH TRUE`. Left a note explaining the issue.
- Nothing we can do about libabigail suppression `std::` regex bug, and
  using `--no-default-suppression` hasn't caused any problems so far.
  Left a note explaining the issue.
- We rejected an issue to investigate ASan on platforms other than Linux
  (OpenAssetIO#381), so no point leaving a TODO in the codebase to fix it for
  MacOS.
- Using `BUILD_SHARED_LIBS` is idiomatic, it's not clear what we'd do
  instead, and the TODO comment was written in the earliest days of
  OpenAssetIO whilst the CMake structure was still forming.
- The CMake `configure_package_config_file()`'s `PATH_VARS` argument is
  a way to notify CMake which variables are paths that need translating
  to fit the install tree. We don't make use of any such variables.
- Tested pybind11 version(s) is documented elsewhere, it is up to the
  consumer to choose an appropriate version. This is the same as for
  other dependencies. No need to pin the version in CMake.
- The location of `OPENASSETIO_CORE_ABI_VERSION` alongside other export
  related macros has turned out to be convenient, so no need to change
  this. There is an issue to extract macros to their own header (OpenAssetIO#1437),
  which may reconsider this.
- Changing the name of OpenAssetIO library file name(s) would be a
  breaking change, and the current names work just fine.
- Supporting pkg-config might be useful, but many other projects don't
  bother, and noone has requested this.

Conan
- More idiomatic/modern conanfile.py can/should be done as part of Conan
  2 adoption (OpenAssetIO#1313).
- libfmt latest versions no longer have the erroneous symbol export, so
  we're free to upgrade like any other library. However, we must
  consider downstream build pipelines before doing so (e.g. the latest
  libfmt version moves some symbols around).

FileUrlPathConverter
- There are no plans to diverge from compatibility with swift-url, so
  remove the TODOs on error priority. Leave explanatory notes.
- Ada supporting `percent_encode<true>` is off the table - left note in
  code comment to upstream issue.
- Optimisation of path parsing is unlikely to happen with the current
  implementation. The implementation is set to change by removing the
  PCRE2 dependency (OpenAssetIO#1289).

C API
- In general the C API is due a major re-think, so most TODOs just seem
  overly specific.
- There is no record of what "@exception messages" is supposed to mean.
- Granting access to tests to private sources is a common practice
  across the codebase.
- We're never likely to simulate and test a `std::bad_alloc` condition.

Python bindings/tests
- `isEntityReferenceString` tests now have a test for
  `kInfoKey_EntityReferencesMatchPrefix` so the TODO is defunct.
- There isn't much we can do to test/solve catastrophic `PyRetainingPtr`
  destruction corner cases. Leave notes explaining the potential issues.
- `__str__`/`__repr__` is now implemented, and tested in
  `test_printable.py`. The TODO re. "debug tricks" is ancient (predates
  the git repo), it's unclear what it means, but is almost definitely
  defunct.
- The pylint suppression of `abstract-method` is irrelevant since we
  migrated to C++, plus the mentioned issue OpenAssetIO#163 has been closed, so the
  TODO is defunct anyway.
- Suppressing the pylint `no-name-in-module` check for a couple of
  imports of `TraitsData` seems superfluous, and the comment(s) is
  ancient and seems to be regarding support for pylint in `--editable`
  installs, which won't be supported until OpenAssetIO#1309.
- Ancient TODO to assert that `TraitsData.traitSet` returns a `set()`
  that doesn't reference internals is unclear and/or seems superfluous,
  given that is how the pybind11 bindings work and is relied on in
  various other tests.

SimpleResolver
- The partial string test is sufficient. We could add an issue to update
  the test when OpenAssetIO/OpenAssetIO-Manager-BAL#127 is done, but
  it doesn't seem worth the effort.

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jan 28, 2025
@feltech
[Lint] Remove TODOs in sources
ece892e 
Closes OpenAssetIO#1091.

Issues created for TODOs that are still relevant:

- MacOS equivalent of `-Wl,--exclude-libs,ALL` OpenAssetIO#1439
- Making use of CMake's `FILE_SET` functionality OpenAssetIO#547.
- Customising the library name and namespace OpenAssetIO#1292.
- pylint scanning of extension modules OpenAssetIO#1136.
- Windows build improvements OpenAssetIO#538.
- Bump pybind11 version and remove LSan suppressions and bindings
  workarounds OpenAssetIO#1440.

TODO comments simply removed:

CI
- Linting of TraitGen isn't relevant to OpenAssetIO integration tests.

CMake
- There's not much we can/will do to resolve innocuous errors printed
  due to `MACOSX_RPATH TRUE`. Left a note explaining the issue.
- Nothing we can do about libabigail suppression `std::` regex bug, and
  using `--no-default-suppression` hasn't caused any problems so far.
  Left a note explaining the issue.
- We rejected an issue to investigate ASan on platforms other than Linux
  (OpenAssetIO#381), so no point leaving a TODO in the codebase to fix it for
  MacOS.
- Using `BUILD_SHARED_LIBS` is idiomatic, it's not clear what we'd do
  instead, and the TODO comment was written in the earliest days of
  OpenAssetIO whilst the CMake structure was still forming.
- The CMake `configure_package_config_file()`'s `PATH_VARS` argument is
  a way to notify CMake which variables are paths that need translating
  to fit the install tree. We don't make use of any such variables.
- Tested pybind11 version(s) is documented elsewhere, it is up to the
  consumer to choose an appropriate version. This is the same as for
  other dependencies. No need to pin the version in CMake.
- The location of `OPENASSETIO_CORE_ABI_VERSION` alongside other export
  related macros has turned out to be convenient, so no need to change
  this. There is an issue to extract macros to their own header (OpenAssetIO#1437),
  which may reconsider this.
- Changing the name of OpenAssetIO library file name(s) would be a
  breaking change, and the current names work just fine.
- Supporting pkg-config might be useful, but many other projects don't
  bother, and noone has requested this.

Conan
- More idiomatic/modern conanfile.py can/should be done as part of Conan
  2 adoption (OpenAssetIO#1313).
- libfmt latest versions no longer have the erroneous symbol export, so
  we're free to upgrade like any other library. However, we must
  consider downstream build pipelines before doing so (e.g. the latest
  libfmt version moves some symbols around).

FileUrlPathConverter
- There are no plans to diverge from compatibility with swift-url, so
  remove the TODOs on error priority. Leave explanatory notes.
- Ada supporting `percent_encode<true>` is off the table - left note in
  code comment to upstream issue.
- Optimisation of path parsing is unlikely to happen with the current
  implementation. The implementation is set to change by removing the
  PCRE2 dependency (OpenAssetIO#1289).

C API
- In general the C API is due a major re-think, so most TODOs just seem
  overly specific.
- There is no record of what "@exception messages" is supposed to mean.
- Granting access to tests to private sources is a common practice
  across the codebase.
- We're never likely to simulate and test a `std::bad_alloc` condition.

Python bindings/tests
- `isEntityReferenceString` tests now have a test for
  `kInfoKey_EntityReferencesMatchPrefix` so the TODO is defunct.
- There isn't much we can do to test/solve catastrophic `PyRetainingPtr`
  destruction corner cases. Leave notes explaining the potential issues.
- `__str__`/`__repr__` is now implemented, and tested in
  `test_printable.py`. The TODO re. "debug tricks" is ancient (predates
  the git repo), it's unclear what it means, but is almost definitely
  defunct.
- The pylint suppression of `abstract-method` is irrelevant since we
  migrated to C++, plus the mentioned issue OpenAssetIO#163 has been closed, so the
  TODO is defunct anyway.
- Suppressing the pylint `no-name-in-module` check for a couple of
  imports of `TraitsData` seems superfluous, and the comment(s) is
  ancient and seems to be regarding support for pylint in `--editable`
  installs, which won't be supported until OpenAssetIO#1309.
- Ancient TODO to assert that `TraitsData.traitSet` returns a `set()`
  that doesn't reference internals is unclear and/or seems superfluous,
  given that is how the pybind11 bindings work and is relied on in
  various other tests.

SimpleResolver
- The partial string test is sufficient. We could add an issue to update
  the test when OpenAssetIO/OpenAssetIO-Manager-BAL#127 is done, but
  it doesn't seem worth the effort.

Signed-off-by: David Feltell <david.feltell@foundry.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs discussion
Projects
OpenAssetIO Development
Status: No status
Milestone
v1.x.x
Development

No branches or pull requests
2 participants
@feltech @elliotcmorris