Project

General

Profile

Actions

action #111605

closed

Moving isotovideo version patch from RPM spec to cmake made it not work in tests

Added by AdamWill almost 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Regressions/Crashes
Target version:
Start date:
2022-05-25
Due date:
% Done:

0%

Estimated time:

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.

Actions #1

Updated by okurz almost 2 years ago

  • Category set to Regressions/Crashes
  • Priority changed from Low to Normal
  • Target version set to Ready
Actions #2

Updated by okurz almost 2 years ago

  • Priority changed from Normal to Low
Actions #3

Updated by mkittler almost 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.

Actions #4

Updated by AdamWill almost 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.

Actions #5

Updated by mkittler almost 2 years ago

It should be fixed by the latest iteration of the PR which has already been merged.

Actions #6

Updated by mkittler almost 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.

Actions

Also available in: Atom PDF