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

PR: Update conda-based installers #20056

Merged
merged 23 commits into from
Nov 18, 2022

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented Nov 15, 2022

Description of Changes

  • Fixed issue where installer job was run when external-deps package build jobs fail
  • Fixed issue where certain directories were missing when package build jobs are skipped
  • Fixed issue where scm version is improperly resolved due to cloned source creating an unclean repository state
  • Minor refactor of conda package building module
  • Use --no-test flag rather than removing test directives in meta.yaml
  • Fix issue where shallow clones could result in unknown commit checkout
  • Do not build for macOS arm64 architecture
  • Update to accommodate new spyder-kernels feedstock
  • Source code patch file is generated on demand rather than tracked

@mrclary mrclary self-assigned this Nov 15, 2022
@mrclary mrclary force-pushed the update-installers-conda branch 2 times, most recently from 905b41a to a005cd5 Compare November 15, 2022 04:24
@dalthviz dalthviz added this to the v5.4.1 milestone Nov 16, 2022
@dalthviz dalthviz changed the title Update conda-based installers PR: Update conda-based installers Nov 16, 2022
Properly allow for arbitrary repo source via environment variable, including Spyder.
Put cloned source into DIST directory to prevent unclean repository state.
Try --skip-existing to see what happens
Try --build-id-pat={n} to help with long path names on windows
Don't load and dump spec file for each package
Enclose in try statement in order to try to build all packages, regardless of failure
@mrclary mrclary force-pushed the update-installers-conda branch 3 times, most recently from d64507e to a1c91af Compare November 17, 2022 06:34
Do not clone Spyder source if it is current repo; this causes problems for determining patch on CI.
Note that ruamel.yaml will not allow duplicate keys (e.g. 'build.string'), so regular expression find/replace must be used to patch the meta.yaml files.
@mrclary mrclary force-pushed the update-installers-conda branch 5 times, most recently from 8b442d1 to 60bd3c3 Compare November 17, 2022 16:49
@mrclary mrclary force-pushed the update-installers-conda branch 3 times, most recently from b925eac to a1dba82 Compare November 17, 2022 18:54
@mrclary mrclary force-pushed the update-installers-conda branch 2 times, most recently from c487ca2 to 42a5d74 Compare November 18, 2022 00:12
@mrclary
Copy link
Contributor Author

mrclary commented Nov 18, 2022

@ccordoba12, this should be ready for review now.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

A couple of minor comments for you @mrclary, otherwise looks good to me.

installers-conda/build_conda_pkgs.py Outdated Show resolved Hide resolved
@@ -1,313 +0,0 @@
From 40db10418cf9b46a70b48026d202d4474d62b99a Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

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

Why is this patch no longer necessary? I thought you needed to do several adjustments to our source code for the installers to work as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The patch is still necessary. However, I think it will be simpler to create the file on-demand rather than track and update it.

Currently, necessary changes are made on the installers-conda-patch branch, but then a separate PR must be made on the 5.x branch to update the patch file when the installers-conda-patch gets new commits. With this PR we don't need to track and update a patch file on the 5.x branch, rather the patch file is created when the spyder conda package is built.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for the clarification. I didn't understand where the patch went.

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @mrclary! These are really nice improvements!

@ccordoba12 ccordoba12 merged commit 9e25883 into spyder-ide:5.x Nov 18, 2022
ccordoba12 added a commit that referenced this pull request Nov 18, 2022
@mrclary mrclary deleted the update-installers-conda branch November 23, 2022 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants