[Skiboot] [PATCH] external: Rework libflash shared library Makefile

Wed May 18 15:33:01 AEST 2016

On Tue, May 17, 2016 at 10:02:01PM -0400, Brad Bishop wrote:
> Thanks Sam
> 
> > On May 17, 2016, at 9:43 PM, Samuel Mendoza-Jonas <sam at mendozajonas.com> wrote:
> > 
> > On Tue, May 17, 2016 at 07:53:33AM -0400, Brad Bishop wrote:
> >> The existing Makefile had a number of issues:
> >> - Redefined targets, caused make whining
> >> - Did not build on arm because of -m64
> >> 
> >> Start with a new approach:
> >> - build pflash directory and re-use .o files to assemble the solib.
> >> - build a version of pflash that uses the the solib
> >> 
> >> Signed-off-by: Brad Bishop <bradleyb at fuzziesquirrel.com <mailto:bradleyb at fuzziesquirrel.com>>
> > 
> > Hi Brad,
> > 
> > This looks like you're updating libflash such that pflash can use it as
> > a shared library, which I like! Unfortunately I'm pretty sure it breaks
> > the existing use case, which is to install libflash as a shared library
> > in a buildroot environment (op-build) for Petitboot.
> > Ideally external/shared should be use-case agnostic, and not know or
> > care about pflash or Petitboot - but this looks like it makes it a bit
> > flash-specific.
> 
> That makes sense.  How about an environment variable for external/pflash that tells the makefile whether or not to use the shared library when building the exe (by default static link)?
> 

That sounds good to me.

> > 
> >> ---
> >> external/pflash/rules.mk   |  4 +--
> >> external/shared/.gitignore |  2 ++
> >> external/shared/Makefile   | 74 +++++++++++++++++++---------------------------
> >> 3 files changed, 35 insertions(+), 45 deletions(-)
> >> create mode 100644 external/shared/.gitignore
> >> 
> >> diff --git a/external/pflash/rules.mk b/external/pflash/rules.mk
> >> index aa426b5..c0199e3 100644
> >> --- a/external/pflash/rules.mk
> >> +++ b/external/pflash/rules.mk
> >> @@ -1,12 +1,12 @@
> >> .DEFAULT_GOAL := all
> >> 
> >> override CFLAGS  += -O2 -Wall -I.
> >> -OBJS    = pflash.o progress.o version.o
> >> +PFLASH_OBJS    = pflash.o progress.o version.o common-arch_flash.o
> >> LIBFLASH_FILES := libflash.c libffs.c ecc.c blocklevel.c file.c
> >> LIBFLASH_OBJS := $(addprefix libflash-, $(LIBFLASH_FILES:.c=.o))
> >> LIBFLASH_SRC := $(addprefix libflash/,$(LIBFLASH_FILES))
> >> +OBJS	+= $(PFLASH_OBJS)
> >> OBJS	+= $(LIBFLASH_OBJS)
> >> -OBJS	+= common-arch_flash.o
> >> EXE     = pflash
> >> 
> >> PFLASH_VERSION ?= $(shell ../../make_version.sh $(EXE))
> >> diff --git a/external/shared/.gitignore b/external/shared/.gitignore
> >> new file mode 100644
> >> index 0000000..541d2a2
> >> --- /dev/null
> >> +++ b/external/shared/.gitignore
> >> @@ -0,0 +1,2 @@
> >> +pflash-dynamic
> >> +libflash.so*
> >> diff --git a/external/shared/Makefile b/external/shared/Makefile
> >> index 4c31657..aa1d881 100644
> >> --- a/external/shared/Makefile
> >> +++ b/external/shared/Makefile
> >> @@ -1,56 +1,44 @@
> >> -.DEFAULT_GOAL := all
> >> GET_ARCH = ../../external/common/get_arch.sh
> >> +include ../../external/pflash/rules.mk
> >> include ../../external/common/rules.mk
> >> 
> >> -PREFIX ?= /usr/local/
> >> -LIBDIR = $(PREFIX)/lib
> >> -INCDIR = $(PREFIX)/include/libflash
> > 
> > For example this will break installing libflash in op-build.
> 
> Sorry - what doesn’t work exactly?  Is it that the lib/headers aren’t being installed in /usr/local?  I could add the prefix back in and set it to empty over in openbmc land.

Right, it's mainly that we lose PREFIX - op-build will use that to
install them to the staging and/or target directories.

