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

ENH support .conda format #182

Merged
merged 14 commits into from
Nov 17, 2022
Merged

ENH support .conda format #182

merged 14 commits into from
Nov 17, 2022

Conversation

beckermr
Copy link
Member

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

This PR adds support for .conda artifacts.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@beckermr
Copy link
Member Author

@conda-forge-admin rerender

@beckermr
Copy link
Member Author

@conda-forge/core Is this recipe supposed to generate ~76 variants? I guess yes, it just seems like a lot.

@beckermr
Copy link
Member Author

I guess we could build some downstream CUDA tests for all of the variants and list them here. We'd have to update the list here manually which kind of stinks. I think the right answer here is some sort of intrinsic testing loop over variants within conda build, but who has the time!

Moving on.

@beckermr
Copy link
Member Author

@conda-forge-admin rerender

@github-actions
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest rerendering GutHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or try re-rendering locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-forge-ci-setup-feedstock/actions/runs/1879574153.

@beckermr
Copy link
Member Author

@conda-forge-admin rerender

recipe/meta.yaml Outdated Show resolved Hide resolved
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@beckermr
Copy link
Member Author

beckermr commented Nov 7, 2022

@conda-forge-admin rerender

1 similar comment
@beckermr
Copy link
Member Author

beckermr commented Nov 7, 2022

@conda-forge-admin rerender

@beckermr beckermr marked this pull request as ready for review November 7, 2022 14:16
@beckermr beckermr requested a review from a team as a code owner November 7, 2022 14:16
@beckermr beckermr marked this pull request as draft November 7, 2022 14:58
@beckermr
Copy link
Member Author

beckermr commented Nov 7, 2022

Thanks @xhochy. This needs to be tested before it is merged.

@xhochy
Copy link
Member

xhochy commented Nov 7, 2022

Code was looking but I was under the impression that this would simply be for preparation and we could not really test this yet?

@beckermr
Copy link
Member Author

beckermr commented Nov 7, 2022

We can copy the package wholesale to our test feedstock and it will get run live there. Not going to do that today but will do before we merge.

@beckermr
Copy link
Member Author

This one is being tested here: conda-forge/cf-autotick-bot-test-package-feedstock#439

@beckermr beckermr marked this pull request as ready for review November 15, 2022 19:06
@beckermr
Copy link
Member Author

@conda-forge/core This one is ready for review. It lets us upload .conda artifacts by setting a key in the conda-forge.yml. This key is not read by smithy, but is instead used by the ci setup code the builds the .condarc. IDK if this choice is consistent with the other ways we do things, so I wanted to flag this in case people had concerns.

@jakirkham
Copy link
Member

Should we go ahead and add that key to feedstocks, but set it to false?

@beckermr
Copy link
Member Author

You would want to set it to None, but no we should not.

@beckermr
Copy link
Member Author

Or at least we don't gain anything by setting it to None now since old versions of the ci setup package simply ignore it.

@jakirkham
Copy link
Member

jakirkham commented Nov 15, 2022

Sorry None. Oh what is the issue with setting it?

Edit: Am guessing at some point in the future we would want .conda to be the default. So am wondering if we should go ahead and take steps to flag/protect feedstocks from that change. Though guess this doesn't have to happen immediately.

@beckermr
Copy link
Member Author

I'm not following sorry. The possible values are

conda_pkg_format: None  # .tar.bz2
conda_pkg_format: "2"   # .conda

The code here does

def maybe_use_dot_conda(feedstock_root):
    """Maybe set the .condarc to use .conda files."""
    if os.path.exists(os.path.join(feedstock_root, "conda-forge.yml")):
        with open(os.path.join(feedstock_root, "conda-forge.yml")) as f:
            repo_config = safe_load(f)
        pkg_format = repo_config.get("conda_pkg_format", None)
        if pkg_format is not None:
            try:
                call([
                    "conda", "config", "--env", "--set",
                    "conda_build.pkg_format", str(pkg_format)
                ])
            except subprocess.CalledProcessError:
                pass

Once we want to move the default, we change

pkg_format = repo_config.get("conda_pkg_format", None)

to

pkg_format = repo_config.get("conda_pkg_format", "2")

So the current default of tar.bz2 is hard coded into our scripts until we change it.

Does that answer your question?

@beckermr
Copy link
Member Author

@conda-forge/core any more comments here?

@beckermr beckermr merged commit 40e67c5 into conda-forge:main Nov 17, 2022
@beckermr beckermr deleted the dot-conda branch November 17, 2022 17:40
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.

5 participants