Added Gromac's Tng lib #2218

GavinNL · 2020-07-15T20:09:22Z

Specify library name and version: tng/1.8.2

Gromac's external library for loading binary molecular dynamics trajectory files.

Tested on Linux Mint 20 with gcc9

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.

- using an example provided by tng as the test_package - added a tng file

conan-center-bot · 2020-07-15T20:17:50Z

Some configurations of 'tng/1.8.2' failed in build 1 (47fef6c6917f7c45da52cd350b3c51dbf5ecf90a):

Linux x86_64, Release, gcc 5, libstdc++ . Options: tng:shared-False
Access to all the logs
Macos x86_64, Debug, apple-clang 9.1, libc++ . Options: tng:shared-True
Access to all the logs
Windows x86_64, Debug, Visual Studio 14, MTd. Options: tng:shared-False
Access to all the logs

recipes/tng/all/test_package/conanfile.py

recipes/tng/all/CMakeLists.txt

recipes/tng/all/conanfile.py

madebr · 2020-07-15T20:29:31Z

recipes/tng/all/conanfile.py

+
+        self._cmake.definitions["TNG_BUILD_OWN_ZLIB"] = False
+
+        self._cmake.configure(build_folder=self._build_subfolder)


I think this problem is caused by these lines in BuildTNG.cmake:

CMAKE_SOURCE_DIR and CMAKE_BUILD_DIR should be PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR.

I figured it would be an issue with the project's cmake file. Not sure what I should do about it, should we apply a patch? My preference is to not edit the original if I can avoid it.

The change would be equivalent to your move of sources.
The advantage of patching is that, when upstream fixes this problem, the patch can simply be dropped without modifying the recipe.

I just tested it locally.
This patch avoids any special actions in the _configure_cmake function:

--- BuildTNG.cmake +++ BuildTNG.cmake @@ -1,9 +1,9 @@ set(TNG_ROOT_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}) -file(RELATIVE_PATH TNG_ROOT_BINARY_DIR ${CMAKE_SOURCE_DIR} ${TNG_ROOT_SOURCE_DIR}) +file(RELATIVE_PATH TNG_ROOT_BINARY_DIR ${PROJECT_SOURCE_DIR} ${TNG_ROOT_SOURCE_DIR}) if ("${TNG_ROOT_BINARY_DIR}" MATCHES "^\.\.") set(TNG_ROOT_BINARY_DIR tng) endif() -set(TNG_ROOT_BINARY_DIR ${CMAKE_BINARY_DIR}/${TNG_ROOT_BINARY_DIR}) +set(TNG_ROOT_BINARY_DIR ${PROJECT_BINARY_DIR}/${TNG_ROOT_BINARY_DIR}) set(TNG_MAJOR_VERSION "1") set(TNG_MINOR_VERSION "8")

I think upstream would be happy to apply this.
Because right now, it could not be included as a subproject.

Okay, I added it as a patch.

I've opened an issue with the patch at gromacs/tng#1

I'm not sure if anyone is even bothering to look at the issues in the original project. I don't think that library has changed in many years

It's up to them to accept or not...

recipes/tng/all/conanfile.py

recipes/tng/all/test_package/CMakeLists.txt

Co-authored-by: Anonymous Maarten <[email protected]>

conan-center-bot · 2020-07-15T20:41:06Z

Some configurations of 'tng/1.8.2' failed in build 3 (c6b1badd0a2a51eecbed94943a052ad7dde54038):

Linux x86_64, Release, gcc 5, libstdc++ . Options: tng:shared-False
Access to all the logs
Windows x86_64, Debug, Visual Studio 14, MTd. Options: tng:shared-False
Access to all the logs

recipes/tng/all/conanfile.py

Co-authored-by: Anonymous Maarten <[email protected]>

conan-center-bot · 2020-07-15T21:01:51Z

Some configurations of 'tng/1.8.2' failed in build 4 (dd9ee64c149a5f42e4dac678f96da69d1c717922):

Windows x86_64, Debug, Visual Studio 14, MTd. Options: tng:shared-False
Access to all the logs

conan-center-bot · 2020-07-15T21:53:45Z

Some configurations of 'tng/1.8.2' failed in build 5 (35cd4972b886ee54fb2495a9ae03fab2b0b39aca):

Windows x86_64, Debug, Visual Studio 14, MTd. Options: tng:shared-False
Access to all the logs

recipes/tng/all/conanfile.py

Co-authored-by: Uilian Ries <[email protected]>

conan-center-bot · 2020-11-27T14:26:22Z

Failure in build 6 (62ccddbf61e83e14c43b2868cabab962be0d24ae):

