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

App entry #7

Merged
merged 7 commits into from
Apr 25, 2016
Merged

App entry #7

merged 7 commits into from
Apr 25, 2016

Conversation

melund
Copy link
Contributor

@melund melund commented Apr 22, 2016

@scopatz @ocefpaf . I recreated the PR from my fork.

@conda-forge-linter
Copy link

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.

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

This failed on Python 3.4 on windows for some reason @melund. Seems like PLY wasn't installed? Or maybe it was some other dependency.... Any ideas?

@melund
Copy link
Contributor Author

melund commented Apr 22, 2016

@scopatz This seems weird. I will try to build manually to see if I get the same error. There are a few other updates to the recipe that we need port over. But none that can explain this.

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

Yeah, this one is super weird...

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

@conda-forge-linter
Copy link

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.

@melund
Copy link
Contributor Author

melund commented Apr 22, 2016

@scopatz . I can build locally on 3.4 without any problems.

Btw. Should conda dependency on prompt-toolkit/pygments be windows only?

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

Btw. Should conda dependency on prompt-toolkit/pygments be windows only?

Probably not, it is nice for it to come with all the bells and whistles. I think that ptk is now working well enough that it is a clear improvement over readline. This recipe is from before that time.

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

Huh, this is very strange. Do you have any idea what is going on @ocefpaf @pelson or @jakirkham? It only fails on Python 3.4 on windows. Other windows builds work just fine.

@jakirkham
Copy link
Member

(On my phone) Is it failing on Python 3.4 64-bit only?

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

Yes, win 64, py 3.4 only. But it failing with trying to use setuptools to download dependencies that have already been downloaded by conda (I think)

@jakirkham
Copy link
Member

Does it appear to be installing everything to the proper prefix for _build/_test or is it installing stuff too some other potentially higher directory?

@scopatz
Copy link
Member

scopatz commented Apr 22, 2016

Ahh, yeah. Maybe we need to remove the bld.bat file....

@jakirkham
Copy link
Member

Not sure if that's it. This is the second such case of this I have seen. Both follow the recent conda-build update. Maybe it's not a bug, but am thinking it is. See this other case. Would you please report this to conda-build, @scopatz? Also cc myself @msarahan and @patricksnape?

@jakirkham
Copy link
Member

Could you try an experiment for me? Try adding something like this to the appveyor.yml. Preferably after any other conda install steps. Please let me know what happens.

        - cmd: if "%TARGET_ARCH%" == "x64" if "%CONDA_PY%" == "27" conda install conda-build=1.20.0

@melund
Copy link
Contributor Author

melund commented Apr 23, 2016

@jakirkham @scopatz . I added the line to downgrade conda-build to version 1.20.0. That seemed to do the trick...

@@ -53,6 +53,7 @@ install:
- cmd: conda config --add channels http://conda.binstar.org/conda-forge
- cmd: conda info
- cmd: conda install -n root --quiet --yes conda-build anaconda-client jinja2 setuptools
- cmd: if "%TARGET_ARCH%" == "x64" if "%CONDA_PY%" == "34" conda install conda-build=1.20.0 --yes
Copy link
Member

@ocefpaf ocefpaf Apr 25, 2016

Choose a reason for hiding this comment

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

@melund I was away, but it seems that @jakirkham fixed that in conda-forge/conda-smithy#143

So in the future we can use conda-smithy to update the feedstock, but right now I will merge this as a workaround.

BTW conda-smithy is tool to update the feedstocks, lint the recipes, and much more. You can install it with

conda install -c conda-forge conda-smithy

and then you can update the feedstock with

conda smithy rerender

from the feedstock directory.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Actually that is not true, @ocefpaf. I only fixed Travis CI and for a different issue. This was my best guess while working on my cell in the airport waiting area moments before boarding and haven't had a chance to look until now. We need to add this in a new PR to conda-smithy with @melund's change.

Copy link
Member

Choose a reason for hiding this comment

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

I actually just notice that. I was confused due to the same version we had to roll bacl.

We need to add this in a new PR to conda-smithy with @melund's change.

Not sure about that. I am fixing the new feedstocks only and this is fixed in staged recipes. So let's hope that a new conda-build is out soon to avoid modifying conda-smithy.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's confusing for sure. There were two AFAICT unrelated bugs.

Not sure about that. I am fixing the new feedstocks only and this is fixed in staged recipes. So let's hope that a new conda-build is out soon to avoid modifying conda-smithy.

I don't think this is a universal problem. There are some specific cases where it occurred, but again this was merely a thought of how to fix before leaving. So, haven't had time to grok everything that has happened with it yet.

@ocefpaf ocefpaf merged commit 9c8e484 into conda-forge:master Apr 25, 2016
@melund
Copy link
Contributor Author

melund commented Apr 25, 2016

@ocefpaf Thanks a lot. I will try to play around with conda smithy

@pelson
Copy link
Member

pelson commented Apr 25, 2016

👍

Looks like the bld.bat can be removed though if you wanted to.

@melund
Copy link
Contributor Author

melund commented Apr 25, 2016

Great I didn't know that conda-build finally fixed this. I will try to use conda-smithy to update the feedstock and remove the bld.bat file

@melund
Copy link
Contributor Author

melund commented Apr 25, 2016

I have update the recipe again #8

@jakirkham
Copy link
Member

I added the line to downgrade conda-build to version 1.20.0. That seemed to do the trick...

Thanks for trying, @melund. Now we know roughly where the issue is.

This hasn't entered conda-smithy yet. So even though you were unaware that conda-smithy can and should be used for these sorts of changes, there wouldn't have been a fix for this issue in conda-smithy to use. Will work on that.

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.

None yet

6 participants