On Sat, 15 May 2010, Pavan Kumar Sunkara wrote:
> Here's the patch as an attachment.
>
> The file is not whitespace damaged. That is a problem with my mail client
>
> PFA the patch.
If you cannot use ordinary email client configured to send email via SMTPS
(ports 465 or 587), or via git-send-email, you should consider attaching
patches (perhaps in addition to having them inline) as file with *.txt
extension (to force to use 'text/plain' mimetype, 8bit, no transfer
encoding).
-- >8 --
> From: Pavan Kumar Sunkara <pavan.sss1991@xxxxxxxxx>
> Subject: [PATCH 4/5] gitweb: Move static files into seperate subdirectory
>
> Create a new subdirectory called 'static' in gitweb/, and move
> all static files required by gitweb.cgi when running, which means
> styles, images and Javascript code. This should make gitweb more
> readable and easier to maintain.
>
> Update t/gitweb-lib.sh to reflect this change. The install-gitweb
> now also include moving of static files into 'static' subdirectory
> in target directory: update Makefile, gitweb's INSTALL, README and
> Makefile accordingly.
>
> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@xxxxxxxxx>
Almost Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>
You need to use 'install -d' instead of 'mkdir -p' for modified 'install'
target of gitweb/Makefile.
> ---
Here you should mention that you base your patch on 'next', or even better
on which commit / which branch you base your changes on.
See this fragment of Documentation/SubmittingPatches:
If you are preparing a work based on "next" branch, that is fine, but
please mark it as such.
> Makefile | 20 ++++++++--------
> gitweb/INSTALL | 19 +++++++--------
> gitweb/Makefile | 41 ++++++++++++++++++----------------
> gitweb/README | 14 ++++++-----
> gitweb/{ => static}/git-favicon.png | Bin 115 -> 115 bytes
> gitweb/{ => static}/git-logo.png | Bin 207 -> 207 bytes
> gitweb/{ => static}/gitweb.css | 0
> gitweb/{ => static}/gitweb.js | 0
> t/gitweb-lib.sh | 6 ++--
> 9 files changed, 52 insertions(+), 48 deletions(-)
> rename gitweb/{ => static}/git-favicon.png (100%)
> rename gitweb/{ => static}/git-logo.png (100%)
> rename gitweb/{ => static}/gitweb.css (100%)
> rename gitweb/{ => static}/gitweb.js (100%)
[...]
> diff --git a/gitweb/INSTALL b/gitweb/INSTALL
> index d484d76..8230531 100644
> --- a/gitweb/INSTALL
> +++ b/gitweb/INSTALL
> @@ -2,9 +2,10 @@ GIT web Interface (gitweb) Installation
> =======================================
>
> First you have to generate gitweb.cgi from gitweb.perl using
> -"make gitweb", then copy appropriate files (gitweb.cgi, gitweb.js,
> -gitweb.css, git-logo.png and git-favicon.png) to their destination.
> -For example if git was (or is) installed with /usr prefix, you can do
> +"make gitweb", then "make install-gitweb" will copy appropriate files
> +(gitweb.cgi, gitweb.js, gitweb.css, git-logo.png and git-favicon.png)
> +to their destination. For example if git was (or is) installed with
> +/usr prefix and gitwebdir is /var/www/cgi-bin, you can do
>
> $ make prefix=/usr gitweb ;# as yourself
> # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root
Thanks for noticing of what I missed when updating gitweb/INSTALL in 152d943
(gitweb: Create install target for gitweb in Makefile, 2010-05-01)
> @@ -81,16 +82,14 @@ Build example
> minifiers, you can do
>
> make GITWEB_PROJECTROOT="/home/local/scm" \
> - GITWEB_JS="/gitweb/gitweb.js" \
> - GITWEB_CSS="/gitweb/gitweb.css" \
> - GITWEB_LOGO="/gitweb/git-logo.png" \
> - GITWEB_FAVICON="/gitweb/git-favicon.png" \
> + GITWEB_JS="gitweb/static/gitweb.js" \
> + GITWEB_CSS="gitweb/static/gitweb.css" \
> + GITWEB_LOGO="gitweb/static/git-logo.png" \
> + GITWEB_FAVICON="gitweb/static/git-favicon.png" \
> bindir=/usr/local/bin \
> gitweb
>
> - cp -fv gitweb/gitweb.{cgi,js,css} \
> - gitweb/git-{favicon,logo}.png \
> - /var/www/cgi-bin/gitweb/
> + make gitwebdir=/var/www/cgi-bin/gitweb install-gitweb
Here I am not sure of we should not leave an example how to copy files
manually... but I guess with this form we wouldn't have to update this part
if/when gitweb is split...
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> @@ -16,6 +16,7 @@ gitwebdir ?= /var/www/cgi-bin
>
> RM ?= rm -f
> INSTALL ?= install
> +MKDIR ?= mkdir
Is MKDIR really needed? The main Makefile doesn't use it. It is what
"$(INSTALL) -d ..." is for this (the '-d' / '--directory') option would
create each given directory and any missing parent directories).
> @@ -54,6 +55,7 @@ PERL_PATH ?= /usr/bin/perl
> # Shell quote;
> bindir_SQ = $(subst ','\'',$(bindir))#'
> gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
> +gitwebfile_SQ = $(subst ','\'',$(gitwebdir)/static)#'
> SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
> PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
> DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
I would name it gitwebstaticdir_SQ, but admittedly this is a matter of
taste. It can be named gitwebfile_SQ... although truth to be said it is
NOT strictly NECESSARY, as "$(gitwebdir_SQ)/static" would work as well.
> @@ -147,12 +149,13 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
> install: all
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> $(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> - $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> + $(MKDIR) -p '$(DESTDIR_SQ)$(gitwebfile_SQ)'
> + $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebfile_SQ)'
Use
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/static'
+ $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebdir_SQ)/static'
instead.
BTW. mkdir doesn't need to support '-p' option. See for example this part of
autoconf documentation:
- Macro: AS_MKDIR_P (FILENAME)
Make the directory FILENAME, including intervening directories as
necessary. This is equivalent to `mkdir -p FILENAME', except that
it is portable to older versions of `mkdir' that lack support for
the `-p' option.
> diff --git a/gitweb/README b/gitweb/README
> index 71742b3..5787260 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -80,24 +80,26 @@ You can specify the following configuration variables when building GIT:
> Points to the location where you put gitweb.css on your web server
> (or to be more generic, the URI of gitweb stylesheet). Relative to the
> base URI of gitweb. Note that you can setup multiple stylesheets from
> - the gitweb config file. [Default: gitweb.css (or gitweb.min.css if the
> - CSSMIN variable is defined / CSS minifier is used)]
> + the gitweb config file. [Default: static/gitweb.css (or
> + static/gitweb.min.css if the CSSMIN variable is defined / CSS minifier
> + is used)]
^----------------- stray space character?
> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
> index 5a734b1..b70b891 100644
> --- a/t/gitweb-lib.sh
> +++ b/t/gitweb-lib.sh
> @@ -19,9 +19,9 @@ our \$site_name = '[localhost]';
> our \$site_header = '';
> our \$site_footer = '';
> our \$home_text = 'indextext.html';
> -our @stylesheets = ('file:///$TEST_DIRECTORY/../gitweb/gitweb.css');
> -our \$logo = 'file:///$TEST_DIRECTORY/../gitweb/git-logo.png';
> -our \$favicon = 'file:///$TEST_DIRECTORY/../gitweb/git-favicon.png';
> +our @stylesheets = ('file:///$TEST_DIRECTORY/../gitweb/static/gitweb.css');
> +our \$logo = 'file:///$TEST_DIRECTORY/../gitweb/static/git-logo.png';
> +our \$favicon = 'file:///$TEST_DIRECTORY/../gitweb/static/git-favicon.png';
> our \$projects_list = '';
> our \$export_ok = '';
> our \$strict_export = '';
Thanks for updating that.
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html