Add yaml #397

jakirkham · 2016-04-19T03:19:17Z

Adds a recipe to build yaml. Written from scratch. Seems to work ok and is building the same version in defaults so shouldn't cause conflicts by its addition.

Would really appreciate any feedback on this. The Windows portion was just my best guess at what might work and could be totally off. The Mac and Linux builds actually seem to work ok.

Windows needs:

Headers.
Patch for DLL.
Submit patch upstream ( see this PR Support static and dynamic libraries yaml/libyaml#10 )

conda-forge-admin · 2016-04-19T03:19:19Z

Hi! This is the friendly conda-forge-admin automated user.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/yaml) and found it was in an excellent condition.

jakirkham · 2016-04-19T03:19:45Z

cc @msarahan

jakirkham · 2016-04-19T03:57:56Z

Please don't merge yet. Am waiting for feedback from @msarahan as to whether this recipe is in line with Continuum's build and if there is anything needing fixing or improvement.

jakirkham · 2016-04-19T06:47:03Z

So, it didn't have an install step. Right now, I am copying over the library. However, we should be copying over the headers too. Also, it only builds a static library, but a dynamic library should be doable too. This would require a very simple patch.

jakirkham · 2016-04-20T03:18:57Z

recipes/yaml/meta.yaml

+  patches:
+    # Change CMakeLists so that we can build static and shared libraries.
+    # See this PR ( https://github.com/yaml/libyaml/pull/10 ) for details.
+    - CMakeLists.txt.patch


