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

Rebuild for Python 3.7, GCC 7, R 3.5.1, openBLAS 0.3.2 #60

Merged
merged 16 commits into from
Nov 12, 2018

Conversation

regro-cf-autotick-bot
Copy link
Contributor

It is likely this feedstock needs to be rebuilt.
Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.

This package has the following downstream children:
r-acepack
r-ada
r-ade4
r-adgoftest
r-aer
And potentially more.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one.

This PR was created by the cf-regro-autotick-bot.
The cf-regro-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. If you would like a local version of this bot, you might consider using rever. Rever is a tool for automating software releases and forms the backbone of the bot's conda-forge PRing capability. Rever is both conda (conda install -c conda-forge rever) and pip (pip install re-ver) installable.
Finally, feel free to drop us a line if there are any issues!

@jakirkham jakirkham mentioned this pull request Oct 13, 2018
5 tasks
@mariusvniekerk
Copy link
Member

wow, we're here already

@SylvainCorlay
Copy link
Member

wow, we're here already

so we have come to this.

so_it_has_come_to_this

@scopatz
Copy link
Member

scopatz commented Oct 20, 2018

This is currently blocking most of the compiler migration and only is failing on the new compilers on linux.

@bgruening
Copy link
Contributor

@scopatz; @mingwandroid had planned to align this with AR in this PR: #57 so that we can have something like noarch: generic for R.

@mingwandroid
Copy link
Contributor

mingwandroid commented Oct 20, 2018

Yeah we've had this in place for a while now, just not enabled it; I did a complete build out of 3.5.0 like this and it seemed fine. I threw it away as other parts of our infra weren't quite ready.

Should be good now though!

Also do we want to consider pinning to x.x for noarch generic r packages? I'd support that decision for sure.

@scopatz
Copy link
Member

scopatz commented Oct 20, 2018

That seems like a separate migration to me, no? Or do you want to update the compiler migration code to do this?

@bgruening
Copy link
Contributor

@mingwandroid last time we discussed to pin to x.x. you argued strongly to not do it because R breaks API. Is that not true anymore?

@mingwandroid
Copy link
Contributor

I was talking specifically about packages that needed compilation back then AFAIR, but my worries about that are reduced too now that we have better / less ad-hoc pinning mechanisms.

@mingwandroid
Copy link
Contributor

That seems like a separate migration to me, no? Or do you want to update the compiler migration code to do this?

It would just involve a minor change to conda skeleton cran unless I'm overlooking something the compilers aren't really involved (esp. if we're talking about noarch: generic only).

@mingwandroid
Copy link
Contributor

mingwandroid commented Oct 20, 2018

Also, back then I had concerns about updating the MSYS2 infra mid-minor release, but I've not had the time to do that.

Longer term, now that R upstream are (hopefully) adopting the latest MSYS2 toolchains and libraries (even contemplating calling pacman during builds, but lets hope some offline caching is also supported!) I believe we may want to switch to binary repackaging for Windows (only, macOS CRAN binaries have proven problematic; latest binaries not always provided for macOS 10.9, linking to various non-existent dylibs in /usr/local for example .. luckily conda-build catches that now-a-days).

@mingwandroid
Copy link
Contributor

The downside of repackaging CRAN Windows binaries is static linking. This is the preferred way for CRAN and they seem to be strongly in favour of doing things that way.

AD and conda-forge strongly prefer dynamic for lots of better (IMHO) but different reasons so we'd lose that advantage, though give that I've never found time to refresh our MSYS2 packages that's not so relevant here. We could also try to adopt the MSYS2 CRAN RTools toolchain and libraries instead,. They are much more modern. We could then investigate removing the static preference if that's the concensus. Needs a meeting overall. I percieve that Windows R on conda-forge gets a bit less attention than the other platforms (failing builds getting disabled), is that a fair assessement? On AD I try to ensure that everything that I build is provided on all OSes. Repackaging would obviously make this issue go away. conda skeleton cran is now capable of mixed binary repackaging and build from source btw and of using RTools instead of MSYS2. For the MRO ecosystem, I repackage where it works and build using RTools where it doesn't. It seems to work out well.

Anyway, I reckon these are meeting-worthy discussions. Can we add it to some agenda and invite me?

@bgruening
Copy link
Contributor

I percieve that Windows R on conda-forge gets a bit less attention than the other platforms (failing builds getting disabled), is that a fair assessement?

I think this is a fair statement.

Anyway, I reckon these are meeting-worthy discussions. Can we add it to some agenda and invite me?

Yes, seems like it. Lets add it to one of the next conda-forge meetings: see here: https://paper.dropbox.com/doc/2018-10-30-conda-forge-meeting--APvs1n0U86RqplH6tZTCo3IZAQ-a81WsZAhNzAQYd2qC6z1a (please note, I'm not sure about the date yet)

@mingwandroid how do we solve this in the short term. We need to get a rebuild of r-base with the new compilers in. Skip the noarch for the moment and revise this later with r-base 3.6? Can you help with this PR here?

@isuruf
Copy link
Member

isuruf commented Oct 20, 2018

I would suggest at least getting the windows layout changed. Let's not push it for 3.6

@mingwandroid
Copy link
Contributor

I'm up for that too @isuruf, got a good few work things to tie up first though, will try to get onto this either tomorrow or early next week.

@isuruf
Copy link
Member

isuruf commented Oct 20, 2018

Thanks @mingwandroid. Btw, why does noarch:generic packages have to be tied to a specific r-base version?

@mingwandroid
Copy link
Contributor

mingwandroid commented Oct 20, 2018

CRAN pins to x.x globally for binaries. They build them out at the .0 patch release and I think may do some updates for new versions at later patch releases (though I'm not sure).

You can see an MRO repackaging recipe (windows and mac repackaged, no such binaries exist for linux so I compile from source using our compilers instead) here

Note the binary folder names:

  url: https://cran.microsoft.com/snapshot/2018-08-01/bin/windows/contrib/3.5/jsonlite_1.5.zip  # [win64]
  sha256: 4589baa2119e34c286e6f8ee5e9029703cf16fbc61af0d740b182229cd42b183  # [win64]

  url: https://cran.microsoft.com/snapshot/2018-08-01/bin/macosx/el-capitan/contrib/3.5/jsonlite_1.5.tgz  # [osx]
sha256: 415a0bdb884e40ad42df0b3a38919607084927746ee4e1de0d8fc5ab7a24ca0c  # [osx]

@mingwandroid
Copy link
Contributor

Btw, why does noarch:generic packages have to be tied to a specific r-base version?

Because the bytecode is not recompiled at install time and is incompatible between minor releases.

@jakirkham jakirkham mentioned this pull request Oct 21, 2018
6 tasks
@dhirschfeld
Copy link
Member

Just piping up as a Windows R user here. The biggest pain-point for me is the incompatibility between defaults and conda-forge when it comes to R packages. IIUC that's because conda-forge need to implement the _rmutex scheme as described in #34 (comment).

I think it would be unfortunate to publish a new R version before this issue is resolved as it leads to very hard-to-avoid segfaults. My hope is that I can avoid the incompatibility in future simply by specifying r-base>=3.5

@mingwandroid
Copy link
Contributor

While we do care about compatibility and are looking to achieve it as co-operatively as possible, we have obligations to meet wrt release schedules and cannot hold those up without good reason.

Currently, while we are known to have some bad binary compat. issues, I feel I must say it again: At present mixing AD defaults channel with conda-forge is very likely to result in problems.

Without meaning to sound too snarky @dhirschfeld, you're a pretty advanced user, so how come you're trying this?

@dhirschfeld
Copy link
Member

Not all packages are in defaults and because conda-forge doesn't use mkl or mro we want to use the defaults packages for performance, where that makes a difference. For those packages I pin them to defaults, otherwise I'd simply like to have the latest package from either channel.

The binary incompatibility between the channels makes that very difficult to achieve and if you do have a stable environment your next conda update is highly likely to break it. I have open issues against conda for features which would make this incompatibility easier to manage - e.g. conda/conda#7678, conda/conda#3149 and others.

I might be an advanced user but I manage a Python/R analytics environment for much less sophisticated users and at present it's too easy for them to shoot themselves in the foot without resorting to locking down their environment.

It just seemed that a big change like this presented a good opportunity to also fix the incompatibility with defaults. Maybe it's better to tackle one thing at a time though.

we have obligations to meet wrt release schedules

I was just suggesting holding up the conda-forge release. If that somehow impacts anaconda then sure, that's probably a much bigger concern. It might also be that upgrading to R-3.5 is more important for more people than the incompatibility - this is just my personal bugbear!

@mingwandroid
Copy link
Contributor

mingwandroid commented Oct 21, 2018

Thanks @dhirschfeld, it's really helpful to get these 'field reports', we usually only see them when in issues where things have gone wrong!

BTW, how is MRO holding up against the older R infra (we use openblas here but had to disable libgomp recently as it is not fork-safe and therefore is unsuitable for use with things like MPI, or in general anything that wants to fork while running libgomp threads).

I've done some benchmarks since and the performance delta wasn't vast, but I only tested on a 4 core machine. You should be able to reproduce my results with docker via https://github.com/AnacondaRecipes/mro-docker if you were interested. Adding conda-forge into the mix would also be interesting I think.

Definitely with tackling one thing at a time. I cannot multitask too well!

@mingwandroid
Copy link
Contributor

Do we want to hold off until R 3.5.2 for this change of layout btw? If so do I need to unstick this at 3.5.1 without the layout changes?

If we don't rebuild and release all the R windows packages in one go we'll break things.

@mingwandroid
Copy link
Contributor

Also should we go for x.x pinning, at least for noarch: generic ones for now? I can make the changes to conda skeleton cran if so. It is definitely my preference.

@bgruening
Copy link
Contributor

Also should we go for x.x pinning, at least for noarch: generic ones for now? I can make the changes to conda skeleton cran if so. It is definitely my preference.

👍 for this from my side.

Do we want to hold off until R 3.5.2 for this change of layout btw? If so do I need to unstick this at 3.5.1 without the layout changes?

I leave this to @isuruf and others. I would like to get the mass migration done and not postpone it anytime longer, so whatever is beneficial there gets my vote :)

@isuruf
Copy link
Member

isuruf commented Oct 21, 2018

-----begin rant-----
We agreed upon to put this off for 3.5 series, and then 3.5.1 was released without the layout change. Then we agreed upon to do the updates to 3.5.1 and compiler update at the same time, but then conda-forge-pinning was updated unconditionally (instead of how we did for python 3.7) and people started updating to 3.5.1. Now, the only option left (unless we mark all 3.5.1 windows packages as broken) is to wait for 3.5.2, but we agreed upon to release only 3.x.1 releases, so that we don't have to rebuild for each patch version

Let's just move on without the windows layout change since people want things to be done quickly instead of sensibly and build for all platforms 550+ packages (out of 750 r packages) that could potentially be noarch:generic and compatible with defaults with a x.x (which was one of the major points of compiler upgrade). Layout change and noarch:generic has to be postponed to 3.6 series.
-----end rant-----

@mingwandroid
Copy link
Contributor

We agreed upon to put this off for 3.5 series, and then 3.5.1 was released without the layout change. Then we agreed upon to do the updates to 3.5.1 and compiler update at the same time, but then conda-forge-pinning was updated unconditionally (instead of how we did for python 3.7) and people started updating to 3.5.1. Now, the only option left (unless we mark all 3.5.1 windows packages as broken) is to wait for 3.5.2, but we agreed upon to release only 3.x.1 releases, so that we don't have to rebuild for each patch version

OK, well I wasn't party to much of the latter half of these decisions which is fine, I just need to know that if I change the layout on a 3.5.1 build (which I'm fine with doing) that all the Windows and noarch packages will be rebuilt before r-base 3.5.1 is released. Is the something someone can setup here? I know it can, but really I'm asking about what I need to do on my end? What build number, how do we bump all the r package build numbers etc?

@mariusvniekerk
Copy link
Member

mariusvniekerk commented Nov 7, 2018

Something that might be problematic on osx gcc7

    gsl:             2.2.1-h002c638_3           defaults              
    libcurl:         7.59.0-hf30b1f0_0          defaults     
    curl:            7.59.0-ha441bb4_0          defaults         

during the host install

osx configure fails with linker issues. See logs now printed for details

unknown option -rpath-link might be a culprit

@scopatz
Copy link
Member

scopatz commented Nov 7, 2018

And we are all green here!

@isuruf
Copy link
Member

isuruf commented Nov 7, 2018

Thanks @scopatz.
Can you try applying the commit 23f7254 ?

This is so that we can share noarch packages directly.
@scopatz
Copy link
Member

scopatz commented Nov 7, 2018

Ok @isuruf, I have cherry-picked it in!

@scopatz
Copy link
Member

scopatz commented Nov 7, 2018

So that failed @isuruf, so I think we should revert and merge to kick off the rest migrations. This kind of capability can be worked through in another PR.

@jakirkham
Copy link
Member

@mingwandroid, do you know why that last build is failing on Windows?

@mingwandroid
Copy link
Contributor

I'll take a look.

@scopatz
Copy link
Member

scopatz commented Nov 7, 2018

R -h fails to run, is the issue. Note that without bc057ae, everything runs fine.

@mariusvniekerk
Copy link
Member

mariusvniekerk commented Nov 8, 2018

@scopatz so the osx64 gcc7 build is building with the wrong compiler i think
we need to apply some of the changes from the build.sh on AnacondaRecipes here.

See: 3dca792#diff-44a73bcc045c193c3bd45da87994b03bR312

@scopatz
Copy link
Member

scopatz commented Nov 8, 2018

@mariusvniekerk - I don't understand what you mean, exactly. It seems to be using clang, as needed, right?

@scopatz
Copy link
Member

scopatz commented Nov 8, 2018

I think we are back in business with mac and it is now using the correct compilers.

@isuruf
Copy link
Member

isuruf commented Nov 8, 2018

WARNING (r-base,lib/R/lib/libR.dylib): Needed DSO /usr/lib/libbz2.1.0.dylib not found in any CDT/compiler package, nor the whitelist?!

@scopatz
Copy link
Member

scopatz commented Nov 8, 2018

Alright, I have fixed the DSO warnings on Mac on the new compilers

@scopatz
Copy link
Member

scopatz commented Nov 9, 2018

And we are all green again!

@CJ-Wright
Copy link
Member

@conda-forge/r-base I think this is ready for review/merge.

@isuruf
Copy link
Member

isuruf commented Nov 9, 2018

@CJ-Wright, not yet. Windows layout is not changed yet and since almost everyone wanted to do it, we should do it.

@mingwandroid
Copy link
Contributor

Sorry I've not had time to look at this yet.

@scopatz
Copy link
Member

scopatz commented Nov 9, 2018

I think we can and should wait to change the windows layout since there is no one willing to do it on a reasonable time frame

@mingwandroid
Copy link
Contributor

I have permission to look into this today. I cannot guarantee I will get it fixed today but I will try to.

@mariusvniekerk mariusvniekerk mentioned this pull request Nov 12, 2018
5 tasks
@mariusvniekerk mariusvniekerk merged commit 76f6bb9 into conda-forge:master Nov 12, 2018
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.