Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added Gromac's Tng lib #2218
Added Gromac's Tng lib #2218
Changes from all commits
4a82930
adf520a
47fef6c
f63aeae
c6b1bad
dd9ee64
35cd497
62ccddb
6f92f23
6389563
1310937
ac98fe8
0c20699
7c8918a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this problem is caused by these lines in
BuildTNG.cmake
:CMAKE_SOURCE_DIR
andCMAKE_BUILD_DIR
should bePROJECT_SOURCE_DIR
andPROJECT_BINARY_DIR
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested it locally.
This patch avoids any special actions in the
_configure_cmake
function:I think upstream would be happy to apply this.
Because right now, it could not be included as a subproject.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I added it as a patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened an issue with the patch at gromacs/tng#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's up to them to accept or not...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.