[PATCH v2] libselinux: add ANDROID_HOST=y build option

[PATCH v2] libselinux: add ANDROID_HOST=y build option

[william.c.roberts]

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 \
+		-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
-- 
1.9.1

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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>

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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?

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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 ;(

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.
> 

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

[-- Attachment #1: Type: text/plain, Size: 7623 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 11216 bytes --]

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 2931 bytes --]

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

<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>

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

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.