> 
> > 
> >> +libdir=/usr/lib
> >> +incdir=/usr/include
> >> +sbindir=/usr/sbin
> >> 
> >> -VERSION = $(shell ../../make_version.sh)
> >> +PFLASH_DIR=../pflash/
> >> +EXE=pflash-dynamic
> >> +LIB=libflash.so
> >> +SONAME=$(LIB).1
> >> +VERSION = $(shell ../../make_version.sh "$(SONAME)")
> >> +OBJS=$(addprefix $(PFLASH_DIR), $(LIBFLASH_OBJS))
> >> +OBJS+=$(addprefix $(PFLASH_DIR), $(ARCH_OBJS))
> >> +DYN_OBJS=$(addprefix $(PFLASH_DIR), $(PFLASH_OBJS))
> >> 
> >> -CFLAGS += -m64 -Werror -Wall -g2 -ggdb -I. -fPIC
> >> +LIBFLASH_HEADERS=libffs.h libflash.h blocklevel.h errors.h ffs.h
> >> +ARCHFLASH_HEADERS=arch_flash.h io.h
> >> 
> >> -.PHONY: links
> >> -links: libflash ccan common
> >> +all: $(EXE)
> >> 
> >> -libflash:
> >> -	ln -sf ../../libflash .
> >> +$(LIB):
> >> +	$(MAKE) -C $(PFLASH_DIR)
> >> +	$(Q_CC)$(CC) -shared -fPIC -Wl,-soname,$(SONAME) $(OBJS) -o $(VERSION)
> >> 
> >> -common:
> >> -	ln -sf ../common .
> >> +$(EXE): $(LIB)
> >> +	$(Q_CC)$(CC) $(CFLAGS) $(DYN_OBJS) $(VERSION) -lrt -o $(EXE)
> >> 
> >> -ccan:
> >> -	ln -sf ../../ccan .
> >> +install: install-bins install-headers
> >> 
> >> -LIBFLASH_OBJS = libflash-file.o libflash-libflash.o libflash-libffs.o libflash-ecc.o libflash-blocklevel.o
> >> -ARCHFLASH_OBJS = common-arch_flash.o
> >> -OBJS = $(LIBFLASH_OBJS) $(ARCHFLASH_OBJS)
> >> +install-headers:
> >> +	$(Q_MKDIR)mkdir -p $(DESTDIR)$(incdir)/libflash $(DESTDIR)$(incdir)/common
> >> +	install -m644 $(addprefix $(PFLASH_DIR)libflash/, $(LIBFLASH_HEADERS)) $(DESTDIR)$(incdir)/libflash
> >> +	install -m644 $(addprefix $(PFLASH_DIR)common/, $(ARCHFLASH_HEADERS)) $(DESTDIR)$(incdir)/common
> > 
> > As does this
> 
> Sorry..I’m new here.  Can you elaborate how it breaks?  Are we ok if I just install the so and headers to $(PREFIX) ?= /usr/local ?

Actually I misread this one, but yes it breaks because of the lack of
prefix. Where is DESTDIR set, I don't seem to see it?

> 
> > 
> >> 
> >> -LIBFLASH_H = libflash/file.h libflash/libflash.h libflash/libffs.h libflash/ffs.h libflash/ecc.h libflash/blocklevel.h libflash/errors.h
> >> -ARCHFLASH_H = common/arch_flash.h
> >> -
> >> -$(LIBFLASH_OBJS) : libflash-%.o : libflash/%.c
> >> -	$(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@
> >> +install-bins: all
> >> +	install -D $(EXE) $(DESTDIR)$(sbindir)/pflash
> >> +	install -D $(VERSION) $(DESTDIR)$(libdir)/$(VERSION)
> >> +	$(Q_LN)ln -sf $(VERSION) $(DESTDIR)$(libdir)/$(SONAME)
> >> +	$(Q_LN)ln -sf $(VERSION) $(DESTDIR)$(libdir)/$(LIB)
> > 
> > And this
> > 
> >> 
> >> clean:
> >> -	rm -f $(OBJS) common-*.o *.so*
> >> -
> >> -distclean: clean
> >> -	rm -f ccan libflash common
> >> -
> >> -all: links arch_links $(OBJS)
> >> -	$(CC) -shared -Wl,-soname,libflash.so -o libflash.so.$(VERSION) $(OBJS)
> >> -
> >> -install-lib: all
> >> -	install -D -m 0755 libflash.so.$(VERSION) $(LIBDIR)/libflash.so.$(VERSION)
> >> -	ln -sf libflash.so.$(VERSION) $(LIBDIR)/libflash.so
> >> -
> >> -install-dev: links arch_links
> >> -	mkdir -p $(INCDIR)
> >> -	install -m 0644 $(LIBFLASH_H) $(ARCHFLASH_H) $(INCDIR)
> >> -
> >> -install: install-lib install-dev
> >> -
> >> -uninstall:
> >> -	rm -f $(LIBDIR)/libflash*
> >> -	rm -rf $(INCDIR)
> >> +	rm -rf $(LIB).* $(EXE)
> >> -- 
> >> 1.8.3.1
> >> _______________________________________________
> >> Skiboot mailing list
> >> Skiboot at lists.ozlabs.org
> >> https://lists.ozlabs.org/listinfo/skiboot