tng/1.8.2
- Hooks errors detected:
  - [HOOK - conan-center.py] pre_export(): ERROR: [CMAKE VERSION REQUIRED (KB-H048)] The test_package/CMakeLists.txt requires CMake 3.1 at least. Update to 'cmake_minimum_required(VERSION 3.1)'. (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H048)

uilianries

LGTM

conan-center-bot · 2020-11-27T15:03:10Z

Some configurations of 'tng/1.8.2' failed in build 7 (6f92f238326a88ef2fb035e002c6b435b5d3a296):

Windows x86_64, Release, Visual Studio 15, MT. Options: tng: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/blob/master/docs/error_knowledge_base.md#KB-H015)
- Log download
- Log error Download
- Profile
Access to all the logs
Windows x86_64, Release, Visual Studio 15, MD. Options: tng: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/blob/master/docs/error_knowledge_base.md#KB-H015)
- Log download
- Log error Download
- Profile
Access to all the logs
Windows x86_64, Release, Visual Studio 16, MD. Options: tng: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/blob/master/docs/error_knowledge_base.md#KB-H015)
- Log download
- Log error Download
- Profile
Access to all the logs

stale · 2020-12-27T16:01:23Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

GavinNL · 2020-12-27T20:01:02Z

Would anyone be able to assist in fixing this? I don't have a windows box to properly debug the issue.

madebr · 2020-12-27T20:10:10Z

@GavinNL
https://github.com/gromacs/tng/blob/14dd68ff46df1ef1cf2d9bcc1f4f5bddb974ce1c/CMakeLists.txt#L47-L50
This needs to become:

