diff mbox

[dpdk-dev] mk: fix libs installation when installing sdk

Message ID 20171214142411.25375-1-olivier.matz@6wind.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Olivier Matz Dec. 14, 2017, 2:24 p.m. UTC 
  From: Samuel Gauthier <samuel.gauthier@6wind.com>

The 'install-sdk' target creates an invalid symlink in the install
directory:

  # make install-sdk DESTDIR=/tmp/toto V=1
  [...]
  ln -snf $(/root/dpdk.org/buildtools/relpath.sh /tmp/toto/usr/local/lib \
            /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc/) \
       /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc/lib
  [...]
  # ls -l /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc
  drwxr-xr-x 2 root root 4096 Dec 11 11:49 app
  lrwxrwxrwx 1 root root   21 Dec 11 11:49 include -> ../../../include/dpdk
  lrwxrwxrwx 1 root root   12 Dec 11 11:49 lib -> ../../../lib  # BAD LINK

This happens because the directory is not created. Moreover, it makes
sense for install-sdk to install the libs, which could be necessary to
build something against dpdk. After the patch, the link is correct and
the *.a libs are properly installed:

  # ls -l /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc
  drwxr-xr-x 2 root root 4096 Dec 11 11:53 app
  lrwxrwxrwx 1 root root   21 Dec 11 11:53 include -> ../../../include/dpdk
  lrwxrwxrwx 1 root root   12 Dec 11 11:53 lib -> ../../../lib
  # ls -l /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc/lib/
  -rw-r--r-- 1 root root   1378 Dec  8 14:08 libdpdk.a
  -rw-r--r-- 1 root root 123380 Dec  8 14:02 librte_acl.a
  -rw-r--r-- 1 root root   3406 Dec  8 14:02 librte_bitratestats.a
  [...]

Fixes: 6efca1e9f873 ("mk: split install rule")
Cc: stable@dpdk.org

Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 mk/rte.sdkinstall.mk | 2 ++
 1 file changed, 2 insertions(+)

Comments

Thomas Monjalon Dec. 15, 2017, 10:19 a.m. UTC | #1 
14/12/2017 15:24, Olivier Matz:
> @@ -157,6 +157,8 @@ install-sdk:
>  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
>  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
>  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
>  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)

The libs are already installed with "make install-runtime".
Either we add a dependency between install-sdk and install-runtime,
or we clearly document it.
Olivier Matz Dec. 15, 2017, 10:25 a.m. UTC | #2 
On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> 14/12/2017 15:24, Olivier Matz:
> > @@ -157,6 +157,8 @@ install-sdk:
> >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> 
> The libs are already installed with "make install-runtime".
> Either we add a dependency between install-sdk and install-runtime,
> or we clearly document it.

To me, libs are needed when installing the sdk (to compile against them)
and when installing the runtime (to use them).

Is it a problem to have it in both targets?
Thomas Monjalon Dec. 15, 2017, 10:32 a.m. UTC | #3 
15/12/2017 11:25, Olivier MATZ:
> On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> > 14/12/2017 15:24, Olivier Matz:
> > > @@ -157,6 +157,8 @@ install-sdk:
> > >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> > >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> > 
> > The libs are already installed with "make install-runtime".
> > Either we add a dependency between install-sdk and install-runtime,
> > or we clearly document it.
> 
> To me, libs are needed when installing the sdk (to compile against them)
> and when installing the runtime (to use them).
> 
> Is it a problem to have it in both targets?

Yes it is a problem because the general use is to call every targets,
so the libs will be installed twice. Look at the global "install" target.

Do you want to be able to install the SDK without the runtime?
Olivier Matz Dec. 15, 2017, 10:45 a.m. UTC | #4 
On Fri, Dec 15, 2017 at 11:32:12AM +0100, Thomas Monjalon wrote:
> 15/12/2017 11:25, Olivier MATZ:
> > On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> > > 14/12/2017 15:24, Olivier Matz:
> > > > @@ -157,6 +157,8 @@ install-sdk:
> > > >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> > > >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > > > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > > > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> > > 
> > > The libs are already installed with "make install-runtime".
> > > Either we add a dependency between install-sdk and install-runtime,
> > > or we clearly document it.
> > 
> > To me, libs are needed when installing the sdk (to compile against them)
> > and when installing the runtime (to use them).
> > 
> > Is it a problem to have it in both targets?
> 
> Yes it is a problem because the general use is to call every targets,
> so the libs will be installed twice. Look at the global "install" target.
> 
> Do you want to be able to install the SDK without the runtime?

