On 22 November, Dave Barr <barr@math.psu.edu> wrote:
> One shouldn't assume that root can write to the build directory.
Good point.
Three other comments (the first two are nitpicks):
> install-wrapper: wrapper
> @$(INSTALL) -o $(WRAPPER_USER) -g $(W_GROUP) -m $(WRAPPER_CHMOD) wrappe
r $(W_BIN)/wrapper
> @echo ""
> @echo "To verify that all the permissions and etc are correct,"
> @echo "run the command"
> @echo ""
> @echo " cd $(W_HOME); ./wrapper config-test"
install-wrapper is run as root, so the person installing Majordomo may
run "./wrapper config-test" as root, too. wrapper should work the same
when invoked as root... but would it make sense to change the message
to tell people _not_ to do the test as root?
> config-scripts:
...deleted...
> @-for file in $(TOOLS); do \
> cp contrib/$$file tmp ; \
> done
That loop could be replaced with:
@cd contrib && cp $(TOOLS) ../tmp
couldn't it? Because each line of the Makefile is handled by a
different invocation of the shell, the "cd contrib" won't change the
current directory for lines below it.
Also, if the "@-for file" loop stays in the Makefile (if my change,
above, isn't used)... is the "-" really a good idea? If seems to me
that the Makefile should abort if one of the cp commands fails.
> install-scripts: config-scripts
> @test -d $(W_HOME) || (mkdir $(W_HOME); chmod 751 $(W_HOME); \
> chown $(W_USER) $(W_HOME); chgrp $(W_GROUP) $(W_HOME) )
> @test -d $(W_BIN)/bin || (mkdir $(W_BIN)/bin; \
> chown $(W_USER) $(W_BIN)/bin; chgrp $(W_GROUP) $(W_BIN)/bin )
...deleted...
> @test -d $(W_BIN)/Tools || (mkdir $(W_BIN)/Tools; \
> chown $(W_USER) $(W_BIN)/Tools; chgrp $(W_GROUP) $(W_BIN)/Tools
>
> install-man:
> @echo "Copying manual pages to $(MAN1) and $(MAN8)"
> @test -w $(MAN1) || (mkdir -p $(MAN1); \
> chown $(W_USER) $(MAN1); chgrp $(W_GROUP) $(MAN1) )
> @test -w $(MAN8) || (mkdir -p $(MAN8); \
> chown $(W_USER) $(MAN8); chgrp $(W_GROUP) $(MAN1) )
I haven't tested the new Makefile patch yet. (Sorry.) But it looks
like these chown's may be run by a non-root user? If that's true,
they will fail on systems that only let root do a chown. How about
replacing these with calls to $(INSTALL)?
--
Jerry Peek, jpeek@jpeek.com, http://www.jpeek.com/~jpeek/
My opinions may have changed, but not the fact that I am right. ;-)
Follow-Ups:
References:
|
|
