Update 0.11.14 #3

jakirkham · 2016-11-06T22:01:02Z

Closes #2
Closes #7

I suspect this will fix issue ( conda-forge/conda-smithy#351 ).

Pulled these changes from anaconda-recipes mainly with some other tweaks.

cc @mwcraig @MSeifert04 @conda-forge/core

conda-forge-linter · 2016-11-06T22:01:05Z

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.

MSeifert04 · 2016-11-06T22:05:40Z

recipe/__init__.py

+# coding: utf-8
+from __future__ import print_function, absolute_import
+
+__version__ = "0.11.7"


This seems wrong compared with the 0.11.14 everywhere else...

This is replaced by this sed call.

Note it is run on Unix and Windows.

That said I can pick something else it looks like. For example, __version__ = "$VERSION" or similar as it replaces all lines with __version__ in them.

jakirkham · 2016-11-06T22:20:21Z

@kalefranz can you please advise how to proceed with this?

mwcraig

Never tried building this so can't really make any intelligent comments on the broader issue...but the immediate build problem is an easy fix.

mwcraig · 2016-11-07T01:49:00Z

recipe/meta.yaml

@@ -1,8 +1,6 @@
-{% set version = "0.11.7" %}


Removing this is causing a fail when the recipe tries to render (see comment below )

Yeah there was an error when staging. Fixing it now.

Note: Still shows up in the diff as the version was bumped. Please see the line below for the version bump.

mwcraig · 2016-11-07T01:49:31Z

recipe/meta.yaml

 package:
  name: ruamel_yaml
-  version: 0.11.7
+  version: 0.11.14

 source:
  fn: ruamel_yaml-{{ version }}.tar.gz


Actually, seems like {{ version }} may as well be used in the package section too.

Good point. 👍

mwcraig · 2016-11-07T01:56:39Z

One other comment: any reason not to grab the source from the mercurial repo like the continuum recipe does?

jakirkham · 2016-11-07T05:25:33Z

One other comment: any reason not to grab the source from the mercurial repo like the continuum recipe does?

We have preferred downloading tarballs and the like. As that actually does work ok here (with the right URL), it makes sense that we go ahead and do it.

jakirkham · 2016-11-07T05:54:15Z

FYI have submitted most of the changes to these patches upstream in PR ( ContinuumIO/anaconda-recipes#57 ) assuming that is the official source.

jakirkham · 2016-11-07T05:57:09Z

I think I kind of have this working. Would be good to get another review of these changes.

jakirkham · 2016-11-07T22:42:23Z

@mingwandroid , could really use your help sorting this out. 😟

mingwandroid · 2016-11-07T23:34:49Z

I'll try to take a look tomorrow.

kalefranz · 2016-11-23T19:02:59Z

Sorry I'm late to the party here. Just saw this. Will try to get some time today to work on it.

conda-forge-linter · 2017-01-06T04:34:08Z

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.

jakirkham · 2017-01-06T04:46:49Z

Appears all that was needed was to change m2-bash to posix. Thanks to @pkgw for figuring that out. Appears this is passing on AppVeyor (~~status didn't update in GitHub~~ fixed) and everywhere else. Would appreciate a final review and merge.

pkgw · 2017-01-06T21:05:39Z

This version still has the weird problem with pulling out the version from the source files on Windows; note this line in the log file. That gets propagated into the setup.py having an empty version string. My PR fixes that by restructuring the bash script a bit (although I won't claim to understand why the version here doesn't work). So, I think my PR should be merged rather than this one.

jakirkham · 2017-01-07T21:09:03Z

recipe/prepare.bash

+RECIPE_DIR=${1-$RECIPE_DIR}
+SRC_DIR=${2-$SRC_DIR}
+
+VERSION=$(cd $SRC_DIR && python setup.py --version)


Thanks for raising the version point on AppVeyor, @pkgw , was not aware of that issue. For whatever reason, this seemed to not affect Linux and macOS, but in retrospect it probably should.

It seems that before python setup.py --version was not being run in the source directory, but instead the path to setup.py was being provided. From past experience, have found this to be a bad idea. So have updated this line so that we switch to the source directory. As can be seen by the referenced build, the problem goes away with this change.

xref: https://ci.appveyor.com/project/jakirkham/ruamel-yaml-feedstock/build/1.0.1/job/oetlp37xtcp8eima#L586

jakirkham · 2017-01-07T22:08:51Z

Have submitted some additional fixes based off this work upstream in PR ( ContinuumIO/anaconda-recipes#72 ).

pkgw · 2017-01-08T17:46:43Z

Time to merge one of these bad boys and finally get this released?

This includes a ton of patches and other changes from upstream and various other modifications to make them compatible at conda-forge.

jakirkham · 2017-01-09T06:06:29Z

Are there any more concerns that you think need to be addressed in this PR, @pkgw and/or others?

Note that upstream is making some rather large changes to how they build ruamel_yaml. However, I don't think we need to address them right now. They can certainly be addressed in a follow-up PR.

pkgw · 2017-01-10T13:53:50Z

The build logs look OK to me, so I guess I'd say go for it. I'd be happier if there was a way to test more thoroughly since this package is so low in the dependency stack ... is there?

jjhelmus · 2017-01-11T02:33:24Z

LGTM. Agree with @pkgw that a better test method would be good since if this package is broken so is conda. There is a _test directory in the source directory. Is there a reasonable way to run those with pytest?

pkgw · 2017-01-11T14:44:09Z

Running internal tests is, obviously, good, but I don't think you can become really confident until you test out installation of the package in a full Conda environment. You could imagine adding some commands to the test suite that exercise conda and friends to see that they still work, I suppose?

Linux distributions have been playing this game a long time and they all have "testing" channels where new packages are deployed and, hopefully, given some real-world testing before being promoted to be distributed to the full user base. Would be nice to work toward having a similar setup for conda-forge.

jakirkham · 2017-01-11T17:54:55Z

These are all good ideas.

Some of them have come up in some form before. For example, have some additional package install step after the build, but before upload. This would be done for things like conda, conda-build, python, perhaps this, etc. Also downloading and testing random packages. Initial motivation was for handling ABI issues that may arise.

This all said, many of these are rather nebulous and involve changes that extend beyond this update PR. I'm in favor of discussing and pursuing them as they become better formed, but don't think this PR is the proper context for doing so. Please feel free to follow-up on those issue and share other thoughts/suggestions or raise new ones if needed.

xref: conda-forge/conda-smithy#253
xref: conda-forge/conda-forge.github.io#113

pkgw · 2017-01-12T13:44:38Z

Agreed. @jakirkham I think it's time to merge this.

jakirkham · 2017-01-13T21:39:40Z

Thanks @pkgw. Would someone like to do the honors?

mwcraig · 2017-01-13T21:41:32Z

Sure, I'll do the easy part 😀

jakirkham · 2017-01-13T21:54:50Z

Thanks @mwcraig.

MSeifert04 reviewed Nov 6, 2016

View reviewed changes

jakirkham mentioned this pull request Nov 6, 2016

Package version 0.11.14 #2

Closed

mwcraig reviewed Nov 7, 2016

View reviewed changes

kalefranz mentioned this pull request Nov 23, 2016

AppVeyor VS2008 slow startup/memory issues conda-forge/conda-smithy#351

Closed

jakirkham mentioned this pull request Jan 6, 2017

A different attempt at fixing Windows #7

Closed

jakirkham changed the title ~~WIP: Update 0.11.14~~ Update 0.11.14 Jan 6, 2017

jakirkham commented Jan 7, 2017

View reviewed changes

jakirkham mentioned this pull request Jan 7, 2017

More ruamel fixes ContinuumIO/anaconda-recipes#72

Closed

Update to 0.11.14.

b4c2056

This includes a ton of patches and other changes from upstream and various other modifications to make them compatible at conda-forge.

mwcraig merged commit 71fb31b into conda-forge:master Jan 13, 2017

jakirkham deleted the update_0.11.14 branch January 13, 2017 21:54

Update 0.11.14 #3

Update 0.11.14 #3

Conversation

jakirkham commented Nov 6, 2016 • edited Loading

conda-forge-linter commented Nov 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Nov 6, 2016

mwcraig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham Nov 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwcraig commented Nov 7, 2016

jakirkham commented Nov 7, 2016

jakirkham commented Nov 7, 2016

jakirkham commented Nov 7, 2016

jakirkham commented Nov 7, 2016

mingwandroid commented Nov 7, 2016

kalefranz commented Nov 23, 2016

conda-forge-linter commented Jan 6, 2017

jakirkham commented Jan 6, 2017 • edited Loading

pkgw commented Jan 6, 2017

Choose a reason for hiding this comment

jakirkham commented Jan 7, 2017

pkgw commented Jan 8, 2017

jakirkham commented Jan 9, 2017

pkgw commented Jan 10, 2017

jjhelmus commented Jan 11, 2017

pkgw commented Jan 11, 2017

jakirkham commented Jan 11, 2017

pkgw commented Jan 12, 2017

jakirkham commented Jan 13, 2017

mwcraig commented Jan 13, 2017

jakirkham commented Jan 13, 2017

jakirkham commented Nov 6, 2016 •

edited

Loading

jakirkham Nov 7, 2016 •

edited

Loading

jakirkham commented Jan 6, 2017 •

edited

Loading