Skip to content

Conversation

carlopi
Copy link
Contributor

@carlopi carlopi commented Nov 25, 2023

This PR ties in to a rework to the extension pipeline that touches both duckdb-wasm extensions and regular extensions.

Idea is that for a bit both the 'old' extension format and the 'new' extension format can coexist, both to allow smooth and piece-by-piece transition of CI and to allows extensions to be packaged and build both for newer and older duckdb version, given there will be no breaking changes.

Currently extensions once built live at build/release/extension/${extension_name}/${extension_name}.duckdb_extension, and require signature to be appended in post-processing.
Proposed standard is have the old version live at the same place, with new one being preset ad build/release/extension/${extension_name}.duckdb-extension, and those will have a standard to package some additional metadata (extension name, platform, DuckDB version, and to be soon standardised distribution), plus a default signature that will need to be overridden during signing step.

Note that metadata-enabled extensions are compatible and can be rolled in and tested alongside with regular extensions, given they will still have signature in the same spot.

Binary format is to be specified yet, feedback is welcome on what fields could be of use, and what checks we expect to be able to make from those fields.
Current proposal is:

  #    a 32 bytes field for the extension name
  #    a 32 bytes field for the platform name
  #    a 32 bytes field for the DuckDB version
  #    a 32 bytes field for the distribution

This align with currently imposed restrictions on length of these fields, and keeping this simple to read and write would allow for tooling to detect what format are those files.

End goal, tentatively to be rolled on for v0.10.0, is allowing checks both on INSTALL and on LOAD, that would prevent problems where INSTALL of a local file is successful (and currently unchecked) and ends up in a corrupt ~/.duckdb/extension folder. Or on LOAD pointing to an extension for a wrong platform might end up in segfaults.

Yet to be added: have the default location for these metadata-enabled extensions be compatible with setting default_extension_folder, bringing in more simplification in tests and usability of dynamically loaded extensions for development.

@carlopi carlopi added the Draft label Nov 25, 2023
Copy link
Contributor

@samansmink samansmink left a comment

Choose a reason for hiding this comment

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

In general, maybe we can unify this step with the scripts/create_local_extension_repo.py script?

That script is used to do the autoloading tests. We should probably include some way to test the metadata we generate here. I think we can create a sqlogictest with require-env LOCAL_EXTENSION_REPO which can then assume that the extensions hold metadata

# three 32 bytes field left empty for yet to be decided properties, future proof
#
# Currently all those fields, ammounting to 512 bytes are set to zero, but some information can be already overridden
add_custom_command(
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed in person, it would be nice to wrap this in a single cmake function write_extension_metadata that writes the whole thing to a string then we append that string at once. I think this does not guarantee correct order.

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

Successfully merging this pull request may close these issues.

2 participants