eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.22k stars 170 forks source link

fakeit from conan center doesn't get header with integration (e.g. header with gtest integration) #306

Closed malcolmdavey closed 1 year ago

malcolmdavey commented 1 year ago

I've tried out using the fakeit from conan center Have tried both both version 2.3.0 and 2.3.2 and specifying the fakeit "integration" option as "gtest".

This correctly pulls in gtest as a conan dependency, but the header file is just the plane one without any integration. i.e. it's just using the one called "standalone".

Seems like there is only one package. Either there should be a package based on each integration option, or there should be one package with all the headers, though the former would likely be preferred.

https://conan.io/center/fakeit?tab=recipe&os=

Seems it is choosing the specific header, but guessing we only build the package once, with the default option of standalone

    def package(self):
        self.copy(pattern="fakeit.hpp", dst="include", src=os.path.join(self._source_subfolder, "single_header", str(self.options.integration)))

I only get one package file downloaded to my conan cache.

.conan\data\fakeit\2.3.2\_\_\package\5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9

I tried on another library (jasper) from conan center varied it's option for shared - and this worked to have a separate package for each option value.

malcolmdavey commented 1 year ago

I've found the issue. The problem is the package_id() clears and hence ignores the options, which is incorrect.

Currently the recipe specifies:

    def package_id(self):
        self.info.header_only()

But the header_only is designed to ignore options, which is incorrect in this case, as we have a different header filee to package based on the integrated test library e.g. gtest or boost (or standalone).

    def header_only(self):
        self.settings.clear()
        self.options.clear()
        self.requires.clear()

Also raised an issue here: https://github.com/conan-io/conan-center-index/issues/14900

FranckRJ commented 1 year ago

Initially it worked (but was probably not the best implementation) but since version 2.2 a commit broke it: https://github.com/conan-io/conan-center-index/commit/bfec4728aeb3c86eec64722508f2cc9500f2c6ae#diff-dcefd0522786a3ad8878130e31083cfe9133f64cbdd09eafc309e5119a3f33d1L52

I'll keep the issue here to remember to fix it when I can (in January) but keep in mind this repo is not related to the conan package, it's probably more suited in the cci repo.

malcolmdavey commented 1 year ago

yeah Thanks. I raised it there as well.

FranckRJ commented 1 year ago

The fix has been merged in conan center.

malcolmdavey commented 1 year ago

Thanks