search

cucumber / cucumber-cpp

Support for writing Cucumber step definitions in C++
MIT License
304 stars 131 forks source link

Please add pkg-config support for use with GNU AutoTools (was #237 not merged by mistake?) #297

Open loren-osborn opened 3 months ago

loren-osborn commented 3 months ago

🤔 What's the problem you're trying to solve?

GNU AutoTools refuses to identify cucumber-cpp as installed because it doesn't install .pc files. There was a PR for this (#237) in 2019 that appeared to have been approved for inclusion, but never got merged into main. Was this PR discarded in error? /

✨ What's your proposed solution?

Either

⛏ Have you considered any alternatives or workarounds?

I could write my own upstream *.pc files, but this seems like an inferior solution to including them upstream.

đź“š Any additional context?

I'm really looking for an explanation of why #237 was closed, and if it was closed in error.

mpkorstanje commented 3 months ago

Was this PR discarded in error?

Looking at the timeline, it seems this PR was closed when the default branch was renamed from master to main. So an unintended side effects I assume.

While I'm not the maintainer of this project, I imagine it would speed things along if you could recreate the pull request.

mpkorstanje commented 3 months ago

Also closed in error

And I'm surprised it's only one other.

loren-osborn commented 3 months ago

I'm not sure If I understand cucumber-cpp quite well enough to do that yet. The PR does appear to be missing a direct dependency on pthreads that I'm testing locally at the moment.

loren-osborn commented 3 months ago

Also closed in error

And I'm surprised it's only one other.

Good catch

mpkorstanje commented 3 months ago

I'm not sure If I understand cucumber-cpp quite well enough to do that yet.

Ah that's unfortunate.

I'm afraid I know just enough CPP and CMake to be dangerous. So I can't really help. But I hope other people than me can comment on the specifics.

loren-osborn commented 3 months ago

As per the README.md, I tried to move the discussion here: https://community.smartbear.com/discussions/cucumberos/cpp-pkg-configgnu-autotools-support/264750

mpkorstanje commented 3 months ago

I think those instructions are out of date. I can't imagine anyone reading the SmartBear forums. And for context https://mattwynne.net/new-beginning.

ursfassler commented 3 months ago

As mentioned, this seems to be an error. I will look into it soon.

ursfassler commented 3 months ago

In the PR #237 the cucumber-cpp version is hard coded in the file src/cmake/cucumber-cpp-nomain.pc.in and src/cmake/cucumber-cpp.pc.in. I'd like to get this version from git and only from git to reduce a source of errors. Unfortunately we then run into the problem of not doing it correctly when it is build from the source archive. We have to solve this anyway for #295.

ursfassler commented 3 months ago

@loren-osborn do you mean that pthread is missing within the src/cmake/cucumber-cpp.pc.in file?

ursfassler commented 3 months ago

@loren-osborn The link to the discussion group is fixed with #299

loren-osborn commented 3 months ago

@loren-osborn do you mean that pthread is missing within the src/cmake/cucumber-cpp.pc.in file?

I saw the CMake macro for linking in a threading library, and didn’t see a cooresponding package “Requires:” specification… that isn’t to say that such a line is necessary… I don’t know… that’s why I didn’t volunteer to do this myself.

loren-osborn commented 3 months ago

I'd like to get this version from git and only from git to reduce a source of errors

I think industry best practice for this is to actually have one single copy of the version number somewhere in the source so that the project, including its version number, can remain buildable from a tarball. I fully agree, if git is detected, that the build system should verify that the version tag sanely corresponds to the indicated version, but should robustly handle pre-tagged scenarios and interim builds. (I’ve worked on mechanisms like this before if you need help.)

(UPDATED: clarified wording)