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

Add APR #7386

Merged
merged 33 commits into from
Feb 26, 2019
Merged

Add APR #7386

merged 33 commits into from
Feb 26, 2019

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Dec 24, 2018

Fixes #1662

Adds packages for APR, APR-util, and APR-iconv. Uses a split package idea that Ray and I discussed sometime ago in PR ( #2815 ), which it appears he and Nehal implemented. Going to give this a try at staged-recipes and see if this just works or needs some changes.

Including myself as a maintainer.
Adding Ray, as it looks like he was one of the authors of this recipe.
@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 (recipes/apr) and found some lint.

Here's what I've got...

For recipes/apr:

  • The recipe must have a build/number section.
  • The requirements/ sections should be defined in the following order: build, host, run; instead saw: host, build.
  • The recipe license should not include the word "License".

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

Here's what I've got...

For recipes/apr:

  • The recipe must have a build/number section.
  • The recipe license should not include the word "License".

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

Here's what I've got...

For recipes/apr:

  • The recipe must have a build/number section.

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

Appears the older APR 1.6.3 version is not available from this mirror
any more. So go ahead and bump the APR version to 1.6.5 (latest), which
seems to work.
@jakirkham jakirkham force-pushed the add_apr_3 branch 2 times, most recently from a9abd5d to d72f1f5 Compare December 24, 2018 06:09
Borrows a nasty hack that was discovered a long time ago to ensure an
older `msbuild` is not picked up by accident. Forces the newer `msbuild`
from .NET to be picked up and used instead. This is needed for building
on Visual Studio 2008 where the activation script doesn't get this right
for some reason.
Try disabling usage of `libiconv` to avoid linking to the system copy
from macOS and instead only link against `libapriconv`, which we built
from the APR stack.
This reverts commit eb4adf2.
Should avoid linking to the system copy sqlite3 on macOS and should
ensure that the sqlite3 dependency does not need to be added (nor
additional libraries for it built).
@jakirkham
Copy link
Member Author

jakirkham commented Jan 3, 2019

Thanks for pointing that out. The error appears to be the same.

$ python .ci_support/build_all.py recipes/
Building apr with conda-forge/label/main
Setting build arch. This is only useful when pretending to be on another arch, such as for rendering necessary dependencies on a non-native arch. I trust that you know what you're doing.
rendering /Users/kirkhamj/Developer/Conda/conda-forge/staged-recipes/recipes/apr for osx-64
Setting build platform. This is only useful when pretending to be on another platform, such as for rendering necessary dependencies on a non-native platform. I trust that you know what you're doing.
Dependency libapr, version any is not installable from your channels: ['local', 'conda-forge', 'defaults', 'conda-forge', 'defaults'] with subdir osx-64.  Seeing if we can build it...
Dependency libapriconv, version any is not installable from your channels: ['local', 'conda-forge', 'defaults', 'conda-forge', 'defaults'] with subdir osx-64.  Seeing if we can build it...
Dependency libaprutil, version any is not installable from your channels: ['local', 'conda-forge', 'defaults', 'conda-forge', 'defaults'] with subdir osx-64.  Seeing if we can build it...
Traceback (most recent call last):
  File ".ci_support/build_all.py", line 134, in <module>
    build_all(args.recipes_dir, args.arch)
  File ".ci_support/build_all.py", line 68, in build_all
    build_folders(recipes_dir, old_comp_folders, arch, channel_urls)
  File ".ci_support/build_all.py", line 109, in build_folders
    order = list(nx.topological_sort(G))
  File "/zopt/conda3/lib/python3.6/site-packages/networkx/algorithms/dag.py", line 208, in topological_sort
    raise nx.NetworkXUnfeasible("Graph contains a cycle or graph changed "
networkx.exception.NetworkXUnfeasible: Graph contains a cycle or graph changed during iteration

@chrisburr
Copy link
Member

This isn't the same issue as #7426. It seems conda-forge can't handle building a top level recipe at the same time as the outputs. I think you can use the workaround from clangdev 5.0.0 and apply this patch:

diff --git a/recipes/apr/meta.yaml b/recipes/apr/meta.yaml
index f0bec3839..cd2aec98a 100644
--- a/recipes/apr/meta.yaml
+++ b/recipes/apr/meta.yaml
@@ -4,7 +4,7 @@
 {% set mirror = "http://www-us.apache.org/dist/" %}

 package:
-  name: apr
+  name: apr_combined
   version: {{ apr_ver }}

 source:
@@ -30,13 +30,20 @@ source:
 build:
   number: 0

+# Specifying `compiler('c')` as a top-level build requirements to force
+# conda-smithy to generate the correct build matrix.
 requirements:
-  run:
-    - libapr
-    - libaprutil
-    - libapriconv
+  build:
+    - {{ compiler('c') }}

 outputs:
+  - name: apr
+    version: {{ apr_ver }}
+    requirements:
+      run:
+        - libapr
+        - libaprutil
+        - libapriconv
   - name: libapr
     version: {{ apr_ver }}
     build:

(It's important that the top level recipe isn't called apr to avoid conda/conda-build#3337.)

@jakirkham
Copy link
Member Author

Was afraid someone might say this. 😄 Thanks for the info.

@jakirkham
Copy link
Member Author

jakirkham commented Feb 24, 2019

Have tried merging with master and it appears the macOS and Linux builds now pass. 🎉

Windows is failing rather early on due to some sort of issue determining what to build.

Traceback (most recent call last):
  File ".ci_support\build_all.py", line 130, in <module>
    build_all(args.recipes_dir, args.arch)
  File ".ci_support\build_all.py", line 68, in build_all
    build_folders(recipes_dir, new_comp_folders, arch, channel_urls)
  File ".ci_support\build_all.py", line 104, in build_folders
    config=config, finalize=False)
  File "C:\projects\staged-recipes\.ci_support\compute_build_graph.py", line 403, in construct_graph
    recipes_dir, config=config, finalize=finalize)
  File "C:\projects\staged-recipes\.ci_support\compute_build_graph.py", line 219, in add_recipe_to_graph
    rendered = _get_or_render_metadata(recipe_dir, worker, config=config, finalize=finalize)
  File "C:\Miniconda36-x64\lib\site-packages\conda\exports.py", line 182, in __call__
    return self.func(*args, **kw)
  File "C:\projects\staged-recipes\.ci_support\compute_build_graph.py", line 212, in _get_or_render_metadata
    bypass_env_check=True, config=config, finalize=finalize)
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\api.py", line 51, in render
    bypass_env_check=bypass_env_check):
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\metadata.py", line 1994, in get_output_metadata_set
    for k in out_metadata.get_used_vars()}))] = out, out_metadata
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\metadata.py", line 2122, in get_used_vars
    force_global=force_global)
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\metadata.py", line 2175, in _get_used_vars_meta_yaml
    force_top_level=force_top_level, force_global=force_global, apply_selectors=False)
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\metadata.py", line 2162, in _get_used_vars_meta_yaml_helper
    apply_selectors=apply_selectors))
  File "C:\Miniconda36-x64\lib\site-packages\conda_build\metadata.py", line 1693, in extract_single_output_text
    output = output_matches[output_index] if output_matches else ''
