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 librasterlite/1.1g #6815

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Aug 11, 2021

Specify library name and version: lib/1.0

depends on #6799


  • 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

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm SpaceIm closed this Aug 17, 2021
@SpaceIm SpaceIm reopened this Aug 17, 2021
@conan-center-bot
Copy link
Collaborator

All green in build 3 (2b4d9f88c33cce34ec3056c5c9f24b5eb23d445e):

  • librasterlite/1.1g@:
    All packages built successfully! (All logs)

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 20, 2021

@SSE4 @madebr @ericLemanissier @uilianries @prince-chrismc Call for review (I don't why some PRs are not displayed in prince-chrismc/conan-center-index-pending-review#1).

@ericLemanissier
Copy link
Contributor

side topic: It's not listed because it has no reviews and the last commit is older than 24 hours

@prince-chrismc
Copy link
Contributor

We have been slacking. We were ~100 open PRs it's been creeping up all summer 🙊

@@ -1,6 +1,6 @@
ACLOCAL_AMFLAGS = -I m4

-SUBDIRS = headers lib src test
Copy link
Contributor

Choose a reason for hiding this comment

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

What's in the src dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

utilities

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 20, 2021

I see, criteria of prince-chrismc/conan-center-index-pending-review#1 shouldn't exclude PR based on last commit date.

@prince-chrismc
Copy link
Contributor

It made sense 9 months ago. But I agree ot needs to be changed

- C:\OSGeo4W\lib\libpng13.lib C:\OSGeo4W\lib\zlib.lib \
- C:\OSGeo4W\lib\geotiff_i.lib C:\OSGeo4W\lib\spatialite_i.lib
+ link /dll /out:$(RASTERLITE_DLL) \
+ /implib:rasterlite_i.lib $(LIBOBJ) $(SYSTEM_LIBS)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the recipe, you set SYSTEM_LIBS to the system libraries of the dependencies.
How are the libraries of the dependencies themselves linked? Or aren't they required?

Are the libraries of the dependencies included in self.deps_cpp_info.system_libs?

Copy link
Contributor Author

@SpaceIm SpaceIm Aug 20, 2021

Choose a reason for hiding this comment

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

It's automatically handled by VisualStudioBuildEnvironment:
https://docs.conan.io/en/latest/reference/build_helpers/visual_studio.html#visualstudiobuildenvironment
https://github.com/conan-io/conan/blob/95f2ed4fd7a6b3940b3760ec76e429ed247ea8a6/conans/client/build/visual_environment.py#L79-L89

But it doesn't propagate system libs, so it must be managed manually (I use more or less the same trick in gdal recipe).

Copy link
Contributor

@madebr madebr Aug 20, 2021

Choose a reason for hiding this comment

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

Are you sure?
The documentation says it sets the CL and LIB environment variable.
So does it add the the libraries of the dependencies to the CL variable?

Shouldn't it be considered a bug that system libraries are not added?

I suppose it uses this behavior: https://docs.microsoft.com/en-us/cpp/build/reference/cl-environment-variables?view=msvc-160

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked the logs and it contains:

	link /dll /out:rasterlite.dll  /implib:rasterlite_i.lib lib\rasterlite.obj lib\rasterlite_gif.obj  lib\rasterlite_png.obj lib\rasterlite_jpeg.obj  lib\rasterlite_io.obj lib\rasterlite_image.obj  lib\rasterlite_tiff.obj lib\rasterlite_aux.obj  lib\rasterlite_quantize.obj lib\rasterlite_version.obj shell32.lib Ole32.lib wsock32.lib crypt32.lib ws2_32.lib advapi32.lib user32.lib
Microsoft (R) Incremental Linker Version 14.28.29333.0
Copyright (C) Microsoft Corporation.  All rights reserved.

geotiff.lib libpng16.lib spatialite.lib proj.lib freexl.lib rttopo.lib libxml2_a.lib minizip.lib tiffxx.lib tiff.lib sqlite3.lib libcurl.lib iconv.lib charset.lib geos_c.lib geos.lib bz2.lib zlib.lib libdeflatestatic.lib lzma.lib libjpeg.lib jbig.lib zstd_static.lib webpdecoder.lib webpdemux.lib webpmux.lib webp.lib libssl.lib libcrypto.lib 
   Creating library rasterlite_i.lib and object rasterlite_i.exp

So it looks like it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've opened conan-io/conan#9456

@conan-center-bot conan-center-bot merged commit 9f8a3a9 into conan-io:master Aug 20, 2021
@SpaceIm SpaceIm deleted the new/librasterlite/1.1g branch August 20, 2021 14:33
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.

None yet

6 participants