[PATCH 0/3] Fixing out-of-tree builds
Building virt-v2v out-of-tree does not work and requires several small fixes here and there. Tomáš Golembiovský (3): build: perform gnulib check from source directory build: run ocaml-link.sh from build directory docs: don't perform lookup on absolute paths cfg.mk | 1 + podwrapper.pl.in | 2 ++ v2v/Makefile.am | 16 ++++++++-------- 3 files changed, 11 insertions(+), 8 deletions(-) -- 2.25.0
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- cfg.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/cfg.mk b/cfg.mk index a303ee72..4bd2d006 100644 --- a/cfg.mk +++ b/cfg.mk @@ -132,6 +132,7 @@ ifeq (0,$(MAKELEVEL)) # b653eda3ac4864de205419d9f41eec267cb89eeb _submodule_hash = sed 's/^[ +-]//;s/ .*//' _update_required := $(shell \ + cd $(top_srcdir); \ actual=$$(git submodule status | grep gnulib | $(_submodule_hash)); \ stamp="$$($(_submodule_hash) $(_curr_status) 2>/dev/null)"; \ test "$$stamp" = "$$actual"; echo $$?) -- 2.25.0
ocaml-link.sh is generated file. It means that for out-of-tree builds it is located in a build directory which is different from source tree dir. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/Makefile.am | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/v2v/Makefile.am b/v2v/Makefile.am index 64703c36..1c10d07b 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -248,9 +248,9 @@ OCAMLLINKFLAGS = \ mlv2v.$(MLARCHIVE) \ $(LINK_CUSTOM_OCAMLC_ONLY) -virt_v2v_DEPENDENCIES = $(OBJECTS) $(top_srcdir)/ocaml-link.sh +virt_v2v_DEPENDENCIES = $(OBJECTS) $(top_builddir)/ocaml-link.sh virt_v2v_LINK = \ - $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ + $(top_builddir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLPACKAGES) $(OCAMLLINKFLAGS) \ $(OBJECTS) -o $@ @@ -293,9 +293,9 @@ virt_v2v_copy_to_local_DEPENDENCIES = \ ../bundled/libvirt-ocaml/mllibvirt.$(MLARCHIVE) \ ../common/mlcustomize/mlcustomize.$(MLARCHIVE) \ ../common/mlv2v/mlv2v.$(MLARCHIVE) \ - $(top_srcdir)/ocaml-link.sh + $(top_builddir)/ocaml-link.sh virt_v2v_copy_to_local_LINK = \ - $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ + $(top_builddir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLPACKAGES) $(OCAMLLINKFLAGS) \ $(COPY_TO_LOCAL_OBJECTS) -o $@ @@ -367,9 +367,9 @@ v2v_unit_tests_DEPENDENCIES = \ ../common/mltools/mltools.$(MLARCHIVE) \ ../common/mlcustomize/mlcustomize.$(MLARCHIVE) \ ../common/mlv2v/mlv2v.$(MLARCHIVE) \ - $(top_srcdir)/ocaml-link.sh + $(top_builddir)/ocaml-link.sh v2v_unit_tests_LINK = \ - $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ + $(top_builddir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) \ $(OCAMLPACKAGES) -package oUnit \ $(OCAMLLINKFLAGS) \ @@ -378,9 +378,9 @@ v2v_unit_tests_LINK = \ var_expander_tests_DEPENDENCIES = \ $(var_expander_tests_THEOBJECTS) \ ../common/mlpcre/mlpcre.$(MLARCHIVE) \ - $(top_srcdir)/ocaml-link.sh + $(top_builddir)/ocaml-link.sh var_expander_tests_LINK = \ - $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ + $(top_builddir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) \ $(OCAMLPACKAGES) -package oUnit \ $(OCAMLLINKFLAGS) \ -- 2.25.0
On Monday, 27 January 2020 10:39:33 CET Tomáš Golembiovský wrote:
ocaml-link.sh is generated file. It means that for out-of-tree builds it is located in a build directory which is different from source tree dir.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> ---
LGTM. What about doing the same changes in libguestfs, please? :-) Thanks, -- Pino Toscano
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- podwrapper.pl.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/podwrapper.pl.in b/podwrapper.pl.in index f12a173f..1e4aa149 100755 --- a/podwrapper.pl.in +++ b/podwrapper.pl.in @@ -689,6 +689,8 @@ sub find_file my $use_path = shift; local $_; + return $input if File::Spec->file_name_is_absolute($input) and -f $input; + my @search_path = ("."); push (@search_path, @paths) if $use_path; foreach (@search_path) { -- 2.25.0
On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- podwrapper.pl.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/podwrapper.pl.in b/podwrapper.pl.in index f12a173f..1e4aa149 100755 --- a/podwrapper.pl.in +++ b/podwrapper.pl.in @@ -689,6 +689,8 @@ sub find_file my $use_path = shift; local $_;
+ return $input if File::Spec->file_name_is_absolute($input) and -f $input;
Do you really need to use file_name_is_absolute? -f seems to work fine also with absolute paths. In case the path is relative, -f will be fine too, as...
my @search_path = ("."); push (@search_path, @paths) if $use_path; foreach (@search_path) {
... the first search path is always '.'. -- Pino Toscano
On Mon, Jan 27, 2020 at 12:17:42PM +0100, Pino Toscano wrote:
On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- podwrapper.pl.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/podwrapper.pl.in b/podwrapper.pl.in index f12a173f..1e4aa149 100755 --- a/podwrapper.pl.in +++ b/podwrapper.pl.in @@ -689,6 +689,8 @@ sub find_file my $use_path = shift; local $_;
+ return $input if File::Spec->file_name_is_absolute($input) and -f $input;
Do you really need to use file_name_is_absolute? -f seems to work fine also with absolute paths. In case the path is relative, -f will be fine too, as...
It's all about skipping the code below. The '.' will turn your nice absolute path '/foo/bar' into relative path './foo/bar' and the lookup will fail. Instead of letting it fall through maybe something like this would be more expressive: if File::Spec->file_name_is_absolute($input) { return $input if -f $input; die "$progname: $input: cannot find input file" }
my @search_path = ("."); push (@search_path, @paths) if $use_path; foreach (@search_path) {
... the first search path is always '.'.
-- Pino Toscano
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
-- Tomáš Golembiovský <tgolembi@redhat.com>
On Monday, 27 January 2020 12:37:38 CET Tomáš Golembiovský wrote:
On Mon, Jan 27, 2020 at 12:17:42PM +0100, Pino Toscano wrote:
On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- podwrapper.pl.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/podwrapper.pl.in b/podwrapper.pl.in index f12a173f..1e4aa149 100755 --- a/podwrapper.pl.in +++ b/podwrapper.pl.in @@ -689,6 +689,8 @@ sub find_file my $use_path = shift; local $_;
+ return $input if File::Spec->file_name_is_absolute($input) and -f $input;
Do you really need to use file_name_is_absolute? -f seems to work fine also with absolute paths. In case the path is relative, -f will be fine too, as...
It's all about skipping the code below. The '.' will turn your nice absolute path '/foo/bar' into relative path './foo/bar' and the lookup will fail.
Oh sorry, most probably I did not explain properly what I meant. Since -f works on both absolute and relative paths, my suggestion is to change your line into a simpler: return $input if $input; and most probably removing '.' from the search loop below. -- Pino Toscano
On Mon, Jan 27, 2020 at 01:46:03PM +0100, Pino Toscano wrote:
On Monday, 27 January 2020 12:37:38 CET Tomáš Golembiovský wrote:
On Mon, Jan 27, 2020 at 12:17:42PM +0100, Pino Toscano wrote:
On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- podwrapper.pl.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/podwrapper.pl.in b/podwrapper.pl.in index f12a173f..1e4aa149 100755 --- a/podwrapper.pl.in +++ b/podwrapper.pl.in @@ -689,6 +689,8 @@ sub find_file my $use_path = shift; local $_;
+ return $input if File::Spec->file_name_is_absolute($input) and -f $input;
Do you really need to use file_name_is_absolute? -f seems to work fine also with absolute paths. In case the path is relative, -f will be fine too, as...
It's all about skipping the code below. The '.' will turn your nice absolute path '/foo/bar' into relative path './foo/bar' and the lookup will fail.
Oh sorry, most probably I did not explain properly what I meant.
Since -f works on both absolute and relative paths, my suggestion is to change your line into a simpler:
return $input if $input;
and most probably removing '.' from the search loop below.
Now I see what you mean. Yeah, that makes sense too. Tomas
-- Pino Toscano
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
-- Tomáš Golembiovský <tgolembi@redhat.com>
participants (2)
-
Pino Toscano -
Tomáš Golembiovský