Skip to content

Support CircleCI 2.0#530

Merged
jakirkham merged 7 commits intoconda-forge:masterfrom
jakirkham:support_circle_2.0
Jul 19, 2017
Merged

Support CircleCI 2.0#530
jakirkham merged 7 commits intoconda-forge:masterfrom
jakirkham:support_circle_2.0

Conversation

@jakirkham
Copy link
Copy Markdown
Member

@jakirkham jakirkham commented Jun 17, 2017

Fixes #527

Closes conda-forge/pyelftools-feedstock#14
Closes conda-forge/gnureadline-feedstock#8
Closes conda-forge/rank_filter-feedstock#26

Upgrades to new CircleCI 2.0 by borrowing from this migration guide, the reference spec, and a bunch of examples. This should make it easier to explore matrix builds in CircleCI and provide us access to all the other latest features.

Test cases for the Linux build and non-Linux build can be seen. Note that CircleCI still doesn't build in the non-Linux case (and if it does it just exits quietly). Also despite looking like a big overhaul, the behavior in the CircleCI 2.0 case is basically the same. Though this will change when matrix builds are considered.

In the upgrade, we make sure to clean out the old circle.yml file too.

As the filtered branches having been moved closer to the build step in
the CircleCI config file, having two Jinja `if`-clauses no longer makes
sense. So this groups them together with the CircleCI skip fallback
test. Also this is done such that the CircleCI config file should not
change due to this difference in the template.
command: docker pull condaforge/linux-anvil
- run:
# Run, test and (if we have a BINSTAR_TOKEN) upload the distributions.
command: ./ci_support/run_docker_build.sh
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be nice to check how the cache works in this new version. So we could place this command in a section which could be cached and need fewer changes to use ccache.

Copy link
Copy Markdown
Contributor

@gqmelo gqmelo Jun 17, 2017

Choose a reason for hiding this comment

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

The cache is much more flexible and it seems we can choose exactly when to save and restore caches: https://circleci.com/docs/2.0/caching/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We should resurrect the package caching work you did in this context. There are many pure Python or metapackages now floating around that have huge lists of dependencies that slow down the test phase due to downloading. Having a package cache there will certainly help the CIs cope with our ever increasing number of feedstocks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure, we should definitely revisit this. I can take a look after you merge this.

@jakirkham
Copy link
Copy Markdown
Member Author

So a couple things to consider that I could use feedback on.

First we could move the ci_support scripts into .circleci. All of these scripts are CircleCI specific and it seems in some of CircleCI's examples they do the same thing. This would keep things pretty nicely organized. That said, it would move the run_docker_build.sh script, which has lived in this directory for a long time. Not sure if that would impact offline build usage. Also as the .circleci directory would appear hidden on Unix platforms, it may make this script less visible and could cause some confusion. For now I have done nothing on this front. Thoughts on this would definitely be appreciated.

Second technically CircleCI 2.0 is beta. That said, the closed beta started on Nov 7 (last year). Also it looks like they opened the beta up on Mar 17. Assuming a similar time range for ending beta, I would expect this to be official by sometime in July (i.e. next month). Presumably some time after that CircleCI 1.0 is going to be phased out. Given the number of users who have already tried out the beta and time elapsed (not to mention our slow rate of adoption for things like this), I don't think this is risky and it is something we are going to need to do anyways. Combined with better features for caching and matrix builds, this seems very desirable to adopt soon. That said, it would still be good to get feedback from others on this.

Given both of these changes can be seen as breaking, I'd be ok marking this as conda-smithy 3.0.0. As we need to start phasing out NumPy 1.11 as well to lighten the CIs (and in some cases already are), making this a breaking release seems reasonable. If it seems necessary, we could discuss some maintenance support for 2.x, but the NumPy issue would need to be taken into account as well.

@jakirkham
Copy link
Copy Markdown
Member Author

Would appreciate if @conda-forge/core could please take a look at this. Also feedback on the items noted above would be very helpful. Thanks in advance.

For CircleCI matrix support, we will need to construct multiple build
steps for different matrix parameters. So it makes more sense to include
`build` inside the Jinja conditional instead of outside. This will
facilitate making a clean `for`-loop within the last part of the
conditional.
@jakirkham
Copy link
Copy Markdown
Member Author

