Skip to content

Update cmake-format version - #441

Merged
ajschmidt8 merged 3 commits into
rapidsai:branch-22.04from
vyasr:feature/update_cmake_format
Apr 22, 2022
Merged

Update cmake-format version#441
ajschmidt8 merged 3 commits into
rapidsai:branch-22.04from
vyasr:feature/update_cmake_format

Conversation

@vyasr

@vyasr vyasr commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

Update to cmake-format 0.6.13

CC @robertmaynard

@vyasr
vyasr requested a review from a team as a code owner March 15, 2022 22:44
ajschmidt8
ajschmidt8 reviewed Mar 16, 2022
View reviewed changes
Comment thread conda/recipes/rapids-build-env/meta.yaml Outdated
- conda-forge::clang-tools {{ clang_version }} # [x86_64]
- cmake {{ cmake_version }}
- cmake-format {{ cmake_format_version }}
- cmakelang {{ cmake_format_version }}

@ajschmidt8 ajschmidt8 Mar 16, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vyasr, was this intentionally changed to cmakelang? It looks like the CI logs are showing the error message below after this change. It doesn't seem like that package exists.

image

@ajschmidt8 ajschmidt8 mentioned this pull request Mar 16, 2022
Merged
@vyasr

vyasr commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author

I saw that the package existed but it didn't even occur to me to consider that it was on a channel other than conda-forge. Turns out it is: https://anaconda.org/programfan/cmakelang.

The issue is that cmake-format is the older name of the package and that conda-forge package has not been updated. I can contact the owner and see if he is interested in updating it there; I had simply assumed that the preferred name on conda had changed, but apparently not. @robertmaynard have you had any contact/do you have a relationship with the cmake-format maintainer? Otherwise I can just reach out via issues.

@vyasr

vyasr commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author

The simpler alternative would be to install it as a pip dependency instead of directly via conda. Is that something that we could support here? cmake-format should be entirely independent of other libraries, so we shouldn't run into any conflicts.

@robertmaynard

robertmaynard commented Mar 17, 2022

Copy link
Copy Markdown

I saw that the package existed but it didn't even occur to me to consider that it was on a channel other than conda-forge. Turns out it is: https://anaconda.org/programfan/cmakelang.

The issue is that cmake-format is the older name of the package and that conda-forge package has not been updated. I can contact the owner and see if he is interested in updating it there; I had simply assumed that the preferred name on conda had changed, but apparently not. @robertmaynard have you had any contact/do you have a relationship with the cmake-format maintainer? Otherwise I can just reach out via issues.

I don't have any existing relationship with the maintainers of cmake-format

@ajschmidt8

ajschmidt8 commented Mar 17, 2022

Copy link
Copy Markdown
Member

It looks like there was a PR opened to update the conda-forge package to 0.6.13 below, but it was ultimately decided that a new conda-forge repository should be created for the new name. It doesn't appear that anyone ever took the initiative to create the repo though 😕. @vyasr, is this something you'd be interested in attempting? The docs for conda-forge contributions are here: https://conda-forge.org/#contribute

@vyasr

vyasr commented Mar 17, 2022

Copy link
Copy Markdown
Contributor Author

Oh yeah I am happy to follow up on that.

@bdice

bdice commented Apr 18, 2022

Copy link
Copy Markdown
Contributor

@ajschmidt8 @vyasr I created a PR to address the cmakelang / 0.6.13 issue. conda-forge/staged-recipes#18686

@vyasr

vyasr commented Apr 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks @bdice! Once your new feedstock is merged and the relevant packages are uploaded we should be able to rerun and see tests pass here.

@vyasr

vyasr commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

PR is merged and the package appears to be up!

rerun tests

@ajschmidt8

ajschmidt8 commented Apr 21, 2022

Copy link
Copy Markdown
Member

Awesome! 🚀 🔥

bdice
bdice reviewed Apr 21, 2022
View reviewed changes
Comment thread conda/recipes/versions.yaml Outdated
@vyasr

vyasr commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

I guess maybe not all the necessary indexes on conda are updated yet? I tried [conda|mamba] install -c conda-forge cmakelang locally and it's still not finding the package even though we can see it online, so my first guess is that it can't find the packages yet.

@vyasr

vyasr commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

rerun tests

@vyasr
vyasr requested a review from ajschmidt8 April 21, 2022 20:58
@vyasr

vyasr commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

@ajschmidt8 I think we're good to go here! The integration test failures aren't related.

@ajschmidt8
ajschmidt8 merged commit 6f3151d into rapidsai:branch-22.04 Apr 22, 2022
@bdice

bdice commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

@ajschmidt8 Just noticed - was this supposed to target branch-22.04?

@ajschmidt8

ajschmidt8 commented Apr 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

@bdice, nope! Should've targeted 22.06. I realized that as well after I merged this and forward-merged it to 22.06.

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

Reviewers

@bdice bdice bdice left review comments
@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vyasr @robertmaynard @ajschmidt8 @bdice