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

[wayland] Bump required dependencies #22788

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

MartinDelille
Copy link
Contributor

@MartinDelille MartinDelille commented Feb 16, 2024

Naive intent to solve the conflict detected here


Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/wayland//'.

👋 @jwillikers you might be interested. 😉

@conan-center-bot

This comment has been minimized.

@ghost
Copy link

ghost commented Feb 16, 2024

I detected other pull requests that are modifying wayland/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@ghost ghost mentioned this pull request Feb 21, 2024
3 tasks
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ericLemanissier
Copy link
Contributor

@conan-io/barbarians We are in a worse situation on all accounts:

  • conflicts still happen as they used to
  • the fix of these conflict is much slower than before, because we have to wait for a team member twice per PR

also pinging @uilianries @RubenRBS @franramirez688 because it looks like regular users cannot ping barbarians

@conan-center-bot

This comment has been minimized.

@jcar87
Copy link
Contributor

jcar87 commented Feb 21, 2024

Please be aware that we are very unlikely to pay attention to a PR without a description, no matter how trivial it may seem.

This other PR: #22838 - is a good example of a good description - this provides us with enough context to trace back why the version conflict happened in the first place, and also lets us see a scenario where the conflict is happening.

We should have been more diligent before approving #22727 - as we have enough tools to see that this would've caused conflicts.

This PR, by updating libxml2 to version 2.12.4 - actually creates a different conflict - since xkb-common still depends on `2.12.3. So if this PR was automatically merged it would just create another conflict (I have been testing this from the qt recipe), and you would still have an issue with Qt. This is why we have disabled automerging of all dependency bumps - it requires a coordination to follow up with additional PRs to get consistency, both in terms of opening them, running them and merging them.

I'm reverting the change for this PR to avoid another conflict and help PRs held up by the expat conflict. We will ensure that we minimise the introduction of conflicts while reviewing any future PR. We are simultaneously working on tooling to automate the detection of potential conflicts (to prevent them in the first place), and also the ability to touch multiple recipes in a single PR (to be able to consistently version bump the entire repo, if necessary).

@conan-center-bot conan-center-bot added Bump dependencies Only bumping dependencies versions in the recipe and removed Service Under Maintenance labels Feb 21, 2024
@conan-center-bot

This comment has been minimized.

@ericLemanissier
Copy link
Contributor

ericLemanissier commented Feb 21, 2024

We should have been more diligent before approving #22727 - as we have enough tools to see that this would've caused conflicts.

The fact that the proper tooling to detect these conflicts is not ready to process all pull requests (even those which do not bump deps can create conflicts by changing an option value for example) makes the PR embargo counter-productive. In a perfect world, this tool would be open-source so that every conan user can benefit, and running as a github action on CCI so that it is transparent to contributors (any contributor would be able to test PRs on any repo, and fix all conflicts before submitting the PR to CCI)

This PR, by updating libxml2 to version 2.12.4 - actually creates a different conflict - since xkb-common still depends on `2.12.3. So if this PR was automatically merged it would just create another conflict (I have been testing this from the qt recipe), and you would still have an issue with Qt. This is why we have disabled automerging of all dependency bumps - it requires a coordination to follow up with additional PRs to get consistency, both in terms of opening them, running them and merging them.

I can see two solutions here, until you allow several recipes to be modified by a single PR:

  1. either you somehow put a definitive hold on all the PRs bumping libxml2 on all recipes, and this effectively prevents all conflicts from happening (but this should be documented)
  2. or you somehow allow libxml2 to be bumped sometimes, and when it happens you get the same conflict as the one you just avoided.

This was referenced Feb 22, 2024
@conan-center-bot conan-center-bot removed the Bump dependencies Only bumping dependencies versions in the recipe label Feb 22, 2024
@conan-center-bot

This comment has been minimized.

@MartinDelille
Copy link
Contributor Author

Hi @jcar87 ! I should have indeed have given a proper description (which I just did). My goal is to get #21892 merged, a PR that I am working on since a long time.

Thank you for all your explanations by the way!

@ericLemanissier
Copy link
Contributor

@uilianries @RubenRBS @franramirez688 @jcar87 can one of you please trigger the CI so that this PR is merged, and the conflict finally resolved ?

@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot added Waiting for CI and removed Unexpected Error Bump dependencies Only bumping dependencies versions in the recipe labels Feb 25, 2024
@conan-center-bot

This comment has been minimized.

@ericLemanissier
Copy link
Contributor

@uilianries @RubenRBS @franramirez688 @jcar87 can one of you please trigger the CI so that this PR is merged, and the conflict finally resolved ?

@conan-center-bot conan-center-bot added Bump dependencies Only bumping dependencies versions in the recipe and removed Waiting for CI labels Feb 26, 2024
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 10 (996164dfa5e9a95b240afa887ef5744611edb4eb):

  • wayland/1.22.0:
    All packages built successfully! (All logs)

  • wayland/1.21.0:
    All packages built successfully! (All logs)

  • wayland/1.20.0:
    All packages built successfully! (All logs)

  • wayland/1.19.0:
    All packages built successfully! (All logs)

  • wayland/1.18.0:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 10 (996164dfa5e9a95b240afa887ef5744611edb4eb):

  • wayland/1.22.0:
    All packages built successfully! (All logs)

  • wayland/1.21.0:
    All packages built successfully! (All logs)

  • wayland/1.19.0:
    All packages built successfully! (All logs)

  • wayland/1.18.0:
    All packages built successfully! (All logs)

  • wayland/1.20.0:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 75c9983 into conan-io:master Feb 26, 2024
38 checks passed
@conan-center-bot
Copy link
Collaborator

This PR has been automatically merged due to Bump version or Bump dependencies label.
Read https://github.com/conan-io/conan-center-index/blob/master/docs/labels.md#bump-version to obtain more information.

@MartinDelille MartinDelille deleted the bump-wayland-dep branch February 27, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bump dependencies Only bumping dependencies versions in the recipe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants