action #89077
closed
os-autoinst Makefile is missing symlinks configuration
Added by ybonatakis almost 4 years ago.
Updated almost 4 years ago.
Category:
Regressions/Crashes
Description
i tried to use a forked os-autoinst on my local OpenQA instance following the steps from the documentation[0].
From the root of the forked repository i run make
which finished without a problem and i started a worker with
sudo /usr/bin/perl /usr/share/openqa/script/worker --isotovideo /home/iob/os-autoinst-distri-opensuse/os-autoinst/isotovideo --instance 1
Although the job was complaining about
Please build the tinycv bindings first (see os-autoinst's README)
.[1]
With the help of @okurz we found that running make symlinks
fixes the issue.
Looking around i believe that the link that was missing was
lrwxrwxrwx 1 iob users 68 Feb 24 16:06 videoencoder -> /home/iob/os-autoinst-distri-opensuse/os-autoinst/build/videoencoder
Expected:
jobs which use that instance should run without a problem
Actual:
Jobs fails to run asking to build tinycv which it should have done so from the make
[0] https://github.com/os-autoinst/os-autoinst#build-instructions
[1] http://aquarius.suse.cz/tests/5013
- Tracker changed from communication to action
- Project changed from openSUSE admin to openQA Project (public)
- Assignee set to okurz
[moving to openQA - Oliver, please reassign as needed]
- Subject changed from [os-autoinst] Makefile is missing symlinks configuration to os-autoinst Makefile is missing symlinks configuration
- Category set to Regressions/Crashes
- Assignee deleted (
okurz)
- Target version set to Ready
- Private changed from Yes to No
I implemented it this way intentionally. I don't think the default target should mess with the source directory (e.g. by creating symlinks to the build directory somewhere in the source directory). It would be annoying if you're doing a special build of os-autoinst and it would by default change the symlinks within the source directory to that build. It would also be annoying if the source directory isn't writable. Note that the symlinks are also not required if you properly install os-autoinst using the install target.
The documentation also already contains an example with make symlinks
.
Well, but you certainly understand that people also assume that the default of make
does "all that is needed" to run it by default. So we can consider changing that or we could try to make it work without symlinks, e.g. add additional lookup paths within our files?
"all that is needed" depends from use-case to use-case. One can not expect that we can cover all use-cases in one target.
e.g. add additional lookup paths within our files?
The build directory could be anywhere so that's not that easy.
In a normal C++ project (without Perl) I'd simply launch only built files found within the build directory which of course can contain references to the source directory. Here it is quite annoying that we need to launch scripts from the source directory which need files from the build directory. In that sense it would be natural to "build" isotovideo by simply copying it over to the build directory adjusting search paths. However, I don't think that would be much appreciated as it complicates the overall development workflow.
Maybe we should just move the mentioning of the symlinks
target a bit more to the top of the README so also people who only like to read the first few paragraphs are covered.
Maybe we should just move the mentioning of the symlinks target a bit more to the top of the README so also people who only like to read the first few paragraphs are covered.
this would be also helpful imo. it wasnt clear to me if that step was mandatory or not in first sight.
I've noticed that the symlinks
target is only not mentioned at the beginning because the first steps show how to use the convenience Makefile. We could actually make symlinks
the default target within the convenience Makefile because that is only focusing on this use-case anyways (and shouldn't be used by packages anyways).
- Status changed from New to In Progress
- Status changed from In Progress to Resolved
PR merged. The fix is available in git for developers
Also available in: Atom
PDF