Re: [LTP] [PATCH] Don't install datafiles with executable mode

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 598 bytes --]

On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
> the makefiles install all files with executable mode by default. This patch
>  changes it for some of the Makefiles, that install data files, which
>  should IMHO not be executable. The change makes INSTALL_MODE decide the
>  actual mode when it is expanded inside the install rule (depending on $@,
>  which is the install target name).

that's pretty fugly way to go about it.  let's go the more natural route and 
have the common code default to $(INSTALL_MODE_$@) and if that's unset, use 
the default $(INSTALL_MODE).
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 300 bytes --]

------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing. 
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Jiří Paleček]

On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>  
wrote:

> On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
>> the makefiles install all files with executable mode by default. This  
>> patch
>>  changes it for some of the Makefiles, that install data files, which
>>  should IMHO not be executable. The change makes INSTALL_MODE decide the
>>  actual mode when it is expanded inside the install rule (depending on  
>> $@,
>>  which is the install target name).
>
> that's pretty fugly way to go about it.  let's go the more natural route  
> and
> have the common code default to $(INSTALL_MODE_$@) and if that's unset,  
> use
> the default $(INSTALL_MODE).

Sorry, but I don't agree with that. This is a function-like approach which  
is not any less "natural" than what you propose - and it makes no demands  
on anyone who doesn't need it, and gives great power to those who need it.

The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.  
INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot be  
any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate  
all data files - or all program files - which would be a chore esp. if any  
of these isn't known in the makefile until "make all" is ran.

OTOH I would agree on some method to automagically determine the correct  
mode.

   Jiri Palecek

------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing. 
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1742 bytes --]

On Friday 04 December 2009 20:19:13 Jiří Paleček wrote:
> On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>
> 
> wrote:
> > On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
> >> the makefiles install all files with executable mode by default. This
> >> patch
> >>  changes it for some of the Makefiles, that install data files, which
> >>  should IMHO not be executable. The change makes INSTALL_MODE decide the
> >>  actual mode when it is expanded inside the install rule (depending on
> >> $@,
> >>  which is the install target name).
> >
> > that's pretty fugly way to go about it.  let's go the more natural route
> > and
> > have the common code default to $(INSTALL_MODE_$@) and if that's unset,
> > use
> > the default $(INSTALL_MODE).
> 
> Sorry, but I don't agree with that. This is a function-like approach which
> is not any less "natural" than what you propose - and it makes no demands
> on anyone who doesn't need it, and gives great power to those who need it.

your solution requires non-obvious delayed evaluation that isnt 
straightforward.  mine is explicit, unambiguous, and already in use by many 
other projects that ltp is related to.

> The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.
> INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot be
> any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate
> all data files - or all program files - which would be a chore esp. if any
> of these isn't known in the makefile until "make all" is ran.

so fix install targets to expand the globs at make time and not shell time.  
i.e. something like INSTALL_TARGETS := $(wildcard $(INSTALL_TARGETS))
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 300 bytes --]

------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing. 
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Jiří Paleček]

On Sat, 05 Dec 2009 05:58:59 +0100, Mike Frysinger <vapier@gentoo.org>  
wrote:

> On Friday 04 December 2009 20:19:13 Jiří Paleček wrote:
>> On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>
>>
>> wrote:
>> > On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
>> >> the makefiles install all files with executable mode by default. This
>> >> patch
>> >>  changes it for some of the Makefiles, that install data files, which
>> >>  should IMHO not be executable. The change makes INSTALL_MODE decide  
>> the
>> >>  actual mode when it is expanded inside the install rule (depending  
>> on
>> >> $@,
>> >>  which is the install target name).
>> >
>> > that's pretty fugly way to go about it.  let's go the more natural  
>> route
>> > and
>> > have the common code default to $(INSTALL_MODE_$@) and if that's  
>> unset,
>> > use
>> > the default $(INSTALL_MODE).
>>
>> Sorry, but I don't agree with that. This is a function-like approach  
>> which
>> is not any less "natural" than what you propose - and it makes no  
>> demands
>> on anyone who doesn't need it, and gives great power to those who need  
>> it.
>
> your solution requires non-obvious delayed evaluation that isnt
> straightforward.  mine is explicit, unambiguous, and already in use by  
> many
> other projects that ltp is related to.

... and weak.

>> The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.
>> INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot  
>> be
>> any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate
>> all data files - or all program files - which would be a chore esp. if  
>> any
>> of these isn't known in the makefile until "make all" is ran.
>
> so fix install targets to expand the globs at make time and not shell  
> time.
> i.e. something like INSTALL_TARGETS := $(wildcard $(INSTALL_TARGETS))

We do that currently, but this doesn't change the problem in any way. The  
problem is that if I am able to write

INSTALL_TARGETS := 1* 2*

I want to be able to specify that the files in 1* are executables and  
files in 2* are not, without having to enumerate them all. This is  
impossible with your "explicit" solution.

I would be happy with something like

EXECUTABLES := 1*

   Jiri Palecek

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Garrett Cooper]

2009/12/18 Jiří Paleček <jpalecek@web.de>:
> On Sat, 05 Dec 2009 05:58:59 +0100, Mike Frysinger <vapier@gentoo.org>
> wrote:
>
>> On Friday 04 December 2009 20:19:13 Jiří Paleček wrote:
>>> On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>
>>>
>>> wrote:
>>> > On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
>>> >> the makefiles install all files with executable mode by default. This
>>> >> patch
>>> >>  changes it for some of the Makefiles, that install data files, which
>>> >>  should IMHO not be executable. The change makes INSTALL_MODE decide
>>> the
>>> >>  actual mode when it is expanded inside the install rule (depending
>>> on
>>> >> $@,
>>> >>  which is the install target name).
>>> >
>>> > that's pretty fugly way to go about it.  let's go the more natural
>>> route
>>> > and
>>> > have the common code default to $(INSTALL_MODE_$@) and if that's
>>> unset,
>>> > use
>>> > the default $(INSTALL_MODE).
>>>
>>> Sorry, but I don't agree with that. This is a function-like approach
>>> which
>>> is not any less "natural" than what you propose - and it makes no
>>> demands
>>> on anyone who doesn't need it, and gives great power to those who need
>>> it.
>>
>> your solution requires non-obvious delayed evaluation that isnt
>> straightforward.  mine is explicit, unambiguous, and already in use by
>> many
>> other projects that ltp is related to.
>
> ... and weak.
>
>>> The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.
>>> INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot
>>> be
>>> any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate
>>> all data files - or all program files - which would be a chore esp. if
>>> any
>>> of these isn't known in the makefile until "make all" is ran.
>>
>> so fix install targets to expand the globs at make time and not shell
>> time.
>> i.e. something like INSTALL_TARGETS := $(wildcard $(INSTALL_TARGETS))
>
> We do that currently, but this doesn't change the problem in any way. The
> problem is that if I am able to write
>
> INSTALL_TARGETS := 1* 2*
>
> I want to be able to specify that the files in 1* are executables and
> files in 2* are not, without having to enumerate them all. This is
> impossible with your "explicit" solution.
>
> I would be happy with something like
>
> EXECUTABLES := 1*

    I like that proposal a lot :). Maybe have an extension to the
install define that defaults to 00644 by default, but can be
overridden by an optional argument added to the define, e.g.

define generate_install_rule

$(1)INSTALL_MODE ?= 00664

ifneq ($(4),)
$(1)INSTALL_MODE := $(4)
endef

#...

endef

Some of the testcases that require CAP_CHOWN for seteuid need
different install bits than the rest. Or maybe a different method of
installation should be used here?
Thanks,
-Garrett

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Garrett Cooper]

On Fri, Dec 4, 2009 at 5:19 PM, Jiří Paleček <jpalecek@web.de> wrote:
> On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>
> wrote:
>
>> On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
>>> the makefiles install all files with executable mode by default. This
>>> patch
>>>  changes it for some of the Makefiles, that install data files, which
>>>  should IMHO not be executable. The change makes INSTALL_MODE decide the
>>>  actual mode when it is expanded inside the install rule (depending on
>>> $@,
>>>  which is the install target name).
>>
>> that's pretty fugly way to go about it.  let's go the more natural route
>> and
>> have the common code default to $(INSTALL_MODE_$@) and if that's unset,
>> use
>> the default $(INSTALL_MODE).
>
> Sorry, but I don't agree with that. This is a function-like approach which
> is not any less "natural" than what you propose - and it makes no demands
> on anyone who doesn't need it, and gives great power to those who need it.
>
> The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.
> INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot be
> any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate
> all data files - or all program files - which would be a chore esp. if any
> of these isn't known in the makefile until "make all" is ran.
>
> OTOH I would agree on some method to automagically determine the correct
> mode.

    We can do it one of a few ways...
    1. Anything not going into $(datarootdir), $(libdir), $(mandir),
etc (e.g. bin, testcases/bin, etc) should be 00664.
    2. We can filter by extension, e.g. INSTALL_TARGETS with
extensions like .py, .sh, etc can have 00755.
Thanks,
-Garrett

------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing. 
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Don't install datafiles with executable mode

[Jiří Paleček]

Hello,

I'm sorry it took me some time to get to this.

On Sat, 05 Dec 2009 07:39:10 +0100, Garrett Cooper <yanegomi@gmail.com>  
wrote:

> On Fri, Dec 4, 2009 at 5:19 PM, Jiří Paleček <jpalecek@web.de> wrote:
>> On Fri, 04 Dec 2009 20:06:40 +0100, Mike Frysinger <vapier@gentoo.org>
>> wrote:
>>
>>> On Friday 04 December 2009 11:58:13 Jiri Palecek wrote:
>>>> the makefiles install all files with executable mode by default. This
>>>> patch
>>>>  changes it for some of the Makefiles, that install data files, which
>>>>  should IMHO not be executable. The change makes INSTALL_MODE decide  
>>>> the
>>>>  actual mode when it is expanded inside the install rule (depending on
>>>> $@,
>>>>  which is the install target name).
>>>
>>> that's pretty fugly way to go about it.  let's go the more natural  
>>> route
>>> and
>>> have the common code default to $(INSTALL_MODE_$@) and if that's unset,
>>> use
>>> the default $(INSTALL_MODE).
>>
>> Sorry, but I don't agree with that. This is a function-like approach  
>> which
>> is not any less "natural" than what you propose - and it makes no  
>> demands
>> on anyone who doesn't need it, and gives great power to those who need  
>> it.
>>
>> The main reason I don't like the INSTALL_MODE_$@ thing, is that eg.
>> INSTALL_TARGETS can contain wildcards (eg. dir/*.ext), and there cannot  
>> be
>> any such thing as $(INSTALL_MODE_dir/*.ext). You would have to enumerate
>> all data files - or all program files - which would be a chore esp. if  
>> any
>> of these isn't known in the makefile until "make all" is ran.
>>
>> OTOH I would agree on some method to automagically determine the correct
>> mode.
>
>     We can do it one of a few ways...
>     1. Anything not going into $(datarootdir), $(libdir), $(mandir),
> etc (e.g. bin, testcases/bin, etc) should be 00664.

You meant 755? (ie. not $(mandir) etc. -> 755).

>     2. We can filter by extension, e.g. INSTALL_TARGETS with
> extensions like .py, .sh, etc can have 00755.

I think 1 is not satisfactory here, given that we install some datafiles  
in testcases/bin.

2 would be possible if it would work for most cases automatically and  
allowed a simple way of fixing corner cases.

I thought we could engage "file" command in determining what is what, but  
it may be really an overkill.

Regards
    Jiri Palecek

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH] Don't install datafiles with executable mode

[Jiri Palecek]

Hello,

the makefiles install all files with executable mode by default. This patch changes it for some of the Makefiles, that install data files, which should IMHO not be executable. The change makes INSTALL_MODE decide the actual mode when it is expanded inside the install rule (depending on $@, which is the install target name).

Regards
    Jiri Palecek

Signed-off-by: Jiri Palecek <jpalecek@web.de>
---
 testcases/commands/ade/ar/Makefile             |    2 ++
 testcases/commands/ade/ldd/Makefile            |    2 ++
 testcases/commands/ade/nm/Makefile             |    2 ++
 testcases/commands/cron/Makefile               |    2 ++
 testcases/kernel/io/stress_floppy/Makefile     |    1 +
 testcases/kernel/sched/clisrv/Makefile         |    2 ++
 testcases/kernel/sched/sched_stress/Makefile   |    2 ++
 testcases/network/multicast/mc_member/Makefile |    2 ++
 8 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/testcases/commands/ade/ar/Makefile b/testcases/commands/ade/ar/Makefile
index b6ab633..baf4b5b 100644
--- a/testcases/commands/ade/ar/Makefile
+++ b/testcases/commands/ade/ar/Makefile
@@ -33,4 +33,6 @@ MAKE_TARGETS		:= $(patsubst $(abs_srcdir)/%.c,%.obj,$(wildcard $(abs_srcdir)/*.c
 $(MAKE_TARGETS): %.obj: %.o
 	mv "$^" "$@"
 
+INSTALL_MODE = $(if $(filter %ar01,$@), 0755, 0644)
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/ade/ldd/Makefile b/testcases/commands/ade/ldd/Makefile
index eae06cd..ce5e64f 100644
--- a/testcases/commands/ade/ldd/Makefile
+++ b/testcases/commands/ade/ldd/Makefile
@@ -30,6 +30,8 @@ INSTALL_TARGETS		:= ldd01
 
 MAKE_TARGETS		:= $(patsubst $(abs_srcdir)/%.c,%.obj,$(wildcard $(abs_srcdir)/*.c))
 
+INSTALL_MODE = $(if $(filter %ldd01,$@), 0755, 0644)
+
 $(MAKE_TARGETS): %.obj: %.o
 	mv $^ $@
 
diff --git a/testcases/commands/ade/nm/Makefile b/testcases/commands/ade/nm/Makefile
index de8e3b4..ca03462 100644
--- a/testcases/commands/ade/nm/Makefile
+++ b/testcases/commands/ade/nm/Makefile
@@ -28,6 +28,8 @@ INSTALL_TARGETS		:= nm01 nmfile.c
 
 MAKE_TARGETS		:= $(patsubst $(abs_srcdir)/%.c,%.obj,$(wildcard $(abs_srcdir)/*.c))
 
+INSTALL_MODE = $(if $(filter %nm01,$@), 0755, 0644)
+
 $(MAKE_TARGETS): %.obj: %.o
 	mv $^ $@
 
diff --git a/testcases/commands/cron/Makefile b/testcases/commands/cron/Makefile
index 1b49488..53c0956 100644
--- a/testcases/commands/cron/Makefile
+++ b/testcases/commands/cron/Makefile
@@ -26,4 +26,6 @@ include $(top_srcdir)/include/mk/env_pre.mk
 
 INSTALL_TARGETS		:= cron0[23] cron_illegal_cron_lines cron_allow01 cron_deny01 cron_dirs_checks01 *.sh
 
+INSTALL_MODE = $(if $(filter %cron_illegal_cron_lines,$@), 0644, 0755)
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/io/stress_floppy/Makefile b/testcases/kernel/io/stress_floppy/Makefile
index 0a590b3..ad9f9b0 100644
--- a/testcases/kernel/io/stress_floppy/Makefile
+++ b/testcases/kernel/io/stress_floppy/Makefile
@@ -26,6 +26,7 @@ INSTALL_DEPS		:= dumpdir
 
 MAKE_TARGETS		:=
 
+
 # dumpdir's a directory.
 RM			+= -r
 
diff --git a/testcases/kernel/sched/clisrv/Makefile b/testcases/kernel/sched/clisrv/Makefile
index 0c293d3..ffe45ea 100644
--- a/testcases/kernel/sched/clisrv/Makefile
+++ b/testcases/kernel/sched/clisrv/Makefile
@@ -29,6 +29,8 @@ LDLIBS			+= -lpthread
 
 INSTALL_TARGETS		:= data run_sched_cliserv.sh
 
+INSTALL_MODE = $(if $(filter %data,$@), 0644, 0755)
+
 MAKE_TARGETS		:= pthcli pthserv
 
 $(MAKE_TARGETS): %: %.o readline.o writen.o
diff --git a/testcases/kernel/sched/sched_stress/Makefile b/testcases/kernel/sched/sched_stress/Makefile
index 140b630..a85a5e8 100644
--- a/testcases/kernel/sched/sched_stress/Makefile
+++ b/testcases/kernel/sched/sched_stress/Makefile
@@ -36,6 +36,8 @@ $(filter-out sched_datafile,$(MAKE_TARGETS)): %: %.o sched.o
 
 MAKE_TARGETS		+= sched_datafile
 
+INSTALL_MODE = $(if $(filter %sched_datafile,$@), 0644, 0755)
+
 sched_datafile:
 	"$(top_srcdir)/tools/make-file.sh" "$@" 1200010
 
diff --git a/testcases/network/multicast/mc_member/Makefile b/testcases/network/multicast/mc_member/Makefile
index afd1114..44aa5d4 100644
--- a/testcases/network/multicast/mc_member/Makefile
+++ b/testcases/network/multicast/mc_member/Makefile
@@ -26,4 +26,6 @@ include $(top_srcdir)/include/mk/env_pre.mk
 
 INSTALL_TARGETS		:= mc_member *ManyGroups
 
+INSTALL_MODE = $(if $(filter %mc_member,$@), 0755, 0644)
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
1.6.4.3




------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing. 
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list