[CI] Add patch argument to patch the extension's sources before building #10831
Conversation
|
Thanks! I will check with @samansmink what's the path forward for extensions, in general more complex ones are sort of assumed to be hard to be maintainable for both v0.10.0 and v0.9.2, but that might not be true for all relevant extensions and going forward we need anyway a path for supporting multiple duckdb versions. duckdb_spatial has an experiment running where 2 different branches of the extension (each tied to a different duckdb version), and binaries are built for either duckdb_spatial on branch main + duckdb on branch main OR duckdb_spatial on branch v0.10.0 with duckdb on tag v0.10.0. (with branch v0.10.0 being basically main + the diff) Also somehow connected, it would be cool to have an explicit policy on what duckdb versions are expected to be supported (or we are going to support), and for the moment I guess it's only v0.10.0 (without putting effort in updating for v0.9.2) but that will need to adapt moving forward. Minor feedback is that elsewhere we use |
There was a problem hiding this comment.
TLDR; i'm fine with adding patching to the reusable workflow, but I think it should probably be used sparingly
The reason we have patching in other places is to basically have a "poor man's monorepo" where breaking changes can be pushed to DuckDB along with the required changes to extension or duckdb-wasm. Patching by itself should only be used sparingly I feel because the patches are kind of annoying to read and handle.
Also, I feel we should not push extension developers into supporting multiple stable duckdb versions at the same time at all. We do not strive to support that with our current extension API. I think duckdb's extension API should strive to have 2 ways of building extensions:
- Unstable C++ extension API where you can hook into duckdb wherever you want, however you want. (this is the current extension template).
- Stable C extension API which allows using a duckdb extension across multiple duckdb releases.
As part of implementing an extension against the C++ API, I think developers should strive to avoid the combinatorial explosion of trying to maintain the extension for the latest x stable duckdb versions.
Finally, I think this can also be achieved by creating separate branches in the extension repo, although admittedly that could be more work to maintain.
|
Agree with @samansmink, this is not ideal / recommended, but on the other side provide a clean way out. I think this can go in |
|
Thanks - could you just resolve the merge conflict? |
|
Done. I have a slight preference for |
|
In the meantime, this files have been mirrored to https://github.com/duckdb/extension-ci-tools/tree/main/.github/workflows, and will be eventually discontinued from here. Plan is to move it there, so when this go in we should remember to port the changes also there. |
|
Thanks |
Merge pull request duckdb/duckdb#11072 from Mytherin/parquetprogress Merge pull request duckdb/duckdb#10707 from Mause/bugfix/jdbc-get-time Merge pull request duckdb/duckdb#10831 from krlmlr/f-ext-patch Merge pull request duckdb/duckdb#11070 from mariotaddeucci/code-quality-pep8-compliant
I tried building an extension for both v0.9.2 and v0.10.0. It seems that this requires a different
Makefilefor each version. My solution was to provide a patch file to restore the v0.9.2 state before building it.Example: https://github.com/hannes/duckdb-rfuns/blob/main/.github/0.9.2.patch, used in https://github.com/hannes/duckdb-rfuns/blob/87184f8b2ed13f99c3a102dd27f587adc4367cec/.github/workflows/MainDistributionPipeline-0.9.2.yml#L29