Hmm, you're right, installing the runtime instead of the sdk may be a
solution in our case. We don't need the bin, man, ... but it's probably
not an issue to have them anyway.

So, to summarize:
  install-runtime is the equivalent of the binary package
  install-sdk is the equivalent of the -devel package

And install-sdk depends on install-runtime, right?
Thomas Monjalon Dec. 15, 2017, 11 a.m. UTC | #5 
15/12/2017 11:45, Olivier MATZ:
> On Fri, Dec 15, 2017 at 11:32:12AM +0100, Thomas Monjalon wrote:
> > 15/12/2017 11:25, Olivier MATZ:
> > > On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> > > > 14/12/2017 15:24, Olivier Matz:
> > > > > @@ -157,6 +157,8 @@ install-sdk:
> > > > >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> > > > >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > > > > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > > > > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> > > > 
> > > > The libs are already installed with "make install-runtime".
> > > > Either we add a dependency between install-sdk and install-runtime,
> > > > or we clearly document it.
> > > 
> > > To me, libs are needed when installing the sdk (to compile against them)
> > > and when installing the runtime (to use them).
> > > 
> > > Is it a problem to have it in both targets?
> > 
> > Yes it is a problem because the general use is to call every targets,
> > so the libs will be installed twice. Look at the global "install" target.
> > 
> > Do you want to be able to install the SDK without the runtime?
> 
> Hmm, you're right, installing the runtime instead of the sdk may be a
> solution in our case. We don't need the bin, man, ... but it's probably
> not an issue to have them anyway.
> 
> So, to summarize:
>   install-runtime is the equivalent of the binary package
>   install-sdk is the equivalent of the -devel package

Yes

> And install-sdk depends on install-runtime, right?

Depends logically, yes. But no dependence in the Makefile.
Olivier Matz Jan. 23, 2018, 4:30 p.m. UTC | #6 
Hi Thomas,

On Fri, Dec 15, 2017 at 12:00:01PM +0100, Thomas Monjalon wrote:
> 15/12/2017 11:45, Olivier MATZ:
> > On Fri, Dec 15, 2017 at 11:32:12AM +0100, Thomas Monjalon wrote:
> > > 15/12/2017 11:25, Olivier MATZ:
> > > > On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> > > > > 14/12/2017 15:24, Olivier Matz:
> > > > > > @@ -157,6 +157,8 @@ install-sdk:
> > > > > >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> > > > > >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> > > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > > > > > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > > > > > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> > > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> > > > > 
> > > > > The libs are already installed with "make install-runtime".
> > > > > Either we add a dependency between install-sdk and install-runtime,
> > > > > or we clearly document it.
> > > > 
> > > > To me, libs are needed when installing the sdk (to compile against them)
> > > > and when installing the runtime (to use them).
> > > > 
> > > > Is it a problem to have it in both targets?
> > > 
> > > Yes it is a problem because the general use is to call every targets,
> > > so the libs will be installed twice. Look at the global "install" target.
> > > 
> > > Do you want to be able to install the SDK without the runtime?
> > 
> > Hmm, you're right, installing the runtime instead of the sdk may be a
> > solution in our case. We don't need the bin, man, ... but it's probably
> > not an issue to have them anyway.
> > 
> > So, to summarize:
> >   install-runtime is the equivalent of the binary package
> >   install-sdk is the equivalent of the -devel package
> 
> Yes
> 
> > And install-sdk depends on install-runtime, right?
> 
> Depends logically, yes. But no dependence in the Makefile.

Coming back on this topic, actually I think an issue remains with "make
install-sdk" which generates a bad symlink.

  # ls -l /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc
  drwxr-xr-x 2 root root 4096 Dec 11 11:49 app
  lrwxrwxrwx 1 root root   21 Dec 11 11:49 include -> ../../../include/dpdk
  lrwxrwxrwx 1 root root   12 Dec 11 11:49 lib -> ../../../lib  # BAD LINK

