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.

Sign up for GitHub

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

Use pypi as source #65

Closed
wants to merge 6 commits into from
Closed

Use pypi as source #65

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3ae7660
Use pypi as source
jamesmyatt Feb 13, 2020
506ef40
Bump build number
jamesmyatt Feb 13, 2020
8b81f78
MNT: Re-rendered with conda-build 3.18.11, conda-smithy 3.6.8, and co…
jamesmyatt Feb 13, 2020
905905e
Conda package name should be hard-coded
jamesmyatt Feb 14, 2020
8cb6ffd
Try to get conda build to find the right patch(es)
jamesmyatt Feb 14, 2020
814825e
patches are supposed to be in recipe directory
jamesmyatt Feb 19, 2020
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
2 changes: 1 addition & 1 deletion .azure-pipelines/azure-pipelines-osx.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .azure-pipelines/azure-pipelines-win.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .ci_support/linux_aarch64_python3.6.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ cdt_arch:
cdt_name:
- cos7
channel_sources:
- conda-forge,c4aarch64,defaults
- conda-forge
channel_targets:
- conda-forge main
docker_image:
Expand Down
2 changes: 1 addition & 1 deletion .ci_support/linux_aarch64_python3.7.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ cdt_arch:
cdt_name:
- cos7
channel_sources:
- conda-forge,c4aarch64,defaults
- conda-forge
channel_targets:
- conda-forge main
docker_image:
Expand Down
2 changes: 1 addition & 1 deletion .ci_support/linux_aarch64_python3.8.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ cdt_arch:
cdt_name:
- cos7
channel_sources:
- conda-forge,c4aarch64,defaults
- conda-forge
channel_targets:
- conda-forge main
docker_image:
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
{% set name = "ruamel.yaml" %}
Copy link
Member

Choose a reason for hiding this comment

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

There is already a ruamel.yaml package

Copy link
Author

Choose a reason for hiding this comment

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

This doesn't set the name of the package.
It just sets a variable that's used in the source/url templates.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but you use the same source as another package that is already on conda-forge. If that is really the case, this whole recipe should just be an alias package instead

Copy link
Author

Choose a reason for hiding this comment

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

This is just a fix for #64, so that this package can start updating again. There's a whole different discussion about whether this package should exist or not:

Copy link
Member

Choose a reason for hiding this comment

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

I think those are very coupled conversations

Copy link
Author

@jamesmyatt jamesmyatt Mar 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

How so? Surely. if it exists (which it does) then it's better to update it, so that bug and security fixes can be included.

Besides, as I understand it from conda-forge/ruamel.yaml-feedstock#7 (Specifically conda-forge/ruamel.yaml-feedstock#7 (comment)), this package is supposed to be different from the ruamel.yaml package (via patches), even if it's the same upstream source.

Copy link
Member

Choose a reason for hiding this comment

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

Is this builds exactly the same source as another feedstock, then why should both of them exist?

Copy link
Author

Choose a reason for hiding this comment

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

Because this version has patches (I think) so it's not exactly the same (I think)

Or is there an option to remove it?

Copy link
Member

Choose a reason for hiding this comment

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

But you are getting the source from the other package here I thought, which is why I was confused

Copy link
Author

Choose a reason for hiding this comment

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

Given #64, would it be better to just use the sourceforge repo as the new source?

{% set version = "0.15.80" %}

package:
name: ruamel_yaml
name: {{ name | lower | replace(".", "_") }}
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually a different package then?

Copy link
Author

Choose a reason for hiding this comment

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

As I understand it, this is a mirror of the Anaconda Inc fork (ruamel_yaml) of the upstream project (ruamel.yaml). I'll hard-code the conda package name, but the upstream package is "ruamel.yaml".

Copy link
Member

Choose a reason for hiding this comment

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

Right, but there is a separate ruamel.yaml package & feedstock for that

Copy link
Author

Choose a reason for hiding this comment

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

Right. But there's also a ruamel_yaml package that's based on the same upstream Python code.

Copy link
Author

@jamesmyatt jamesmyatt Feb 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest, I made this PR before I understood anything about the difference between the two packages, but now I wonder whether to give up on the "ruamel_yaml" package entirely.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think giving up on one of them is the right move, and I think it is this one.

version: {{ version }}

source:
url: https://bitbucket.org/ruamel/yaml/get/{{ version }}.tar.gz
sha256: 8529f909d8983a2b80bb3af60e425c418edf54f2fa132d863c43b3353f901203
url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
sha256: 4f203351575dba0829c7b1e5d376d08cf5f58e4a2b844e8ce552b3e41cd414e6
patches:
- ordereddict_test.patch

build:
number: 1000
number: 1001
no_link: .*\.(pyd|dll) # [win]

requirements:
Expand All @@ -38,7 +39,7 @@ test:
- ruamel_yaml.ext._ruamel_yaml

about:
home: https://bitbucket.org/ruamel/yaml
home: https://yaml.readthedocs.io/en/latest/
license: MIT
license_family: MIT
license_file: LICENSE
Expand Down