Also a CircleCI matrix implementation based off of this can be seen in PR ( #532 ) with links to example re-renderings.

@gqmelo
Copy link
Copy Markdown
Contributor

gqmelo commented Jun 19, 2017

That said, it would move the run_docker_build.sh script, which has lived in this directory for a long time.

I would prefer to not hide run_docker_build.sh under .circleci. Even though it's only used on CircleCI it's generic enough to be used standalone (personally I always run it to reproduce linux builds)

Second technically CircleCI 2.0 is beta.

Given the amount of time since it entered beta it seems pretty safe to make those changes now.

@jakirkham
Copy link
Copy Markdown
Member Author

Yeah, I think I agree on both points.

Though the run_docker_build.sh script will need to change for CircleCI matrix builds. ( #532 ) Still can use it externally, but one will want to set CONDA_* variables before running it.

Any thoughts on the other scripts in ci_support? Is it worth moving them over to .circleci? Could declutter things a bit.

@jakirkham
Copy link
Copy Markdown
Member Author

As @gqmelo noted elsewhere, it does seem that CircleCI 2.0 is not correctly grouping pull requests. Have opened a discussion issue on this point. I remember having similar issues on CircleCI 1.0 before when they made changes to pull requests, but it didn't affect anything beyond how things were grouped. Since the build number always increases and is unique to the repo build not how it was created, this issue ends up only being a UI one. Nevertheless it is worth noting.

ref: https://discuss.circleci.com/t/circleci-2-0-incorrectly-groups-pull/13505

@gqmelo
Copy link
Copy Markdown
Contributor

gqmelo commented Jun 19, 2017

Though the run_docker_build.sh script will need to change for CircleCI matrix builds. ( #532 ) Still can use it externally, but one will want to set CONDA_* variables before running it.

Yeah, no problem. This is already necessary when trying to reproduce osx and windows builds. It would be better to have a script for each platform to build all packages locally (like the current run_docker_build.sh), but I know we would probably have to duplicate code as the matrices are on CI config files.

Any thoughts on the other scripts in ci_support? Is it worth moving them over to .circleci? Could declutter things a bit.

Well, the other seems very specific to CircleCI and something one would rarely want to run locally. So I would be fine with moving them.

@jakirkham
Copy link
Copy Markdown
Member Author

It would be better to have a script for each platform to build all packages locally (like the current run_docker_build.sh), but I know we would probably have to duplicate code as the matrices are on CI config files.

Not to digress, but to mention this brief. Have been wanting to do something like this for a while. If we go for setting CONDA_* variables, it shouldn't be too bad. The main trick is we don't want to mess up the user's conda install. Also we probably don't want to add stuff to their home directory. So we might want to download conda to a temporary directory and go from there.

@gqmelo
Copy link
Copy Markdown
Contributor

gqmelo commented Jun 22, 2017

The main trick is we don't want to mess up the user's conda install. Also we probably don't want to add stuff to their home directory. So we might want to download conda to a temporary directory and go from there.

Yeah, if the scripts are going to install packages on the root env it would be better to have a separate miniconda installation. But we could have separate scripts for configuring miniconda and installing needed packages and other scripts to build all packages for the given plataform.

Something like:

  feedstock-dir
    |_ build_scripts
        |_ build_linux.sh
        |_ build_osx.sh
        |_ build_windows.bat
        |_ setup_linux.sh
        |_ setup_osx.sh
        |_ setup_windows.bat

setup_* scripts would change the user installation, but at least it will be more explicit and build_* would build all the matrix for the given platform.
So someone trying to reproduce windows builds would just have to run setup_windows.bat and then build_windows.bat to build all packages without having to worry about the matrix and the appveyor.yml file.
Anyway, this is just an idea for future refactorings.

@jakirkham
Copy link
Copy Markdown
Member Author

Friendly nudge @conda-forge/core. 😉

@jakirkham
Copy link
Copy Markdown
Member Author

Should add that CircleCI 2.0 is officially released. So no longer in beta.

@jakirkham
Copy link
Copy Markdown
Member Author

Given CircleCI 2.0 is no longer in beta, this is well tested, and has sat for a while to get feedback, going to go ahead and merge it to clear it from the list.

@jakirkham jakirkham merged commit 7e0a33c into conda-forge:master Jul 19, 2017
@jakirkham jakirkham deleted the support_circle_2.0 branch July 19, 2017 21:25
@mwcraig
Copy link
Copy Markdown
Contributor

mwcraig commented Jul 21, 2017

Thanks!

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.

3 participants