| Message ID | 1474923189-26431-1-git-send-email-william.c.roberts@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, Sep 26, 2016 at 1:53 PM, <william.c.roberts@intel.com> wrote: > From: William Roberts <william.c.roberts@intel.com> > > To build the selinux host configuration, specify > ANDROID_HOST=y on the Make command line. > > eg) > make ANDROID_HOST=y > > Signed-off-by: William Roberts <william.c.roberts@intel.com> > --- > libselinux/Makefile | 8 +++++++- > libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- > 2 files changed, 41 insertions(+), 17 deletions(-) > > diff --git a/libselinux/Makefile b/libselinux/Makefile > index 5a8d42c..50ae009 100644 > --- a/libselinux/Makefile > +++ b/libselinux/Makefile > @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) > override DISABLE_RPM=y > override DISABLE_BOOL=y > endif > +ifeq ($(ANDROID_HOST),y) > + override DISABLE_SETRANS=y > + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ argghhh .. missing too much, drop DISABLE_RPM... copy+paste+edit fail. Ill let v2 sit for a day, and aggregate other feedback for v3. Sorry for the noise. <snip>
On 09/26/2016 04:55 PM, William Roberts wrote: > On Mon, Sep 26, 2016 at 1:53 PM, <william.c.roberts@intel.com> wrote: >> From: William Roberts <william.c.roberts@intel.com> >> >> To build the selinux host configuration, specify >> ANDROID_HOST=y on the Make command line. >> >> eg) >> make ANDROID_HOST=y >> >> Signed-off-by: William Roberts <william.c.roberts@intel.com> >> --- >> libselinux/Makefile | 8 +++++++- >> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- >> 2 files changed, 41 insertions(+), 17 deletions(-) >> >> diff --git a/libselinux/Makefile b/libselinux/Makefile >> index 5a8d42c..50ae009 100644 >> --- a/libselinux/Makefile >> +++ b/libselinux/Makefile >> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >> override DISABLE_RPM=y >> override DISABLE_BOOL=y >> endif >> +ifeq ($(ANDROID_HOST),y) >> + override DISABLE_SETRANS=y >> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > > argghhh .. missing too much, drop DISABLE_RPM... copy+paste+edit fail. > Ill let v2 sit for a day, and aggregate other > feedback for v3. Sorry for the noise. Sorry, why can't you override DISABLE_RPM=y?
On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: > From: William Roberts <william.c.roberts@intel.com> > > To build the selinux host configuration, specify > ANDROID_HOST=y on the Make command line. > > eg) > make ANDROID_HOST=y Seems oddly named, neither corresponding to the #define it enables (BUILD_HOST) nor to the target platform. > > Signed-off-by: William Roberts <william.c.roberts@intel.com> > --- > libselinux/Makefile | 8 +++++++- > libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- > 2 files changed, 41 insertions(+), 17 deletions(-) > > diff --git a/libselinux/Makefile b/libselinux/Makefile > index 5a8d42c..50ae009 100644 > --- a/libselinux/Makefile > +++ b/libselinux/Makefile > @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) > override DISABLE_RPM=y > override DISABLE_BOOL=y > endif > +ifeq ($(ANDROID_HOST),y) > + override DISABLE_SETRANS=y > + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > + -DBUILD_HOST > + SUBDIRS = src > +endif Don't you actually want to also pick up utils/sefcontext_compile? That is built and used on the build host. And I'm not sure why we'd drop the other SUBDIRS. > ifeq ($(DISABLE_AVC),y) > EMFLAGS+= -DDISABLE_AVC > endif > @@ -22,7 +28,7 @@ endif > ifeq ($(DISABLE_SETRANS),y) > EMFLAGS+= -DDISABLE_SETRANS > endif > -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS > +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS ANDROID_HOST > > USE_PCRE2 ?= n > ifeq ($(USE_PCRE2),y) > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index 36e42b8..d841ef7 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -47,9 +47,17 @@ endif > ifeq ($(DISABLE_BOOL),y) > UNUSED_SRCS+=booleans.c > endif > +ifeq ($(ANDROID_HOST),y) > + SRCS=callbacks.c freecon.c label.c label_file.c \ > + label_android_property.c regex.c label_support.c \ > + matchpathcon.c setrans_client.c sha1.c > + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > + -DBUILD_HOST > +else > + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) > +endif > > GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i > -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) > > MAX_STACK_SIZE=32768 > > @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ $(EMFLAGS) > > SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS) > > +$(LIBA): $(OBJS) > + $(AR) rcs $@ $^ > + $(RANLIB) $@ > + > +$(LIBSO): $(LOBJS) > + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > + ln -sf $@ $(TARGET) > + > +%.o: %.c policy.h > + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< > + > +%.lo: %.c policy.h > + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< Did these truly need to move? I don't see why. > + > +# ANDROID_HOST Build option only builds the shared and static versions of > +# libselinux. > +ifeq ($(ANDROID_HOST),y) > + > +all: $(LIBA) $(LIBSO) > + > +else > + > all: $(LIBA) $(LIBSO) $(LIBPC) Is this worthwhile/necessary? The point of the build option IIUC is just to allow upstream testing that the Android build host version will still build, it shouldn't matter if there are extras. > > pywrap: all $(SWIGFILES) $(AUDIT2WHYSO) > @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ) > $(SWIGRUBYSO): $(SWIGRUBYLOBJ) > $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR) > > -$(LIBA): $(OBJS) > - $(AR) rcs $@ $^ > - $(RANLIB) $@ > - > -$(LIBSO): $(LOBJS) > - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > - ln -sf $@ $(TARGET) > - > $(LIBPC): $(LIBPC).in ../VERSION > sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ > > @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c > $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) > $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux $(LIBDIR)/libsepol.a -L$(LIBDIR) > > -%.o: %.c policy.h > - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< > - > -%.lo: %.c policy.h > - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< > - > $(SWIGCOUT): $(SWIGIF) > $(SWIG) $< > > @@ -178,4 +194,6 @@ distclean: clean > indent: > ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) > > -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean > +.PHONY: clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean > +endif > +.PHONY: all Why is this needed? BTW, this option can be dangerous. Don't build with it and then do a make install later without doing a make clean - you'll brick your Linux system ;(
On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: >> From: William Roberts <william.c.roberts@intel.com> >> >> To build the selinux host configuration, specify >> ANDROID_HOST=y on the Make command line. >> >> eg) >> make ANDROID_HOST=y > > Seems oddly named, neither corresponding to the #define it enables > (BUILD_HOST) nor to the target platform. We could change this to BUILD_HOST=y to enable all of it, but considering that this build is specific for Android, I thought the naming to be more appropriate. Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. > >> >> Signed-off-by: William Roberts <william.c.roberts@intel.com> >> --- >> libselinux/Makefile | 8 +++++++- >> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- >> 2 files changed, 41 insertions(+), 17 deletions(-) >> >> diff --git a/libselinux/Makefile b/libselinux/Makefile >> index 5a8d42c..50ae009 100644 >> --- a/libselinux/Makefile >> +++ b/libselinux/Makefile >> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >> override DISABLE_RPM=y >> override DISABLE_BOOL=y >> endif >> +ifeq ($(ANDROID_HOST),y) >> + override DISABLE_SETRANS=y >> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >> + -DBUILD_HOST >> + SUBDIRS = src >> +endif > > Don't you actually want to also pick up utils/sefcontext_compile? > That is built and used on the build host. And I'm not sure why we'd > drop the other SUBDIRS. You'll start running into linking issues if things that use libselinux, use something not in the build host IIRC. Perhaps, if that is the issue, we just limit it to sefcontext_compile. > >> ifeq ($(DISABLE_AVC),y) >> EMFLAGS+= -DDISABLE_AVC >> endif >> @@ -22,7 +28,7 @@ endif >> ifeq ($(DISABLE_SETRANS),y) >> EMFLAGS+= -DDISABLE_SETRANS >> endif >> -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS >> +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS ANDROID_HOST >> >> USE_PCRE2 ?= n >> ifeq ($(USE_PCRE2),y) >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile >> index 36e42b8..d841ef7 100644 >> --- a/libselinux/src/Makefile >> +++ b/libselinux/src/Makefile >> @@ -47,9 +47,17 @@ endif >> ifeq ($(DISABLE_BOOL),y) >> UNUSED_SRCS+=booleans.c >> endif >> +ifeq ($(ANDROID_HOST),y) >> + SRCS=callbacks.c freecon.c label.c label_file.c \ >> + label_android_property.c regex.c label_support.c \ >> + matchpathcon.c setrans_client.c sha1.c >> + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >> + -DBUILD_HOST >> +else >> + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) >> +endif >> >> GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i >> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) >> >> MAX_STACK_SIZE=32768 >> >> @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ $(EMFLAGS) >> >> SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS) >> >> +$(LIBA): $(OBJS) >> + $(AR) rcs $@ $^ >> + $(RANLIB) $@ >> + >> +$(LIBSO): $(LOBJS) >> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro >> + ln -sf $@ $(TARGET) >> + >> +%.o: %.c policy.h >> + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< >> + >> +%.lo: %.c policy.h >> + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< > > Did these truly need to move? I don't see why. this prevents us from having multiple definitions of the recipes or multiple if's on ANDROID_HOST=y. Both flavors need these wildcard targets. > >> + >> +# ANDROID_HOST Build option only builds the shared and static versions of >> +# libselinux. >> +ifeq ($(ANDROID_HOST),y) >> + >> +all: $(LIBA) $(LIBSO) >> + >> +else >> + >> all: $(LIBA) $(LIBSO) $(LIBPC) > > Is this worthwhile/necessary? The point of the build option IIUC is > just to allow upstream testing that the Android build host version will > still build, it shouldn't matter if there are extras. Those things die on linking... so I didn't want to submit something where Make returns not 0. > >> >> pywrap: all $(SWIGFILES) $(AUDIT2WHYSO) >> @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ) >> $(SWIGRUBYSO): $(SWIGRUBYLOBJ) >> $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR) >> >> -$(LIBA): $(OBJS) >> - $(AR) rcs $@ $^ >> - $(RANLIB) $@ >> - >> -$(LIBSO): $(LOBJS) >> - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro >> - ln -sf $@ $(TARGET) >> - >> $(LIBPC): $(LIBPC).in ../VERSION >> sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ >> >> @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c >> $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) >> $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux $(LIBDIR)/libsepol.a -L$(LIBDIR) >> >> -%.o: %.c policy.h >> - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< >> - >> -%.lo: %.c policy.h >> - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< >> - >> $(SWIGCOUT): $(SWIGIF) >> $(SWIG) $< >> >> @@ -178,4 +194,6 @@ distclean: clean >> indent: >> ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) >> >> -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean >> +.PHONY: clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean >> +endif >> +.PHONY: all > > Why is this needed? The targets for %.o and %.lo are used on all build targets, the rest is committed on ANDROID_HOST=y. So for the stuff that is separated out, we have a .PHONY for it. For the things the define in common, notably ALL, we have a shared .PHONY for them. > > BTW, this option can be dangerous. Don't build with it and then do a > make install later without doing a make clean - you'll brick your Linux > system ;( That is true, but isn't that the point of SE Linux :-P. Kidding aside, perhaps we could create a guard file that's checked on install, if its there, you need to run make clean first.
On Tue, Sep 27, 2016 at 7:03 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 09/26/2016 04:55 PM, William Roberts wrote: >> On Mon, Sep 26, 2016 at 1:53 PM, <william.c.roberts@intel.com> wrote: >>> From: William Roberts <william.c.roberts@intel.com> >>> >>> To build the selinux host configuration, specify >>> ANDROID_HOST=y on the Make command line. >>> >>> eg) >>> make ANDROID_HOST=y >>> >>> Signed-off-by: William Roberts <william.c.roberts@intel.com> >>> --- >>> libselinux/Makefile | 8 +++++++- >>> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- >>> 2 files changed, 41 insertions(+), 17 deletions(-) >>> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>> index 5a8d42c..50ae009 100644 >>> --- a/libselinux/Makefile >>> +++ b/libselinux/Makefile >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>> override DISABLE_RPM=y >>> override DISABLE_BOOL=y >>> endif >>> +ifeq ($(ANDROID_HOST),y) >>> + override DISABLE_SETRANS=y >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >> >> argghhh .. missing too much, drop DISABLE_RPM... copy+paste+edit fail. >> Ill let v2 sit for a day, and aggregate other >> feedback for v3. Sorry for the noise. > > Sorry, why can't you override DISABLE_RPM=y? > It's superfluous because it;s not used in the source files built for ANDROID_HOST, and it doesn't match the build recipe for Android. So if the goal would be to provide a quick flag to test the build against, it wouldn't meet that requirement.
On 09/27/2016 11:08 AM, William Roberts wrote: > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: >>> From: William Roberts <william.c.roberts@intel.com> >>> >>> To build the selinux host configuration, specify >>> ANDROID_HOST=y on the Make command line. >>> >>> eg) >>> make ANDROID_HOST=y >> >> Seems oddly named, neither corresponding to the #define it enables >> (BUILD_HOST) nor to the target platform. > > We could change this to BUILD_HOST=y to enable all of it, but considering > that this build is specific for Android, I thought the naming to be more > appropriate. > > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. Ok, fair point. Actually EMBEDDED=y is broken and no one is using it AFAIK so we should probably kill it at some point separately. > >> >>> >>> Signed-off-by: William Roberts <william.c.roberts@intel.com> >>> --- >>> libselinux/Makefile | 8 +++++++- >>> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- >>> 2 files changed, 41 insertions(+), 17 deletions(-) >>> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>> index 5a8d42c..50ae009 100644 >>> --- a/libselinux/Makefile >>> +++ b/libselinux/Makefile >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>> override DISABLE_RPM=y >>> override DISABLE_BOOL=y >>> endif >>> +ifeq ($(ANDROID_HOST),y) >>> + override DISABLE_SETRANS=y >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >>> + -DBUILD_HOST >>> + SUBDIRS = src >>> +endif >> >> Don't you actually want to also pick up utils/sefcontext_compile? >> That is built and used on the build host. And I'm not sure why we'd >> drop the other SUBDIRS. > > You'll start running into linking issues if things that use > libselinux, use something not > in the build host IIRC. Perhaps, if that is the issue, we just limit > it to sefcontext_compile. Sure, the utils/Makefile can remove entries the same way it already does for DISABLE_*. > >> >>> ifeq ($(DISABLE_AVC),y) >>> EMFLAGS+= -DDISABLE_AVC >>> endif >>> @@ -22,7 +28,7 @@ endif >>> ifeq ($(DISABLE_SETRANS),y) >>> EMFLAGS+= -DDISABLE_SETRANS >>> endif >>> -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS >>> +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS ANDROID_HOST >>> >>> USE_PCRE2 ?= n >>> ifeq ($(USE_PCRE2),y) >>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile >>> index 36e42b8..d841ef7 100644 >>> --- a/libselinux/src/Makefile >>> +++ b/libselinux/src/Makefile >>> @@ -47,9 +47,17 @@ endif >>> ifeq ($(DISABLE_BOOL),y) >>> UNUSED_SRCS+=booleans.c >>> endif >>> +ifeq ($(ANDROID_HOST),y) >>> + SRCS=callbacks.c freecon.c label.c label_file.c \ >>> + label_android_property.c regex.c label_support.c \ >>> + matchpathcon.c setrans_client.c sha1.c >>> + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >>> + -DBUILD_HOST >>> +else >>> + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) >>> +endif >>> >>> GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i >>> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) >>> >>> MAX_STACK_SIZE=32768 >>> >>> @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ $(EMFLAGS) >>> >>> SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS) >>> >>> +$(LIBA): $(OBJS) >>> + $(AR) rcs $@ $^ >>> + $(RANLIB) $@ >>> + >>> +$(LIBSO): $(LOBJS) >>> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro >>> + ln -sf $@ $(TARGET) >>> + >>> +%.o: %.c policy.h >>> + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< >>> + >>> +%.lo: %.c policy.h >>> + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< >> >> Did these truly need to move? I don't see why. > > this prevents us from having multiple definitions of the recipes or > multiple if's on ANDROID_HOST=y. > Both flavors need these wildcard targets. If I undo all of these changes, make ANDROID_HOST=y clean all works just fine. So does make install. You don't need any of this conditional logic in the Makefile, and it just makes it harder to read and maintain. > >> >>> + >>> +# ANDROID_HOST Build option only builds the shared and static versions of >>> +# libselinux. >>> +ifeq ($(ANDROID_HOST),y) >>> + >>> +all: $(LIBA) $(LIBSO) >>> + >>> +else >>> + >>> all: $(LIBA) $(LIBSO) $(LIBPC) >> >> Is this worthwhile/necessary? The point of the build option IIUC is >> just to allow upstream testing that the Android build host version will >> still build, it shouldn't matter if there are extras. > > Those things die on linking... so I didn't want to submit something > where Make returns not 0. > >> >>> >>> pywrap: all $(SWIGFILES) $(AUDIT2WHYSO) >>> @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ) >>> $(SWIGRUBYSO): $(SWIGRUBYLOBJ) >>> $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR) >>> >>> -$(LIBA): $(OBJS) >>> - $(AR) rcs $@ $^ >>> - $(RANLIB) $@ >>> - >>> -$(LIBSO): $(LOBJS) >>> - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro >>> - ln -sf $@ $(TARGET) >>> - >>> $(LIBPC): $(LIBPC).in ../VERSION >>> sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ >>> >>> @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c >>> $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) >>> $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux $(LIBDIR)/libsepol.a -L$(LIBDIR) >>> >>> -%.o: %.c policy.h >>> - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< >>> - >>> -%.lo: %.c policy.h >>> - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< >>> - >>> $(SWIGCOUT): $(SWIGIF) >>> $(SWIG) $< >>> >>> @@ -178,4 +194,6 @@ distclean: clean >>> indent: >>> ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) >>> >>> -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean >>> +.PHONY: clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean >>> +endif >>> +.PHONY: all >> >> Why is this needed? > > The targets for %.o and %.lo are used on all build targets, the rest > is committed on ANDROID_HOST=y. > So for the stuff that is separated out, we have a .PHONY for it. For > the things the define in common, > notably ALL, we have a shared .PHONY for them. > >> >> BTW, this option can be dangerous. Don't build with it and then do a >> make install later without doing a make clean - you'll brick your Linux >> system ;( > > That is true, but isn't that the point of SE Linux :-P. Kidding aside, > perhaps we > could create a guard file that's checked on install, if its there, you need to > run make clean first. >
On Sep 27, 2016 09:50, "Stephen Smalley" <sds@tycho.nsa.gov> wrote: > > On 09/27/2016 11:08 AM, William Roberts wrote: > > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: > >>> From: William Roberts <william.c.roberts@intel.com> > >>> > >>> To build the selinux host configuration, specify > >>> ANDROID_HOST=y on the Make command line. > >>> > >>> eg) > >>> make ANDROID_HOST=y > >> > >> Seems oddly named, neither corresponding to the #define it enables > >> (BUILD_HOST) nor to the target platform. > > > > We could change this to BUILD_HOST=y to enable all of it, but considering > > that this build is specific for Android, I thought the naming to be more > > appropriate. > > > > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. > > Ok, fair point. Actually EMBEDDED=y is broken and no one is using it > AFAIK so we should probably kill it at some point separately. > > > > >> > >>> > >>> Signed-off-by: William Roberts <william.c.roberts@intel.com> > >>> --- > >>> libselinux/Makefile | 8 +++++++- > >>> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- > >>> 2 files changed, 41 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/libselinux/Makefile b/libselinux/Makefile > >>> index 5a8d42c..50ae009 100644 > >>> --- a/libselinux/Makefile > >>> +++ b/libselinux/Makefile > >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) > >>> override DISABLE_RPM=y > >>> override DISABLE_BOOL=y > >>> endif > >>> +ifeq ($(ANDROID_HOST),y) > >>> + override DISABLE_SETRANS=y > >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > >>> + -DBUILD_HOST > >>> + SUBDIRS = src > >>> +endif > >> > >> Don't you actually want to also pick up utils/sefcontext_compile? > >> That is built and used on the build host. And I'm not sure why we'd > >> drop the other SUBDIRS. > > > > You'll start running into linking issues if things that use > > libselinux, use something not > > in the build host IIRC. Perhaps, if that is the issue, we just limit > > it to sefcontext_compile. > > Sure, the utils/Makefile can remove entries the same way it already does > for DISABLE_*. > > > >> > >>> ifeq ($(DISABLE_AVC),y) > >>> EMFLAGS+= -DDISABLE_AVC > >>> endif > >>> @@ -22,7 +28,7 @@ endif > >>> ifeq ($(DISABLE_SETRANS),y) > >>> EMFLAGS+= -DDISABLE_SETRANS > >>> endif > >>> -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS > >>> +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS ANDROID_HOST > >>> > >>> USE_PCRE2 ?= n > >>> ifeq ($(USE_PCRE2),y) > >>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > >>> index 36e42b8..d841ef7 100644 > >>> --- a/libselinux/src/Makefile > >>> +++ b/libselinux/src/Makefile > >>> @@ -47,9 +47,17 @@ endif > >>> ifeq ($(DISABLE_BOOL),y) > >>> UNUSED_SRCS+=booleans.c > >>> endif > >>> +ifeq ($(ANDROID_HOST),y) > >>> + SRCS=callbacks.c freecon.c label.c label_file.c \ > >>> + label_android_property.c regex.c label_support.c \ > >>> + matchpathcon.c setrans_client.c sha1.c > >>> + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > >>> + -DBUILD_HOST > >>> +else > >>> + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) > >>> +endif > >>> > >>> GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i > >>> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) > >>> > >>> MAX_STACK_SIZE=32768 > >>> > >>> @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ $(EMFLAGS) > >>> > >>> SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS) > >>> > >>> +$(LIBA): $(OBJS) > >>> + $(AR) rcs $@ $^ > >>> + $(RANLIB) $@ > >>> + > >>> +$(LIBSO): $(LOBJS) > >>> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > >>> + ln -sf $@ $(TARGET) > >>> + > >>> +%.o: %.c policy.h > >>> + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< > >>> + > >>> +%.lo: %.c policy.h > >>> + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< > >> > >> Did these truly need to move? I don't see why. > > > > this prevents us from having multiple definitions of the recipes or > > multiple if's on ANDROID_HOST=y. > > Both flavors need these wildcard targets. > > If I undo all of these changes, make ANDROID_HOST=y clean all works just > fine. So does make install. You don't need any of this conditional > logic in the Makefile, and it just makes it harder to read and maintain. > > > > >> > >>> + > >>> +# ANDROID_HOST Build option only builds the shared and static versions of > >>> +# libselinux. > >>> +ifeq ($(ANDROID_HOST),y) > >>> + > >>> +all: $(LIBA) $(LIBSO) > >>> + > >>> +else > >>> + > >>> all: $(LIBA) $(LIBSO) $(LIBPC) > >> > >> Is this worthwhile/necessary? The point of the build option IIUC is > >> just to allow upstream testing that the Android build host version will > >> still build, it shouldn't matter if there are extras. > > > > Those things die on linking... so I didn't want to submit something > > where Make returns not 0. > > > >> > >>> > >>> pywrap: all $(SWIGFILES) $(AUDIT2WHYSO) > >>> @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ) > >>> $(SWIGRUBYSO): $(SWIGRUBYLOBJ) > >>> $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR) > >>> > >>> -$(LIBA): $(OBJS) > >>> - $(AR) rcs $@ $^ > >>> - $(RANLIB) $@ > >>> - > >>> -$(LIBSO): $(LOBJS) > >>> - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > >>> - ln -sf $@ $(TARGET) > >>> - > >>> $(LIBPC): $(LIBPC).in ../VERSION > >>> sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ > >>> > >>> @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c > >>> $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) > >>> $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux $(LIBDIR)/libsepol.a -L$(LIBDIR) > >>> > >>> -%.o: %.c policy.h > >>> - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< > >>> - > >>> -%.lo: %.c policy.h > >>> - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< > >>> - > >>> $(SWIGCOUT): $(SWIGIF) > >>> $(SWIG) $< > >>> > >>> @@ -178,4 +194,6 @@ distclean: clean > >>> indent: > >>> ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) > >>> > >>> -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean > >>> +.PHONY: clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean > >>> +endif > >>> +.PHONY: all > >> > >> Why is this needed? > > > > The targets for %.o and %.lo are used on all build targets, the rest > > is committed on ANDROID_HOST=y. > > So for the stuff that is separated out, we have a .PHONY for it. For > > the things the define in common, > > notably ALL, we have a shared .PHONY for them. > > > >> > >> BTW, this option can be dangerous. Don't build with it and then do a > >> make install later without doing a make clean - you'll brick your Linux > >> system ;( > > > > That is true, but isn't that the point of SE Linux :-P. Kidding aside, > > perhaps we > > could create a guard file that's checked on install, if its there, you need to > > run make clean first. > > OK good to know, I ran into issues, but it might have been fore I had the rest of it sorted out.
On 09/27/2016 11:08 AM, William Roberts wrote: > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: >>> From: William Roberts <william.c.roberts@intel.com> >>> >>> To build the selinux host configuration, specify >>> ANDROID_HOST=y on the Make command line. >>> >>> eg) >>> make ANDROID_HOST=y >> >> Seems oddly named, neither corresponding to the #define it enables >> (BUILD_HOST) nor to the target platform. > > We could change this to BUILD_HOST=y to enable all of it, but considering > that this build is specific for Android, I thought the naming to be more > appropriate. > > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. > >> >>> >>> Signed-off-by: William Roberts <william.c.roberts@intel.com> >>> --- >>> libselinux/Makefile | 8 +++++++- >>> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- >>> 2 files changed, 41 insertions(+), 17 deletions(-) >>> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>> index 5a8d42c..50ae009 100644 >>> --- a/libselinux/Makefile >>> +++ b/libselinux/Makefile >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>> override DISABLE_RPM=y >>> override DISABLE_BOOL=y >>> endif >>> +ifeq ($(ANDROID_HOST),y) >>> + override DISABLE_SETRANS=y >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ >>> + -DBUILD_HOST >>> + SUBDIRS = src >>> +endif Also, this is redundant; you can handle it entirely within libselinux/src/Makefile without anything here.
On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov> wrote: > > On 09/27/2016 11:08 AM, William Roberts wrote: > > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com wrote: > >>> From: William Roberts <william.c.roberts@intel.com> > >>> > >>> To build the selinux host configuration, specify > >>> ANDROID_HOST=y on the Make command line. > >>> > >>> eg) > >>> make ANDROID_HOST=y > >> > >> Seems oddly named, neither corresponding to the #define it enables > >> (BUILD_HOST) nor to the target platform. > > > > We could change this to BUILD_HOST=y to enable all of it, but considering > > that this build is specific for Android, I thought the naming to be more > > appropriate. > > > > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. > > > >> > >>> > >>> Signed-off-by: William Roberts <william.c.roberts@intel.com> > >>> --- > >>> libselinux/Makefile | 8 +++++++- > >>> libselinux/src/Makefile | 50 +++++++++++++++++++++++++++++++++---------------- > >>> 2 files changed, 41 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/libselinux/Makefile b/libselinux/Makefile > >>> index 5a8d42c..50ae009 100644 > >>> --- a/libselinux/Makefile > >>> +++ b/libselinux/Makefile > >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) > >>> override DISABLE_RPM=y > >>> override DISABLE_BOOL=y > >>> endif > >>> +ifeq ($(ANDROID_HOST),y) > >>> + override DISABLE_SETRANS=y > >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ > >>> + -DBUILD_HOST > >>> + SUBDIRS = src > >>> +endif > > Also, this is redundant; you can handle it entirely within > libselinux/src/Makefile without anything here. You mean all the ANDROID _HOST stuff? I didn't want to depart from what's there, that seemed to be the spot for disabling things.
On 09/27/2016 02:43 PM, William Roberts wrote: > On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov > <mailto:sds@tycho.nsa.gov>> wrote: >> >> On 09/27/2016 11:08 AM, William Roberts wrote: >> > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov > <mailto:sds@tycho.nsa.gov>> wrote: >> >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com > <mailto:william.c.roberts@intel.com> wrote: >> >>> From: William Roberts <william.c.roberts@intel.com > <mailto:william.c.roberts@intel.com>> >> >>> >> >>> To build the selinux host configuration, specify >> >>> ANDROID_HOST=y on the Make command line. >> >>> >> >>> eg) >> >>> make ANDROID_HOST=y >> >> >> >> Seems oddly named, neither corresponding to the #define it enables >> >> (BUILD_HOST) nor to the target platform. >> > >> > We could change this to BUILD_HOST=y to enable all of it, but > considering >> > that this build is specific for Android, I thought the naming to be more >> > appropriate. >> > >> > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. >> > >> >> >> >>> >> >>> Signed-off-by: William Roberts <william.c.roberts@intel.com > <mailto:william.c.roberts@intel.com>> >> >>> --- >> >>> libselinux/Makefile | 8 +++++++- >> >>> libselinux/src/Makefile | 50 > +++++++++++++++++++++++++++++++++---------------- >> >>> 2 files changed, 41 insertions(+), 17 deletions(-) >> >>> >> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile >> >>> index 5a8d42c..50ae009 100644 >> >>> --- a/libselinux/Makefile >> >>> +++ b/libselinux/Makefile >> >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >> >>> override DISABLE_RPM=y >> >>> override DISABLE_BOOL=y >> >>> endif >> >>> +ifeq ($(ANDROID_HOST),y) >> >>> + override DISABLE_SETRANS=y >> >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND > -DNO_X_BACKEND \ >> >>> + -DBUILD_HOST >> >>> + SUBDIRS = src >> >>> +endif >> >> Also, this is redundant; you can handle it entirely within >> libselinux/src/Makefile without anything here. > > You mean all the ANDROID _HOST stuff? I didn't want to depart from > what's there, that seemed to be the spot for disabling things. You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except in src/Makefile, so you don't need to set them here.
On Tue, Sep 27, 2016 at 11:51 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 09/27/2016 02:43 PM, William Roberts wrote: >> On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov >> <mailto:sds@tycho.nsa.gov>> wrote: >>> >>> On 09/27/2016 11:08 AM, William Roberts wrote: >>> > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov >> <mailto:sds@tycho.nsa.gov>> wrote: >>> >> On 09/26/2016 04:53 PM, william.c.roberts@intel.com >> <mailto:william.c.roberts@intel.com> wrote: >>> >>> From: William Roberts <william.c.roberts@intel.com >> <mailto:william.c.roberts@intel.com>> >>> >>> >>> >>> To build the selinux host configuration, specify >>> >>> ANDROID_HOST=y on the Make command line. >>> >>> >>> >>> eg) >>> >>> make ANDROID_HOST=y >>> >> >>> >> Seems oddly named, neither corresponding to the #define it enables >>> >> (BUILD_HOST) nor to the target platform. >>> > >>> > We could change this to BUILD_HOST=y to enable all of it, but >> considering >>> > that this build is specific for Android, I thought the naming to be more >>> > appropriate. >>> > >>> > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. >>> > >>> >> >>> >>> >>> >>> Signed-off-by: William Roberts <william.c.roberts@intel.com >> <mailto:william.c.roberts@intel.com>> >>> >>> --- >>> >>> libselinux/Makefile | 8 +++++++- >>> >>> libselinux/src/Makefile | 50 >> +++++++++++++++++++++++++++++++++---------------- >>> >>> 2 files changed, 41 insertions(+), 17 deletions(-) >>> >>> >>> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>> >>> index 5a8d42c..50ae009 100644 >>> >>> --- a/libselinux/Makefile >>> >>> +++ b/libselinux/Makefile >>> >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>> >>> override DISABLE_RPM=y >>> >>> override DISABLE_BOOL=y >>> >>> endif >>> >>> +ifeq ($(ANDROID_HOST),y) >>> >>> + override DISABLE_SETRANS=y >>> >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND >> -DNO_X_BACKEND \ >>> >>> + -DBUILD_HOST >>> >>> + SUBDIRS = src >>> >>> +endif >>> >>> Also, this is redundant; you can handle it entirely within >>> libselinux/src/Makefile without anything here. >> >> You mean all the ANDROID _HOST stuff? I didn't want to depart from >> what's there, that seemed to be the spot for disabling things. > > You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except > in src/Makefile, so you don't need to set them here. > The same could be said about DISABLE_SETRANS
On 09/27/2016 03:03 PM, William Roberts wrote: > On Tue, Sep 27, 2016 at 11:51 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 09/27/2016 02:43 PM, William Roberts wrote: >>> On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov >>> <mailto:sds@tycho.nsa.gov>> wrote: >>>> >>>> On 09/27/2016 11:08 AM, William Roberts wrote: >>>>> On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov >>> <mailto:sds@tycho.nsa.gov>> wrote: >>>>>> On 09/26/2016 04:53 PM, william.c.roberts@intel.com >>> <mailto:william.c.roberts@intel.com> wrote: >>>>>>> From: William Roberts <william.c.roberts@intel.com >>> <mailto:william.c.roberts@intel.com>> >>>>>>> >>>>>>> To build the selinux host configuration, specify >>>>>>> ANDROID_HOST=y on the Make command line. >>>>>>> >>>>>>> eg) >>>>>>> make ANDROID_HOST=y >>>>>> >>>>>> Seems oddly named, neither corresponding to the #define it enables >>>>>> (BUILD_HOST) nor to the target platform. >>>>> >>>>> We could change this to BUILD_HOST=y to enable all of it, but >>> considering >>>>> that this build is specific for Android, I thought the naming to be more >>>>> appropriate. >>>>> >>>>> Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. >>>>> >>>>>> >>>>>>> >>>>>>> Signed-off-by: William Roberts <william.c.roberts@intel.com >>> <mailto:william.c.roberts@intel.com>> >>>>>>> --- >>>>>>> libselinux/Makefile | 8 +++++++- >>>>>>> libselinux/src/Makefile | 50 >>> +++++++++++++++++++++++++++++++++---------------- >>>>>>> 2 files changed, 41 insertions(+), 17 deletions(-) >>>>>>> >>>>>>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>>>>>> index 5a8d42c..50ae009 100644 >>>>>>> --- a/libselinux/Makefile >>>>>>> +++ b/libselinux/Makefile >>>>>>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>>>>>> override DISABLE_RPM=y >>>>>>> override DISABLE_BOOL=y >>>>>>> endif >>>>>>> +ifeq ($(ANDROID_HOST),y) >>>>>>> + override DISABLE_SETRANS=y >>>>>>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND >>> -DNO_X_BACKEND \ >>>>>>> + -DBUILD_HOST >>>>>>> + SUBDIRS = src >>>>>>> +endif >>>> >>>> Also, this is redundant; you can handle it entirely within >>>> libselinux/src/Makefile without anything here. >>> >>> You mean all the ANDROID _HOST stuff? I didn't want to depart from >>> what's there, that seemed to be the spot for disabling things. >> >> You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except >> in src/Makefile, so you don't need to set them here. >> > > The same could be said about DISABLE_SETRANS It isn't set in both Makefiles. Pick one.
On Tue, Sep 27, 2016 at 12:08 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 09/27/2016 03:03 PM, William Roberts wrote: >> On Tue, Sep 27, 2016 at 11:51 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >>> On 09/27/2016 02:43 PM, William Roberts wrote: >>>> On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov >>>> <mailto:sds@tycho.nsa.gov>> wrote: >>>>> >>>>> On 09/27/2016 11:08 AM, William Roberts wrote: >>>>>> On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov >>>> <mailto:sds@tycho.nsa.gov>> wrote: >>>>>>> On 09/26/2016 04:53 PM, william.c.roberts@intel.com >>>> <mailto:william.c.roberts@intel.com> wrote: >>>>>>>> From: William Roberts <william.c.roberts@intel.com >>>> <mailto:william.c.roberts@intel.com>> >>>>>>>> >>>>>>>> To build the selinux host configuration, specify >>>>>>>> ANDROID_HOST=y on the Make command line. >>>>>>>> >>>>>>>> eg) >>>>>>>> make ANDROID_HOST=y >>>>>>> >>>>>>> Seems oddly named, neither corresponding to the #define it enables >>>>>>> (BUILD_HOST) nor to the target platform. >>>>>> >>>>>> We could change this to BUILD_HOST=y to enable all of it, but >>>> considering >>>>>> that this build is specific for Android, I thought the naming to be more >>>>>> appropriate. >>>>>> >>>>>> Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. >>>>>> >>>>>>> >>>>>>>> >>>>>>>> Signed-off-by: William Roberts <william.c.roberts@intel.com >>>> <mailto:william.c.roberts@intel.com>> >>>>>>>> --- >>>>>>>> libselinux/Makefile | 8 +++++++- >>>>>>>> libselinux/src/Makefile | 50 >>>> +++++++++++++++++++++++++++++++++---------------- >>>>>>>> 2 files changed, 41 insertions(+), 17 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>>>>>>> index 5a8d42c..50ae009 100644 >>>>>>>> --- a/libselinux/Makefile >>>>>>>> +++ b/libselinux/Makefile >>>>>>>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>>>>>>> override DISABLE_RPM=y >>>>>>>> override DISABLE_BOOL=y >>>>>>>> endif >>>>>>>> +ifeq ($(ANDROID_HOST),y) >>>>>>>> + override DISABLE_SETRANS=y >>>>>>>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND >>>> -DNO_X_BACKEND \ >>>>>>>> + -DBUILD_HOST >>>>>>>> + SUBDIRS = src >>>>>>>> +endif >>>>> >>>>> Also, this is redundant; you can handle it entirely within >>>>> libselinux/src/Makefile without anything here. >>>> >>>> You mean all the ANDROID _HOST stuff? I didn't want to depart from >>>> what's there, that seemed to be the spot for disabling things. >>> >>> You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except >>> in src/Makefile, so you don't need to set them here. >>> >> >> The same could be said about DISABLE_SETRANS > > It isn't set in both Makefiles. Pick one. Its not set in both, did you mean referenced/used? In fact I don't even set the default n, which I should be doing.
<snip> >>> Don't you actually want to also pick up utils/sefcontext_compile? >>> That is built and used on the build host. And I'm not sure why we'd >>> drop the other SUBDIRS. >> >> You'll start running into linking issues if things that use >> libselinux, use something not >> in the build host IIRC. Perhaps, if that is the issue, we just limit >> it to sefcontext_compile. > > Sure, the utils/Makefile can remove entries the same way it already does > for DISABLE_*. Well actually, that means every-time their is a new file added, the Makefile needs to be modified, I was trying to avoid that. Also, the remove list is super long, so it looks pretty unwieldy. <snip>
On 09/27/2016 03:09 PM, William Roberts wrote: > On Tue, Sep 27, 2016 at 12:08 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 09/27/2016 03:03 PM, William Roberts wrote: >>> On Tue, Sep 27, 2016 at 11:51 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >>>> On 09/27/2016 02:43 PM, William Roberts wrote: >>>>> On Sep 27, 2016 10:00, "Stephen Smalley" <sds@tycho.nsa.gov >>>>> <mailto:sds@tycho.nsa.gov>> wrote: >>>>>> >>>>>> On 09/27/2016 11:08 AM, William Roberts wrote: >>>>>>> On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <sds@tycho.nsa.gov >>>>> <mailto:sds@tycho.nsa.gov>> wrote: >>>>>>>> On 09/26/2016 04:53 PM, william.c.roberts@intel.com >>>>> <mailto:william.c.roberts@intel.com> wrote: >>>>>>>>> From: William Roberts <william.c.roberts@intel.com >>>>> <mailto:william.c.roberts@intel.com>> >>>>>>>>> >>>>>>>>> To build the selinux host configuration, specify >>>>>>>>> ANDROID_HOST=y on the Make command line. >>>>>>>>> >>>>>>>>> eg) >>>>>>>>> make ANDROID_HOST=y >>>>>>>> >>>>>>>> Seems oddly named, neither corresponding to the #define it enables >>>>>>>> (BUILD_HOST) nor to the target platform. >>>>>>> >>>>>>> We could change this to BUILD_HOST=y to enable all of it, but >>>>> considering >>>>>>> that this build is specific for Android, I thought the naming to be more >>>>>>> appropriate. >>>>>>> >>>>>>> Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well. >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Signed-off-by: William Roberts <william.c.roberts@intel.com >>>>> <mailto:william.c.roberts@intel.com>> >>>>>>>>> --- >>>>>>>>> libselinux/Makefile | 8 +++++++- >>>>>>>>> libselinux/src/Makefile | 50 >>>>> +++++++++++++++++++++++++++++++++---------------- >>>>>>>>> 2 files changed, 41 insertions(+), 17 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/libselinux/Makefile b/libselinux/Makefile >>>>>>>>> index 5a8d42c..50ae009 100644 >>>>>>>>> --- a/libselinux/Makefile >>>>>>>>> +++ b/libselinux/Makefile >>>>>>>>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) >>>>>>>>> override DISABLE_RPM=y >>>>>>>>> override DISABLE_BOOL=y >>>>>>>>> endif >>>>>>>>> +ifeq ($(ANDROID_HOST),y) >>>>>>>>> + override DISABLE_SETRANS=y >>>>>>>>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND >>>>> -DNO_X_BACKEND \ >>>>>>>>> + -DBUILD_HOST >>>>>>>>> + SUBDIRS = src >>>>>>>>> +endif >>>>>> >>>>>> Also, this is redundant; you can handle it entirely within >>>>>> libselinux/src/Makefile without anything here. >>>>> >>>>> You mean all the ANDROID _HOST stuff? I didn't want to depart from >>>>> what's there, that seemed to be the spot for disabling things. >>>> >>>> You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except >>>> in src/Makefile, so you don't need to set them here. >>>> >>> >>> The same could be said about DISABLE_SETRANS >> >> It isn't set in both Makefiles. Pick one. > > Its not set in both, did you mean referenced/used? In fact I don't > even set the default n, which I should be doing. You set -DNO_*_BACKEND and -DBUILD_HOST in both Makefiles if ANDROID_HOST=y.
On 09/27/2016 03:13 PM, William Roberts wrote: > <snip> >>>> Don't you actually want to also pick up utils/sefcontext_compile? >>>> That is built and used on the build host. And I'm not sure why we'd >>>> drop the other SUBDIRS. >>> >>> You'll start running into linking issues if things that use >>> libselinux, use something not >>> in the build host IIRC. Perhaps, if that is the issue, we just limit >>> it to sefcontext_compile. >> >> Sure, the utils/Makefile can remove entries the same way it already does >> for DISABLE_*. > > Well actually, that means every-time their is a new file added, the > Makefile needs to be modified, > I was trying to avoid that. Also, the remove list is super long, so it > looks pretty unwieldy. Yes, you can just override TARGETS instead.
diff --git a/libselinux/Makefile b/libselinux/Makefile index 5a8d42c..50ae009 100644 --- a/libselinux/Makefile +++ b/libselinux/Makefile @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y) override DISABLE_RPM=y override DISABLE_BOOL=y endif +ifeq ($(ANDROID_HOST),y) + override DISABLE_SETRANS=y + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ + -DBUILD_HOST + SUBDIRS = src +endif ifeq ($(DISABLE_AVC),y) EMFLAGS+= -DDISABLE_AVC endif @@ -22,7 +28,7 @@ endif ifeq ($(DISABLE_SETRANS),y) EMFLAGS+= -DDISABLE_SETRANS endif -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS ANDROID_HOST USE_PCRE2 ?= n ifeq ($(USE_PCRE2),y) diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index 36e42b8..d841ef7 100644 --- a/libselinux/src/Makefile +++ b/libselinux/src/Makefile @@ -47,9 +47,17 @@ endif ifeq ($(DISABLE_BOOL),y) UNUSED_SRCS+=booleans.c endif +ifeq ($(ANDROID_HOST),y) + SRCS=callbacks.c freecon.c label.c label_file.c \ + label_android_property.c regex.c label_support.c \ + matchpathcon.c setrans_client.c sha1.c + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \ + -DBUILD_HOST +else + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) +endif GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort $(wildcard *.c))) MAX_STACK_SIZE=32768 @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ $(EMFLAGS) SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS) +$(LIBA): $(OBJS) + $(AR) rcs $@ $^ + $(RANLIB) $@ + +$(LIBSO): $(LOBJS) + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro + ln -sf $@ $(TARGET) + +%.o: %.c policy.h + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< + +%.lo: %.c policy.h + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< + +# ANDROID_HOST Build option only builds the shared and static versions of +# libselinux. +ifeq ($(ANDROID_HOST),y) + +all: $(LIBA) $(LIBSO) + +else + all: $(LIBA) $(LIBSO) $(LIBPC) pywrap: all $(SWIGFILES) $(AUDIT2WHYSO) @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ) $(SWIGRUBYSO): $(SWIGRUBYLOBJ) $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR) -$(LIBA): $(OBJS) - $(AR) rcs $@ $^ - $(RANLIB) $@ - -$(LIBSO): $(LOBJS) - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro - ln -sf $@ $(TARGET) - $(LIBPC): $(LIBPC).in ../VERSION sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux $(LIBDIR)/libsepol.a -L$(LIBDIR) -%.o: %.c policy.h - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $< - -%.lo: %.c policy.h - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $< - $(SWIGCOUT): $(SWIGIF) $(SWIG) $< @@ -178,4 +194,6 @@ distclean: clean indent: ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean +.PHONY: clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean +endif +.PHONY: all