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

Fix build in Backups/BackupIO_S3.cpp #56974

Merged
merged 1 commit into from Nov 19, 2023

Conversation

rschu1ze
Copy link
Member

PR #56460 broke the build:

/home/ubuntu/repo/ClickHouse/src/Backups/BackupIO_S3.cpp:255:5: error: no matching function for call to 'copyS3File'
  255 |     copyS3File(
      |     ^~~~~~~~~~
/home/ubuntu/repo/ClickHouse/src/IO/S3/copyS3File.h:31:6: note: candidate function not viable: no known conversion from 'std::shared_ptr<S3::Client>' to 'const String' (aka 'const basic_string<char, char_traits<char>, allocator<char>>') for 2nd argument
   33 | void copyS3File(
      |      ^
   34 |     const std::shared_ptr<const S3::Client> & s3_client,
   35 |     const String & src_bucket,
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
[10301/12493] Building CXX object src/CMakeFiles/dbms.dir/Functions/CastOverloadResolver.cpp.o
ninja: build stopped: subcommand failed.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Nov 19, 2023
@robot-ch-test-poll2
Copy link
Contributor

robot-ch-test-poll2 commented Nov 19, 2023

This is an automated comment for commit 60a17ee with description of existing statuses. It's updated for the latest CI running

⏳ Click here to open a full report in a separate page

Successful checks
Check nameDescriptionStatus
Mergeable CheckChecks if all other necessary checks are successful✅ success
Push to DockerhubThe check for building and pushing the CI related docker images to docker hub✅ success
Check nameDescriptionStatus
CI runningA meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR⏳ pending

@alexey-milovidov alexey-milovidov merged commit 276de4a into ClickHouse:master Nov 19, 2023
6 of 8 checks passed
@alexey-milovidov alexey-milovidov self-assigned this Nov 19, 2023
@antonio2368
Copy link
Member

antonio2368 commented Nov 19, 2023

PR #56460 broke the build:

Actually it's this PR #56314 which modifies copyS3File
It didn't have the latest master so it didn't catch the new code that calls copyS3File

This is an interesting problem and I don't have an idea how to avoid such situations. Maybe we can require for a build with latest master before accepting a merge but I think that would simply make things more annoying so manual fixes in such rare cases make more sense.

@rschu1ze
Copy link
Member Author

Agree. Afais, there are physical merge conflicts ever 1-2 weeks which is rarely given the amount of changes that go into master. Fixing them ad hoc seems better than wasting resources in a extra build that would also not find logical merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-not-for-changelog This PR should not be mentioned in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants