Re: Rework of makefiles v5
On Wed, Feb 21, 2018 at 10:46:45PM +0100, Nicolas Iooss wrote: > On Thu, Feb 15, 2018 at 2:04 PM, Marcus Folkesson > wrote: > > Hi Nicolas, > > > > First of all, thank you for your review. > > > > On Wed, Feb 14, 2018 at 08:19:44PM +0100, Nicolas Iooss wrote: > >> On Wed, Feb 14, 2018 at 10:57 AM, Marcus Folkesson > >> wrote: > >> > I have updated the patchset. > >> > > >> > The biggest change is that $(DESTDIR) is now used in the > >> > install stage only. > >> > > >> > Also some overidden CFLAGS/LDFLAGS has been removed since we now have > >> > explicit build rules. > >> > > >> > I have moved the changelog into patches. > >> > > >> > Please test to compile with: > >> > make DESTDIR=/tmp/myroot PREFIX=/myusr install > >> > or > >> > make DESTDIR=/tmp/myroot install > >> > > >> > Thanks for all feedback. > >> > > >> > Best regards > >> > Marcus Folkesson > >> > > >> > >> Hi, > >> Thanks for this update! Here are three comments on this patchset: > >> > >> * you forgot a $(DESTDIR) occurrence in patch 7. > > > > Good catch! > > > >> * .travis.yml needs a simple patch to fix the value of PYSITEDIR. I > >> will send it later. > > > > Please do. > > > >> * While reading the Makefile after patch 15, I have been surprised by > >> "LIBDIR ?= $(DESTDIR)$(PREFIX)/lib", with DESTDIR. As this variable is > >> not exported, it works fine as it is, but it might be cleaner to > >> define it as "LIBDIR ?= $(PREFIX)/lib" and to use $(DESTDIR) in the > >> following lines. This point may be addressed in a follow-up commit > >> after the patchset has been merged. > > > > I agree. > > I also think it could be part of a follow-up commit. I will take a note. > > > >> > >> As the patchset looks almost ready to be merged, I have created > >> https://github.com/SELinuxProject/selinux/pull/79 with a modified > >> patch 7 and my patch for .travis.yml. This pull request holds the > >> commits I plan to apply in a few days if no other maintainer disagrees > >> with this. > > > > Ok, then I will not come up with a v6 that fixes your feedback for patch > > #7. > > I have merged this patchset. Thanks! Thank you. Do you know when the next release is planned? > > Nicolas > Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: Rework of makefiles v5
On Thu, Feb 15, 2018 at 2:04 PM, Marcus Folkesson wrote: > Hi Nicolas, > > First of all, thank you for your review. > > On Wed, Feb 14, 2018 at 08:19:44PM +0100, Nicolas Iooss wrote: >> On Wed, Feb 14, 2018 at 10:57 AM, Marcus Folkesson >> wrote: >> > I have updated the patchset. >> > >> > The biggest change is that $(DESTDIR) is now used in the >> > install stage only. >> > >> > Also some overidden CFLAGS/LDFLAGS has been removed since we now have >> > explicit build rules. >> > >> > I have moved the changelog into patches. >> > >> > Please test to compile with: >> > make DESTDIR=/tmp/myroot PREFIX=/myusr install >> > or >> > make DESTDIR=/tmp/myroot install >> > >> > Thanks for all feedback. >> > >> > Best regards >> > Marcus Folkesson >> > >> >> Hi, >> Thanks for this update! Here are three comments on this patchset: >> >> * you forgot a $(DESTDIR) occurrence in patch 7. > > Good catch! > >> * .travis.yml needs a simple patch to fix the value of PYSITEDIR. I >> will send it later. > > Please do. > >> * While reading the Makefile after patch 15, I have been surprised by >> "LIBDIR ?= $(DESTDIR)$(PREFIX)/lib", with DESTDIR. As this variable is >> not exported, it works fine as it is, but it might be cleaner to >> define it as "LIBDIR ?= $(PREFIX)/lib" and to use $(DESTDIR) in the >> following lines. This point may be addressed in a follow-up commit >> after the patchset has been merged. > > I agree. > I also think it could be part of a follow-up commit. I will take a note. > >> >> As the patchset looks almost ready to be merged, I have created >> https://github.com/SELinuxProject/selinux/pull/79 with a modified >> patch 7 and my patch for .travis.yml. This pull request holds the >> commits I plan to apply in a few days if no other maintainer disagrees >> with this. > > Ok, then I will not come up with a v6 that fixes your feedback for patch > #7. I have merged this patchset. Thanks! Nicolas
Re: Rework of makefiles v5
Hi Nicolas, First of all, thank you for your review. On Wed, Feb 14, 2018 at 08:19:44PM +0100, Nicolas Iooss wrote: > On Wed, Feb 14, 2018 at 10:57 AM, Marcus Folkesson > wrote: > > I have updated the patchset. > > > > The biggest change is that $(DESTDIR) is now used in the > > install stage only. > > > > Also some overidden CFLAGS/LDFLAGS has been removed since we now have > > explicit build rules. > > > > I have moved the changelog into patches. > > > > Please test to compile with: > > make DESTDIR=/tmp/myroot PREFIX=/myusr install > > or > > make DESTDIR=/tmp/myroot install > > > > Thanks for all feedback. > > > > Best regards > > Marcus Folkesson > > > > Hi, > Thanks for this update! Here are three comments on this patchset: > > * you forgot a $(DESTDIR) occurrence in patch 7. Good catch! > * .travis.yml needs a simple patch to fix the value of PYSITEDIR. I > will send it later. Please do. > * While reading the Makefile after patch 15, I have been surprised by > "LIBDIR ?= $(DESTDIR)$(PREFIX)/lib", with DESTDIR. As this variable is > not exported, it works fine as it is, but it might be cleaner to > define it as "LIBDIR ?= $(PREFIX)/lib" and to use $(DESTDIR) in the > following lines. This point may be addressed in a follow-up commit > after the patchset has been merged. I agree. I also think it could be part of a follow-up commit. I will take a note. > > As the patchset looks almost ready to be merged, I have created > https://github.com/SELinuxProject/selinux/pull/79 with a modified > patch 7 and my patch for .travis.yml. This pull request holds the > commits I plan to apply in a few days if no other maintainer disagrees > with this. Ok, then I will not come up with a v6 that fixes your feedback for patch #7. > > Best, > Nicolas > Best regards Marcus Folkesson signature.asc Description: PGP signature
[PATCH Rework of makefiles v5 16/15] Travis-CI: do not duplicate $DESTDIR in $PYSITEDIR
Recent commits removed $DESTDIR from $PYSITEDIR in libselinux and libsemanage: -PYSITEDIR ?= $(DESTDIR)$(shell $(PYTHON) -c 'import site; print(site.getsitepackages()[0])') +PYSITEDIR ?= $(shell $(PYTHON) -c 'import site; print(site.getsitepackages()[0])') As "site.getsitepackages()" does not work within virtualenvs, .travis.yml defines PYSITEDIR's value in it and this definition needs to be updated too. Signed-off-by: Nicolas Iooss --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 88f6297e63bc..0312e996e333 100644 --- a/.travis.yml +++ b/.travis.yml @@ -98,7 +98,7 @@ before_script: - if echo "$PYVER" | grep -q pypy ; then export PYINC=-I$($PYTHON -c 'import sys;print(sys.prefix)')/include PYLIBS= ; fi # Python virtualenvs do not support "import site; print(site.getsitepackages()[0]" # cf. https://github.com/pypa/virtualenv/issues/355#issuecomment-10250452 - - export PYSITEDIR="$DESTDIR/usr/lib/$($PYTHON -c 'import sys;print("python%d.%d" % sys.version_info[:2])')/site-packages" + - export PYSITEDIR="/usr/lib/$($PYTHON -c 'import sys;print("python%d.%d" % sys.version_info[:2])')/site-packages" # Find the Ruby executable with version $RUBYLIBVER - export RUBY="$(ls -d -1 "$HOME/.rvm/rubies/ruby-$RUBYLIBVER"*/bin/ruby | head -n 1)" @@ -126,7 +126,7 @@ script: # Set up environment variables for the tests - export LD_LIBRARY_PATH="$DESTDIR/usr/lib:$DESTDIR/lib" - export PATH="$DESTDIR/usr/sbin:$DESTDIR/usr/bin:$DESTDIR/sbin:$DESTDIR/bin:$PATH" - - export PYTHONPATH="$PYSITEDIR" + - export PYTHONPATH="$DESTDIR$PYSITEDIR" - export RUBYLIB="$DESTDIR/$($RUBY -e 'puts RbConfig::CONFIG["vendorlibdir"]'):$DESTDIR/$($RUBY -e 'puts RbConfig::CONFIG["vendorarchdir"]')" # Show variables (to help debugging issues) -- 2.16.0
Re: Rework of makefiles v5
On Wed, Feb 14, 2018 at 10:57 AM, Marcus Folkesson wrote: > I have updated the patchset. > > The biggest change is that $(DESTDIR) is now used in the > install stage only. > > Also some overidden CFLAGS/LDFLAGS has been removed since we now have > explicit build rules. > > I have moved the changelog into patches. > > Please test to compile with: > make DESTDIR=/tmp/myroot PREFIX=/myusr install > or > make DESTDIR=/tmp/myroot install > > Thanks for all feedback. > > Best regards > Marcus Folkesson > Hi, Thanks for this update! Here are three comments on this patchset: * you forgot a $(DESTDIR) occurrence in patch 7. * .travis.yml needs a simple patch to fix the value of PYSITEDIR. I will send it later. * While reading the Makefile after patch 15, I have been surprised by "LIBDIR ?= $(DESTDIR)$(PREFIX)/lib", with DESTDIR. As this variable is not exported, it works fine as it is, but it might be cleaner to define it as "LIBDIR ?= $(PREFIX)/lib" and to use $(DESTDIR) in the following lines. This point may be addressed in a follow-up commit after the patchset has been merged. As the patchset looks almost ready to be merged, I have created https://github.com/SELinuxProject/selinux/pull/79 with a modified patch 7 and my patch for .travis.yml. This pull request holds the commits I plan to apply in a few days if no other maintainer disagrees with this. Best, Nicolas
Rework of makefiles v5
I have updated the patchset. The biggest change is that $(DESTDIR) is now used in the install stage only. Also some overidden CFLAGS/LDFLAGS has been removed since we now have explicit build rules. I have moved the changelog into patches. Please test to compile with: make DESTDIR=/tmp/myroot PREFIX=/myusr install or make DESTDIR=/tmp/myroot install Thanks for all feedback. Best regards Marcus Folkesson