IndexError: list index out of range

ref: https://ci.appveyor.com/project/conda-forge/staged-recipes/builds/22603852#L185

Edit: Managed to reproduce with conda-build alone. Raised upstream as issue ( conda/conda-build#3411 ).

@scopatz
Copy link
Member

scopatz commented Feb 25, 2019

Glad that this is a conda-build issue.

It seems that for Windows it is very important for this metapackage to
have the `compiler` for rendering purposes. So go ahead and add it.
@jakirkham
Copy link
Member Author

Nope, turns out the error is mine. Though minimizing the issue was productive as that bug is now fixed.

@jakirkham
Copy link
Member Author

Hmm...it appears I'm getting a build failure on Windows. It seems to not be able to find expat. 😕 @mingwandroid @nehaljwani, any ideas?

"%SRC_DIR%\apr-util\libaprutil.vcxproj" (default target) (2) ->
(ClCompile target) -> 
  xml\apr_xml.c(35): fatal error C1083: Cannot open include file: 'expat.h': No such file or directory [%SRC_DIR%\apr-util\libaprutil.vcxproj]

ref: https://ci.appveyor.com/project/conda-forge/staged-recipes/builds/22633909

The dependency on `expat` will be added to split packages as
appropriate. So there is no need for the metapackage to pull it in too
or for it to be pinned to it.
@jakirkham jakirkham force-pushed the add_apr_3 branch 2 times, most recently from d2f4e85 to 225eecc Compare February 26, 2019 04:57
@jakirkham
Copy link
Member Author

Alright this built through the VS 2015 build and ran the tests. Also macOS and Linux haven't been changed since they last passed. However it appears to be hung up on some other bug that is specific to staged-recipes. Have checked locally that a normal feedstock will be created through re-rendering. Am going to go ahead and merge and deal with any other issues that arise in the feedstock. Thanks all for putting up with my noise and helping me slog through this one.

@jakirkham jakirkham changed the title WIP: Add APR Add APR Feb 26, 2019
@jakirkham jakirkham merged commit 6cdd334 into conda-forge:master Feb 26, 2019
@jakirkham jakirkham deleted the add_apr_3 branch February 26, 2019 05:46
@jakirkham
Copy link
Member Author

FWIW in the feedstock Windows VS 2008 and VS 2015 passed. The rest of the packages should follow soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants