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 proj/6.3.0 #751

Closed
wants to merge 1 commit into from
Closed

Add proj/6.3.0 #751

wants to merge 1 commit into from

Conversation

btashton
Copy link
Contributor

Specify library name and version: proj/6.3.0

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 2 (b6f9a0cea1630bdd9fbc3e9ab5b55c841a8ec983):

@conan-center-bot
Copy link
Collaborator

All green in build 1 (96fb8a5c628f4bb253876996d8b4b513350cd07a)! 😊

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 3 (d2ee7a494947cb2d33ad53256cefb395a495d6a4):

@btashton
Copy link
Contributor Author

We don't build the sqlite3 binary as part of the sqlite3 package in CCI, so instead I added a python script to build the database which I think will make this build. This seems really hacky, but I'm not sure what a better solution would be.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 4 (1abdd974d5f76f6f011bc9e86361736cec97df60):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 5 (ca730f751c817a2abee662f2b758c0c671b19cfe):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 6 (0a6742269d7b9443a70975ce308ca26c5d501dd9):

  • Windows x86_64, Debug, Visual Studio 15, MDd. Options: proj:shared-True
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [SHARED ARTIFACTS (KB-H015)] Package with 'shared' option did not contains any shared artifact (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H015)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs
  • Windows x86_64, Release, Visual Studio 15, MD. Options: proj:shared-True
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [SHARED ARTIFACTS (KB-H015)] Package with 'shared' option did not contains any shared artifact (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H015)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 7 (24607b67061994e6ee6c4fea3a86781d5f0ac24b):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 8 (1d3ec5a593a4a8a11f90b3e2804fa48c99c4197c):

Signed-off-by: Brennan Ashton <bashton@brennanashton.com>
@conan-center-bot
Copy link
Collaborator

Some configurations of 'proj/6.3.0' failed in build 9 (2a255c5e055f536356edecfc3f51a2a2f78e9664):

@conan-center-bot
Copy link
Collaborator

All green in build 10 (de0f7321b4c7582a2cf8b5e2aac26cfb277bc6cf)! 😊

import os


class LibaecConan(ConanFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

ProjConan instead of LibaecConan ;)

@conan-center-bot
Copy link
Collaborator

All green in build 11 (de0f7321b4c7582a2cf8b5e2aac26cfb277bc6cf)! 😊

@uilianries
Copy link
Member

libtiff and curl are optional in this project and can be added as optional dependencies: https://github.com/OSGeo/PROJ/blob/master/CMakeLists.txt#L147

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 5, 2020

@uilianries Not in 6.x.x.
7.0.0 was released 4 days ago.

@uilianries
Copy link
Member

My mistake! I didn't check that version!

@uilianries
Copy link
Member

I'll wait for a new PR for 7.0 😉

@SpaceIm SpaceIm mentioned this pull request Mar 7, 2020
@@ -0,0 +1,7 @@
import sqlite3
Copy link
Member

Choose a reason for hiding this comment

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

I think this is quite a hack as well, but I see we don't have the executable in the sqlite3 packages so it is the only way to go for now.

On the other hand, this might fail for Conan users that are using the installers (no Python installed).

My question is the following: If the sqlite3 library is expected to have the executable, why don't we package it? Any ideas @madebr @SSE4 @uilianries ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@danimtb I've submitted a PR to add sqlite3 CLI: #1004

Copy link
Member

Choose a reason for hiding this comment

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

Please @btashton take a look at the sqlite3 approach and try to use the sqlite3 executable from the Conan package now that we got #1004 merged. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at #1004, sqlite3 will fail when using as a build requirement and cross building (e.g. building an arm application on x86).

@btashton
Copy link
Contributor Author

This is only a build time dependency on the executable since it is creating a database that it uses at runtime. In the past it looked like the sqlite utility was packaged up on its own to avoid this problem.

I also think I'm not storing the database in the standard place for the Conan package so I should also address that as well.

@madebr
Copy link
Contributor

madebr commented Mar 12, 2020

This is only a build time dependency on the executable since it is creating a database that it uses at runtime. In the past it looked like the sqlite utility was packaged up on its own to avoid this problem.

Then you should add it to the build_requirements method instead of requirements.

re: #751 (comment)
@danimtb @uilianries
What would you suggest? Until conan supports crossbuilding, using sqlite as a build requirement will fail.

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 12, 2020

@madebr AFAIK, sqlite3 is both a requirements (proj depends on sqlite3) and a build_requirements (for the executable, in order to create proj.db).

@btashton
Copy link
Contributor Author

@madebr AFAIK, sqlite3 is both a requirements (proj depends on sqlite3) and a build_requirements (for the executable, in order to create proj.db).

Exactly. This is also why I think my hack might be ok for now if we don't have a good solution. If they are building with Conan they have the python with sqlite and we don't have to deal with the cross build issue.

@danimtb
Copy link
Member

danimtb commented Mar 23, 2020

Well, reading this again I understand that the cross-building case is not well covered with now with Conan. However, now that we have the sqlite3 executable packaged by the recipe, we should use it in this recipe to avoid the python hack. Some people might use Conan from the stand-alone installers and could not have python available.

Morever, I think that doing this hack is worse for the future fix of the cross-building case than having a clean recipe without misleading python hacks.

@danimtb danimtb assigned danimtb and unassigned jgsogo Mar 23, 2020
Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

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

Please address the changes regarding the squlite3 executable usage

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 25, 2020

@btashton I'm close to provide a working PROJ recipe using sqlite3 executable (without python). I'll submit a PR to your branch.

I've also made several modifications:

  • add threadsafe option
  • dl removed from system libs (need to check on Linux, but I don't think dl is required)
  • PROJ_MSVC_DLL_IMPORT defined for consumer if shared and Visual Studio compiler
  • add path to executables in PATH env (maybe I can add options to disable building these exe, since PROJ provides option for them).
  • bump version of sqlite3
  • fix class name (ProjConan isntead of LibaecConan)
  • move proj.db in res folder (and update PROJ_LIB accordingly)

(I'm waiting for this recipe for libgeotiff and maybe for a first attempt to package gdal)

@btashton
Copy link
Contributor Author

Great I'll set aside some time to take a look. Thanks for helping push it over the finish line I have had a bunch of other things on my plate.

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 26, 2020

Done: btashton#1 ;)

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 26, 2020

For the sake of memory:

  • PROJ < 7 exported target is PROJ4:proj
  • PROJ >=7 exported target is PROJ:proj (PROJ4:proj deprecated)

@SpaceIm
Copy link
Contributor

SpaceIm commented Apr 2, 2020

@btashton @danimtb Would you mind if I submit a new PR for proj/6.3.0 based on https://github.com/SpaceIm/conan-center-index/tree/proj/fix-and-improvements (it includes btashton commits), so we can move forward easily?

@danimtb
Copy link
Member

danimtb commented Apr 3, 2020

I'll let @btashton answer this. For me it ok whatever you both agree 😄

@btashton
Copy link
Contributor Author

btashton commented Apr 3, 2020

@SpaceIm If you want to take this across the finish line that would be great. Sorry I lost track of this in all my github notifications. If you ping me in your PR I would be happy to try and help with the review.

@SpaceIm SpaceIm mentioned this pull request Apr 4, 2020
4 tasks
@btashton btashton closed this Apr 6, 2020
@btashton
Copy link
Contributor Author

btashton commented Apr 6, 2020

Closing in favor of #1272 Thanks @SpaceIm for finishing this up.

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.

8 participants