search

conda-forge / ninja-feedstock

A conda-smithy repository for ninja.
BSD 3-Clause "New" or "Revised" License
2 stars 18 forks source link

Upgrade to using conda-smithy 3 and conda-build 3 #7

Closed jakirkham closed 6 years ago

jakirkham commented 6 years ago

Makes some changes to the recipe to make it compatible with conda-smithy 3 and use conda-build 3.

conda-forge-linter commented 6 years ago

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:

conda-forge-linter commented 6 years ago

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 commented 6 years ago

@msarahan, could you please take a look and let me know if I did this correctly?

jakirkham commented 6 years ago

So at the end of the AppVeyor build, saw that vs2015_win-64 was being pulled in. What's the reason for having the 32-bit and 64-bit VS runtimes include this info in the package names?

msarahan commented 6 years ago

that's not a runtime. That's a compiler package. Runtimes have "runtime" in the package name.

jakirkham commented 6 years ago

@conda-forge/ninja, please review.

frol commented 6 years ago

LGTM

jakirkham commented 6 years ago

Good to merge?

isuruf commented 6 years ago

+1. Can you open an issue upstream about <vsyear>_runtime run_exports ?

msarahan commented 6 years ago

vc depends on vs20??_runtime. You don't need them both. You only need to add it manually when you're not depending on the run_exports addition of vc.

On Fri, May 11, 2018 at 2:55 PM, Isuru Fernando notifications@github.com wrote:

+1. Can you open an issue upstream about _runtime run_exports ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conda-forge/ninja-feedstock/pull/7#issuecomment-388469501, or mute the thread https://github.com/notifications/unsubscribe-auth/AACV-bbTGkkylqfSePBbPsYKD-FhSzMpks5txeyUgaJpZM4TxPRV .

isuruf commented 6 years ago

See my comment at https://github.com/conda-forge/ninja-feedstock/pull/7#discussion_r186900102. It should be enough to ignore vc run_exports. vs2015_runtime should be added regardless.

msarahan commented 6 years ago

Please consider submitting a PR to https://github.com/AnacondaRecipes/aggregate

in the https://github.com/AnacondaRecipes/aggregate/tree/master/vs2015 and https://github.com/AnacondaRecipes/aggregate/tree/master/vs2008

folders. You could also copy those folders and make proper feedstocks out of them via staged recipes. That would be ideal. We are overbooked and don't have time for that right now.

On Fri, May 11, 2018 at 3:07 PM, Isuru Fernando notifications@github.com wrote:

See my comment at #7 (comment) https://github.com/conda-forge/ninja-feedstock/pull/7#discussion_r186900102. It should be enough to ignore vc run_exports. vs2015_runtime should be added regardless.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conda-forge/ninja-feedstock/pull/7#issuecomment-388472458, or mute the thread https://github.com/notifications/unsubscribe-auth/AACV-fUvpbV1_T4k7YhkwKc0Oo9ZnAssks5txe-OgaJpZM4TxPRV .

mingwandroid commented 6 years ago

IMHO for ninja you should just remove the python dependency, it is pointless; I doubt anyone has ever used this feature (it is a daemon for sending build logs to a website).

msarahan commented 6 years ago

Unfortunately, I don't think that simplifies much. We still need to associate the runtime with it, and we need to not have the mutex that the VC package imposes. Getting rid of the Python deep might speed up installation a bit.

On Sunday, May 13, 2018, Ray Donnelly notifications@github.com wrote:

IMHO for ninja you should just remove the python dependency, it is pointless; I doubt anyone has ever used this feature (it is a daemon for sending build logs to a website).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.< https://ci3.googleusercontent.com/proxy/V1pFzozXiV0DBYisVi3iJm3CcIcP7RE_8O6LeMS0m7DQKjQSL8LkYIxd8R92mqGpFl8OPv0Ef37NRVnZEuZmdY2sksRPUB8hsc_DjPNXHe1_vB8uvbPkXL4TOQrrkYb4fcTLNuxN96N8bwCRQimNp65Hmr3jbg=s0-d-e1-ft#https://github.com/notifications/beacon/AACV-bU-TthN8QAL57jcuFBt33iNeyuLks5tyCVxgaJpZM4TxPRV.gif>

mingwandroid commented 6 years ago

While ninja depends upon python, that vc= mutex is always going to be an issue though is it not?

isuruf commented 6 years ago

It doesn't depend on python. Python is a build (not host) dependency to run the configure.py script which bootstraps ninja.

mingwandroid commented 6 years ago

Ah ok, thanks.

jakirkham commented 6 years ago

Interesting about the optional python run time dependency. Guess it could be useful in some build farm. Though agree we probably don’t need this.

@isuruf is correct. We are using this as part of the build process (as before) nothing more.