action #111605
closedMoving isotovideo version patch from RPM spec to cmake made it not work in tests
Description
okurz recently moved the sed command that patches isotovideo's version code to work when it's not part of a git repository from the RPM spec to the cmake bits:
https://github.com/os-autoinst/os-autoinst/pull/2067
however, this has the unfortunate consequence that isotovideo will not work in an RPM build %test block any more, because cmake only patches the installed copy of isotovideo, but when running the tests we use the unpatched isotovideo from the source tree.
SUSE's spec actually disables all the tests that run isotovideo, I think, but in Fedora I try to keep the list of disabled tests as small as possible and was running several tests that use isotovideo. Before this change those tests worked fine, after this change they don't.
I've put the sed command back into Fedora's spec (now in the %test section), but okurz asked me to still file an issue, so here it is.
Updated by okurz over 2 years ago
- Category set to Regressions/Crashes
- Priority changed from Low to Normal
- Target version set to Ready
Updated by mkittler over 2 years ago
- Status changed from New to Feedback
- Assignee set to mkittler
I would have expected that amending the installed version is sufficient because tests can simply use isotovideo as-is. However, I now see that this can be problematic if git is not installed in the environment you're running tests in.
Maybe the following simple change does the trick for you? https://github.com/os-autoinst/os-autoinst/pull/2079 - Since tests shouldn't need the version anyways I suppose this should be good enough.
Updated by AdamWill over 2 years ago
mkittler wrote:
I would have expected that amending the installed version is sufficient because tests can simply use isotovideo as-is. However, I now see that this can be problematic if git is not installed in the environment you're running tests in.
Maybe the following simple change does the trick for you? https://github.com/os-autoinst/os-autoinst/pull/2079 - Since tests shouldn't need the version anyways I suppose this should be good enough.
The issue in our case isn't that git is not installed (it is), the issue is that the github source tarballs we run our package builds from are not git repositories.
I'll test that fix, thanks.
Updated by mkittler over 2 years ago
It should be fixed by the latest iteration of the PR which has already been merged.
Updated by mkittler over 2 years ago
- Status changed from Feedback to Resolved
As mentioned in https://github.com/os-autoinst/os-autoinst/pull/2079#issuecomment-1152144170= I tested all cases I can think of. So I'm resolving the issue. Feel free to reopen if something is missing.