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.

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

update R CMD workflow to apply patches in .github/patches/duckdb-r #9412

Merged
merged 3 commits into from Oct 20, 2023

Conversation

Tmonster
Copy link
Contributor

@Tmonster Tmonster commented Oct 20, 2023

If the duckdb repo ever creates breaking changes to the duckdb-r repo that need patches, we can prevent CI from yelling at us if we include the patches. These patches then need to be applied to duckdb-r at some later date.

Most likely the duckdb-r CI will yell when the sources are updated. One idea might be to apply the patches from within the vendor script.

Needed because #9406 will break the duckdb-r client

@Tmonster Tmonster requested a review from carlopi October 20, 2023 11:42
- name: Apply duckdb-r patches
shell: bash
run: |
for filename in $GITHUB_WORKSPACE/$DUCKDB_SRC/.github/patches/duckdb-r/*.patch; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for filename in $GITHUB_WORKSPACE/$DUCKDB_SRC/.github/patches/duckdb-r/*.patch; do
for filename in ${{ env.DUCKDB_SRC }}/.github/patches/duckdb-r/*.patch; do

(not that I am completely sure, but this seems to make more sense)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$GITHUB_WORKSPACE is just the absolute path. we also use $GITHUB_WORKSPACE/$DUCKDB_SRC/ here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, also realized that this was executing in the wrong directory

Copy link
Contributor

Choose a reason for hiding this comment

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

I got a bit lost with the directories, unsure.

Actually one thing you need to add, to handle the case of no files being there, is:

shopt -s nullglob

See for example this part of the Nightly workflow: https://github.com/duckdb/duckdb/blob/main/.github/workflows/NightlyTests.yml#L630

@github-actions github-actions bot marked this pull request as draft October 20, 2023 12:17
@Tmonster Tmonster marked this pull request as ready for review October 20, 2023 12:24
@Tmonster Tmonster requested a review from carlopi October 20, 2023 12:27
@Mytherin Mytherin merged commit c0db850 into duckdb:main Oct 20, 2023
3 checks passed
@Mytherin
Copy link
Collaborator

Thanks! LGTM

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

Successfully merging this pull request may close these issues.

None yet

3 participants