conda-forge / conda-forge-ci-setup-feedstock

A conda-smithy repository for conda-forge-ci-setup.
BSD 3-Clause "New" or "Revised" License
13 stars 51 forks source link

ENH support .conda format #182

Closed beckermr closed 1 year ago

beckermr commented 2 years ago

Checklist

This PR adds support for .conda artifacts.

conda-forge-linter commented 2 years ago

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 commented 2 years ago

@conda-forge-admin rerender

beckermr commented 2 years ago

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

beckermr commented 2 years ago

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 commented 2 years ago

@conda-forge-admin rerender

github-actions[bot] commented 2 years ago

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 commented 2 years ago

@conda-forge-admin rerender

conda-forge-linter commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

@conda-forge-admin rerender

beckermr commented 1 year ago

@conda-forge-admin rerender

beckermr commented 1 year ago

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

xhochy commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

This one is being tested here: https://github.com/conda-forge/cf-autotick-bot-test-package-feedstock/pull/439

beckermr commented 1 year ago

@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 commented 1 year ago

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

beckermr commented 1 year ago

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

beckermr commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@conda-forge/core any more comments here?