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

conda-smithy 3.23.0 #246

Merged
merged 6 commits into from
Feb 24, 2023
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{% set version = "3.22.1" %}
{% set version = "3.23.0" %}

package:
name: conda-smithy
version: {{ version }}

source:
url: https://github.com/conda-forge/conda-smithy/releases/download/v{{ version }}/conda-smithy-{{ version }}.tar.gz
sha256: 7d8c766ced494aada59f56b2693411c212826c685b762b467d2070120d66c275
sha256: 6808967b9dc81bb42200441a7dc60e13492095db64ca53d4e21ce72119bb58a8

build:
number: 0
Expand All @@ -23,6 +23,9 @@ requirements:
host:
- python >=3.6
- pip
- setuptools >=45
- setuptools-scm >=7
- tomli >=1.0.0
Comment on lines +26 to +28
Copy link
Member

Choose a reason for hiding this comment

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

Added these to line up with upstream build requirements included in this release

run:
- python >=3.6
- setuptools
Expand All @@ -40,6 +43,7 @@ requirements:
- vsts-python-api
- toolz
- scrypt
- shellcheck
Copy link
Member

Choose a reason for hiding this comment

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

Support for shellcheck was added in PR ( conda-forge/conda-smithy#1122 ) some time ago, but the requirement was not included here. Have added it. Though please let me know if I'm missing something

Copy link
Member Author

Choose a reason for hiding this comment

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

It was removed in https://github.com/conda-forge/conda-smithy-feedstock/pull/188/files because of some issue I don't remember. Maybe shellcheck crashed?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm...ok. Thanks for that detail. Unfortunate we don't have more history in that PR. Maybe @beckermr remembers more details?

Copy link
Member

Choose a reason for hiding this comment

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

I don’t recall either way but we don’t currently use it in conda forge for sure.

Copy link
Member

Choose a reason for hiding this comment

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

Was it related to issue ( conda-forge/conda-smithy#1316 )? AFAICT everything there has been fixed ( conda-forge/conda-smithy#1316 (comment) )

Copy link
Member

Choose a reason for hiding this comment

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

Thought shellcheck would only be used by the linter (when enabled), which is Linux x86_64 IIUC. If true, why would Linux ppc64le builds matter?

Copy link
Member

Choose a reason for hiding this comment

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

We wouldn’t be able to even use smithy on ppc64le regardless of the linter.

Copy link
Member

@jakirkham jakirkham Feb 23, 2023

Choose a reason for hiding this comment

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

We could add shellcheck to run_constrained

Edit: Would that be a fair compromise in general here? Considering we are concerned about fallout from making it a hard requirement?

Copy link
Member

Choose a reason for hiding this comment

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

Side note: Created PR ( conda-forge/shellcheck-feedstock#13 ) to add ppc64le to shellcheck. Unclear on why there wasn't a bot PR for this. Documented some history about what happened regarding shellcheck builds on other architectures.

Copy link
Member

Choose a reason for hiding this comment

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

Getting shellcheck to work on ppc64le appears to be non-trivial. Went ahead and moved to run_constrained and added a comment linking to issue ( #248 ). This at least provides a little context to recipe reviewers about what is going on there

- license-expression
- libarchive

Expand Down