install(TARGETS tng_io
        EXPORT tng_io
        LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
        ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
        RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")

conan-center-bot · 2020-12-27T21:12:44Z

All green in build 8 (6389563b326e573aa669360438b61bd9d4064eb1)! 😊

tng/1.8.2: Generated 88 packages. All logs here

prince-chrismc · 2020-12-28T16:38:22Z

recipes/tng/all/conandata.yml

@@ -0,0 +1,10 @@
+sources:
+  "1.8.2":
+    url: "https://github.com/gromacs/tng/archive/v1.8.2.tar.gz"


It seems they moved to gitlab sometime ago.... https://gitlab.com/gromacs/tng/-/tags

is there a reason you are referencing the github page?

Hmm. I did not know this. I'll change this.

prince-chrismc · 2020-12-28T16:39:59Z

recipes/tng/all/test_package/example.c

+#include <stdlib.h>
+#include <stdio.h>
+
+int main(int argc, char **argv)


we like to have much move minimal test_packages... I idea it to make sure the conan packing went well no so much the library is functional.

could you please try to reduce this to a few lines? example instantiate a few objects?

I used an example piece of code from the library to create the test_package.c, figured it would be a good way to test it.

I will reduce it.

conan-center-bot · 2020-12-28T17:27:24Z

All green in build 9 (ac98fe80c996ec3a52e12408c7ecf62799606860)! 😊

tng/1.8.2: Generated 88 packages. All logs here

madebr · 2020-12-28T17:48:03Z

recipes/tng/all/conanfile.py

+
+    def package_info(self):
+        self.cpp_info.libs = ["tng_io"]
+        if self.settings.os == "Linux":


I didn't test, but some zealots (aka @ericLemanissier 🤣) have recently begun with adding FreeBSD support.

Suggested change

if self.settings.os == "Linux":

if self.settings.os in ("Linux", "FreeBSD"):

lol...is that "LInux" a typo? (capital I)

Yes, typo! Fixing it now..

If you want to get a free FreeBSD CI test, just put "FreeBSD" in your PR's description !
(It sounds like a joke, but it's actually not ;))

@prince-chrismc @ericLemanissier
We really need a (pinned) issue with a list of all active bots + their features.

prince-chrismc · 2020-12-28T18:00:09Z

recipes/tng/all/conanfile.py

+    description = "External GROMACS library for loading tng files."
+    license = "BSD-3-Clause"
+    topics = ("conan", "tng", "gromacs")
+    homepage = "https://github.com/gromacs/tng/"


you'll need to point this ti gitlab as well =)

conan-center-bot · 2020-12-28T18:24:54Z

All green in build 11 (7c8918a0785eb87ea495e04e140aefc752ff2d50)! 😊

tng/1.8.2: Generated 88 packages. All logs here

GavinNL · 2020-12-28T21:26:51Z

@uilianries Could we get your approval again so this can be merged?

prince-chrismc · 2020-12-30T20:34:16Z

Seems like the bot is skipping over this PR, can someone take a peak at why that might be?

GavinNL added 3 commits July 15, 2020 15:31

Added Gromac's TNG library

4a82930

Fixed conan-center package requirements

adf520a

Updated the test_package example

47fef6c

- using an example provided by tng as the test_package - added a tng file

conan-center-bot added the Failed label Jul 15, 2020

madebr reviewed Jul 15, 2020

View reviewed changes

GavinNL and others added 2 commits July 15, 2020 16:32

Apply suggestions from code review

f63aeae

Co-authored-by: Anonymous Maarten <[email protected]>

Apply suggestions from code review

c6b1bad

Co-authored-by: Anonymous Maarten <[email protected]>

madebr reviewed Jul 15, 2020

View reviewed changes

recipes/tng/all/conanfile.py Show resolved Hide resolved

Update recipes/tng/all/conanfile.py

dd9ee64

Co-authored-by: Anonymous Maarten <[email protected]>

Added patch to fix upstream's CMakeList.txt file

35cd497

madebr mentioned this pull request Jul 16, 2020

Building tng as a subproject in a subdirectory gromacs/tng#1

Open

uilianries requested changes Nov 27, 2020

View reviewed changes

recipes/tng/all/conanfile.py Outdated Show resolved Hide resolved

recipes/tng/all/conanfile.py Show resolved Hide resolved

Apply suggestions from code review

62ccddb

Co-authored-by: Uilian Ries <[email protected]>

Updated minimum cmake version to 3.1

6f92f23

uilianries previously approved these changes Nov 27, 2020

View reviewed changes

madebr previously approved these changes Nov 27, 2020

View reviewed changes

stale bot added the stale label Dec 27, 2020

stale bot removed the stale label Dec 27, 2020

Added patch to fix windows shared builds

6389563

GavinNL dismissed stale reviews from madebr and uilianries via 6389563 December 27, 2020 20:54

conan-center-bot removed the Failed label Dec 27, 2020

madebr previously approved these changes Dec 27, 2020

View reviewed changes

github-actions bot mentioned this pull request Dec 28, 2020

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

prince-chrismc suggested changes Dec 28, 2020

View reviewed changes

GavinNL added 2 commits December 28, 2020 11:53

Sources now pulled from Gitlab instead of Github

1310937

test_package is now smaller

ac98fe8

GavinNL dismissed madebr’s stale review via ac98fe8 December 28, 2020 17:06

madebr reviewed Dec 28, 2020

View reviewed changes

Added FreeBSD

0c20699

madebr previously approved these changes Dec 28, 2020

View reviewed changes

prince-chrismc reviewed Dec 28, 2020

View reviewed changes

Fixed homepage -> gitlab

7c8918a

GavinNL dismissed madebr’s stale review via 7c8918a December 28, 2020 18:02

madebr approved these changes Dec 28, 2020

View reviewed changes

prince-chrismc approved these changes Dec 28, 2020

View reviewed changes

SSE4 approved these changes Dec 29, 2020

View reviewed changes

uilianries approved these changes Dec 30, 2020

View reviewed changes

conan-center-bot merged commit d8c53b3 into conan-io:master Dec 30, 2020


		self._cmake.definitions["TNG_BUILD_OWN_ZLIB"] = False

		self._cmake.configure(build_folder=self._build_subfolder)

	if self.settings.os == "Linux":
	if self.settings.os in ("Linux", "FreeBSD"):

Added Gromac's Tng lib #2218

Added Gromac's Tng lib #2218

Conversation

GavinNL commented Jul 15, 2020

conan-center-bot commented Jul 15, 2020

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madebr Jul 15, 2020 • edited Loading

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

conan-center-bot commented Jul 15, 2020

conan-center-bot commented Jul 15, 2020

conan-center-bot commented Jul 15, 2020

conan-center-bot commented Nov 27, 2020

uilianries left a comment

Choose a reason for hiding this comment

conan-center-bot commented Nov 27, 2020

stale bot commented Dec 27, 2020

GavinNL commented Dec 27, 2020

madebr commented Dec 27, 2020 • edited Loading

conan-center-bot commented Dec 27, 2020

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

conan-center-bot commented Dec 28, 2020

madebr Dec 28, 2020 • edited Loading

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

conan-center-bot commented Dec 28, 2020

GavinNL commented Dec 28, 2020

prince-chrismc commented Dec 30, 2020

madebr Jul 15, 2020 •

edited

Loading

madebr commented Dec 27, 2020 •

edited

Loading

madebr Dec 28, 2020 •

edited

Loading