This patch ( yaml/libyaml#10 ) let's us build DLLs and LIBs. It was the simplest thing I could come up with. Though improvements are certainly welcome.

patricksnape · 2016-04-20T08:29:29Z

recipes/yaml/CMakeLists.txt.patch

+ add_definitions (-DHAVE_CONFIG_H -DYAML_DECLARE_STATIC)
+-add_library (yaml STATIC ${SRC})
+-
+add_library (yaml ${LIB_TYPE} ${SRC})


Going by the CMake file by @gillins on the jpeg recipe, I think you can achieve the same effect by adding two libraries.

yes, adding 2 different ones is the right approach

Moreover, you should prefer it, as it gives you both a static lib and a dynamic lib. Might need to change the name of the static lib, though, so they don't overlap.

maybe yaml_s

The problem with doing this is this breaks with existing yaml package in defaults. If you could share what you guys are doing to build it there, I would be happy to update this even if it is shared verbally, but I don't think it is a good idea to rename this library and break with the defaults yaml package.

It looks like someone created or copied a custom VS solution and projects. This is "land before time" stuff - no history in our repo on it. File modification times between 2010 and 2012.

Suffice to say, the DLL is the one that everyone uses. It should have the default name. The static one can safely have some other name, IMHO (and the defaults channel package does not include it, anyway).

Would there be a way to have both libraries have the same name and only do one build? I initially tried to find something like this, but was unable to get anything like that to work.

No, Windows has 2 different things both with .lib, and you need both of
them. One filename has to change to have both in one package.

On Thu, Apr 21, 2016, 20:04 jakirkham [email protected] wrote:

In recipes/yaml/CMakeLists.txt.patch
#397 (comment)
:

++OPTION (BUILD_SHARED_LIBS "Build Shared Libraries" OFF)
++SET (LIB_TYPE STATIC)
++IF (BUILD_SHARED_LIBS)
++ SET (LIB_TYPE SHARED)
++ENDIF (BUILD_SHARED_LIBS)
++

set (YAML_VERSION_MAJOR 0)

set (YAML_VERSION_MINOR 1)

set (YAML_VERSION_PATCH 6)
+@@ -12,5 +19,4 @@ file (GLOB SRC src/*.c)
+

include_directories (include win32)

add_definitions (-DHAVE_CONFIG_H -DYAML_DECLARE_STATIC)
+-add_library (yaml STATIC ${SRC})
+-
++add_library (yaml ${LIB_TYPE} ${SRC})

Would there be a way to have both libraries have the same name and only do
one build? I initially tried to find something like this, but was unable to
get anything like that to work.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/conda-forge/staged-recipes/pull/397/files/20da14e8df6aa1519d098c524871ea2191674708#r60678605

You have to make sure the import lib for the DLL doesn't get overwritten by your static lib so probably safest to have different names. I was also unable to work out how to do one build for static/shared with cmake for jpeg. This might be because on Linux (and some other OS's) static and shared builds need different compile flags.

This commit ( 9295684 ) from @msarahan should resolve it.

Sorry to be necromancing an ancient topic, but this static lib name change did indeed break a number of things (as some of the downstream consumers of libyaml are very stable and not often updated, it's taken awhile to discover the issues). We've changed the default back, but made it configurable from cmake (see yaml/libyaml#136) so anyone that might need the new name can at least get it back if they want.

pelson · 2016-04-21T11:31:40Z

Looks like this is pretty close. 👍

msarahan · 2016-04-28T15:17:40Z

Please check out jakirkham#8 - I think that will make this one ready to go.

build static and dll on win. Cleanup.

conda-forge-linter · 2016-04-28T15:48:17Z

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 (recipes/yaml) and found some lint.

Here's what I've got...

For recipes/yaml:

Failed to even lint the recipe (might be a conda-smithy bug) 😢

jakirkham · 2016-04-28T15:48:38Z

Thanks for doing that @msarahan. Sorry I haven't had lots of time to play with this of late.

jakirkham · 2016-04-28T15:49:22Z

Could you please submit this new patch upstream if you haven't already, @msarahan, and cross reference it here? Also, please cc me on the PR.

jakirkham · 2016-05-01T06:56:37Z

recipes/yaml/CMakeLists.txt.patch

+-
+add_library (yaml SHARED ${SRC})
+set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-DYAML_DECLARE_EXPORT -DHAVE_CONFIG_H")
+add_library (yaml_static STATIC ${SRC})


Maybe we could do yaml_s. I know FLANN does something like this.

conda-forge-linter · 2016-06-02T04:19:32Z

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 (recipes/yaml) and found it was in an excellent condition.

jakirkham · 2016-06-02T04:20:09Z

Updated the PR to contain the new patch.

jakirkham · 2016-06-02T04:21:17Z

Going to clear this one out. Thanks everyone for your help.

jakirkham mentioned this pull request Apr 19, 2016

Collections of recipes needing attention? #161

Closed

jakirkham force-pushed the add_yaml branch 2 times, most recently from 73e9f0b to 234d867 Compare April 19, 2016 03:57

jakirkham changed the title ~~Add yaml~~ WIP: Add yaml Apr 19, 2016

jakirkham force-pushed the add_yaml branch 3 times, most recently from 6d759b8 to 2923544 Compare April 19, 2016 06:39

jakirkham force-pushed the add_yaml branch 14 times, most recently from 4c2335e to e1ec38f Compare April 20, 2016 02:57

jakirkham changed the title ~~WIP: Add yaml~~ Add yaml Apr 20, 2016

jakirkham force-pushed the add_yaml branch 2 times, most recently from b9c06e7 to 20da14e Compare April 20, 2016 03:17

jakirkham reviewed Apr 20, 2016
View reviewed changes

yaml: Add recipe to build yaml.

20da14e

jakirkham changed the title ~~Add yaml~~ WIP: Add yaml Apr 20, 2016

patricksnape reviewed Apr 20, 2016
View reviewed changes

build static and dll on win. Cleanup.

9295684

msarahan mentioned this pull request Apr 28, 2016

build static and dll on win. Cleanup. jakirkham/staged-recipes#8

Merged

Merge pull request #8 from msarahan/jakirkham-add_yaml

692e0fc

build static and dll on win. Cleanup.

jakirkham reviewed May 1, 2016
View reviewed changes

jakirkham changed the title ~~WIP: Add yaml~~ Add yaml Jun 2, 2016

jakirkham merged commit 6cd1cd3 into conda-forge:master Jun 2, 2016

jakirkham deleted the add_yaml branch June 2, 2016 04:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add yaml #397

Add yaml #397

jakirkham commented Apr 19, 2016 •

edited

Loading

conda-forge-admin commented Apr 19, 2016

jakirkham commented Apr 19, 2016

jakirkham commented Apr 19, 2016 •

edited

Loading

jakirkham commented Apr 19, 2016

jakirkham Apr 20, 2016

patricksnape Apr 20, 2016

msarahan Apr 20, 2016

msarahan Apr 20, 2016

jakirkham Apr 20, 2016

msarahan Apr 20, 2016

jakirkham Apr 22, 2016

msarahan Apr 22, 2016

gillins Apr 22, 2016

jakirkham Apr 28, 2016

nitzmahone Mar 4, 2019

pelson commented Apr 21, 2016

msarahan commented Apr 28, 2016

conda-forge-linter commented Apr 28, 2016

jakirkham commented Apr 28, 2016

jakirkham commented Apr 28, 2016

jakirkham May 1, 2016

conda-forge-linter commented Jun 2, 2016

jakirkham commented Jun 2, 2016

jakirkham commented Jun 2, 2016

Add yaml #397

Add yaml #397

Conversation

jakirkham commented Apr 19, 2016 • edited Loading

conda-forge-admin commented Apr 19, 2016

jakirkham commented Apr 19, 2016

jakirkham commented Apr 19, 2016 • edited Loading

jakirkham commented Apr 19, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pelson commented Apr 21, 2016

msarahan commented Apr 28, 2016

conda-forge-linter commented Apr 28, 2016

jakirkham commented Apr 28, 2016

jakirkham commented Apr 28, 2016

Choose a reason for hiding this comment

conda-forge-linter commented Jun 2, 2016

jakirkham commented Jun 2, 2016

jakirkham commented Jun 2, 2016

jakirkham commented Apr 19, 2016 •

edited

Loading

jakirkham commented Apr 19, 2016 •

edited

Loading