Would it make sense to remove the creation of the symlink? Something
like this:

  --- a/mk/rte.sdkinstall.mk
  +++ b/mk/rte.sdkinstall.mk
  @@ -157,7 +157,6 @@ install-sdk:
          $(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
          $(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
          $(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
  -       $(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
  
   install-doc:
   ifneq ($(wildcard $O/doc/html),)


Thanks,
Olivier
Olivier Matz June 25, 2018, 2:46 p.m. UTC | #7 
Hi Thomas,

On Tue, Jan 23, 2018 at 05:30:57PM +0100, Olivier Matz wrote:
> Hi Thomas,
> 
> On Fri, Dec 15, 2017 at 12:00:01PM +0100, Thomas Monjalon wrote:
> > 15/12/2017 11:45, Olivier MATZ:
> > > On Fri, Dec 15, 2017 at 11:32:12AM +0100, Thomas Monjalon wrote:
> > > > 15/12/2017 11:25, Olivier MATZ:
> > > > > On Fri, Dec 15, 2017 at 11:19:57AM +0100, Thomas Monjalon wrote:
> > > > > > 14/12/2017 15:24, Olivier Matz:
> > > > > > > @@ -157,6 +157,8 @@ install-sdk:
> > > > > > >  	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
> > > > > > >  	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
> > > > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
> > > > > > > +	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
> > > > > > > +	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
> > > > > > >  	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
> > > > > > 
> > > > > > The libs are already installed with "make install-runtime".
> > > > > > Either we add a dependency between install-sdk and install-runtime,
> > > > > > or we clearly document it.
> > > > > 
> > > > > To me, libs are needed when installing the sdk (to compile against them)
> > > > > and when installing the runtime (to use them).
> > > > > 
> > > > > Is it a problem to have it in both targets?
> > > > 
> > > > Yes it is a problem because the general use is to call every targets,
> > > > so the libs will be installed twice. Look at the global "install" target.
> > > > 
> > > > Do you want to be able to install the SDK without the runtime?
> > > 
> > > Hmm, you're right, installing the runtime instead of the sdk may be a
> > > solution in our case. We don't need the bin, man, ... but it's probably
> > > not an issue to have them anyway.
> > > 
> > > So, to summarize:
> > >   install-runtime is the equivalent of the binary package
> > >   install-sdk is the equivalent of the -devel package
> > 
> > Yes
> > 
> > > And install-sdk depends on install-runtime, right?
> > 
> > Depends logically, yes. But no dependence in the Makefile.
> 
> Coming back on this topic, actually I think an issue remains with "make
> install-sdk" which generates a bad symlink.
> 
>   # ls -l /tmp/toto/usr/local/share/dpdk/x86_64-native-linuxapp-gcc
>   drwxr-xr-x 2 root root 4096 Dec 11 11:49 app
>   lrwxrwxrwx 1 root root   21 Dec 11 11:49 include -> ../../../include/dpdk
>   lrwxrwxrwx 1 root root   12 Dec 11 11:49 lib -> ../../../lib  # BAD LINK
> 
> Would it make sense to remove the creation of the symlink? Something
> like this:
> 
>   --- a/mk/rte.sdkinstall.mk
>   +++ b/mk/rte.sdkinstall.mk
>   @@ -157,7 +157,6 @@ install-sdk:
>           $(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
>           $(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
>           $(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
>   -       $(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
>   
>    install-doc:
>    ifneq ($(wildcard $O/doc/html),)

It looks we forgot this thread for a long time. Any opinon about it?

Thanks
Olivier
diff mbox

Patch

diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 4e97feff9..338fb4971 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -157,6 +157,8 @@  install-sdk:
 	$(Q)cp -a               $O/.config               $(DESTDIR)$(targetdir)
 	$(Q)cp -a               $O/app/dpdk-pmdinfogen   $(DESTDIR)$(targetdir)/app
 	$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
+	$(Q)$(call rte_mkdir,                            $(DESTDIR)$(libdir))
+	$(Q)cp -a               $O/lib/*                 $(DESTDIR)$(libdir)
 	$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),     $(DESTDIR)$(targetdir)/lib)
 
 install-doc: