threads / discuss / 23279

[PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

Subject: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

## tl;dr

15 messages between Apr 1, 2010 and Apr 15, 2010.

replies: 14people: 4as markdown or json

Mark Rada· Apr 1, 2010, 05:36 UTC · lore

The build system added support minifying gitweb.js through a JavaScript minifier, but most minifiers come with support to minify CSS files as well, so we should use it if we can.

This patch will add the same facilities to gitweb.css that gitweb.js has for minification. That does not mean that they will use the same minifier though, as it is not safe to assume that all JavaScript minifiers will also minify CSS files.

Though the bandwidth savings will not be as dramatic as with the JavaScript minifier, every byte saved is important.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
Changes since v4:
	- Reworded some of the comments and documentation
 Makefile        |   21 +++++++++++++++------
 gitweb/INSTALL  |    5 +++++
 gitweb/Makefile |   28 +++++++++++++++++++++-------
 gitweb/README   |    3 ++-
 4 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile
index 5384d33..450e4df 100644
--- a/Makefile
+++ b/Makefile
@@ -203,6 +203,9 @@ all::
 # Define JSMIN to point to JavaScript minifier that functions as
 # a filter to have gitweb.js minified.
 #
+# Define CSSMIN to point to a CSS minifier in order to generate a minified
+# version of gitweb.css
+#
 # Define DEFAULT_PAGER to a sensible pager command (defaults to "less") if
 # you want to use something different.  The value will be interpreted by the
 # shell at runtime when it is used.
@@ -279,8 +282,9 @@ lib = lib
 # DESTDIR=
 pathsep = :
 
-# JavaScript minifier invocation that can function as filter
+# JavaScript/CSS minifier invocation that can function as filter
 JSMIN =
+CSSMIN =
 
 export prefix bindir sharedir sysconfdir
 
@@ -1564,18 +1568,23 @@ gitweb:
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
 
 ifdef JSMIN
-OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
-gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
-else
-OTHER_PROGRAMS += gitweb/gitweb.cgi
-gitweb/gitweb.cgi: gitweb/gitweb.perl
+GITWEB_PROGRAMS += gitweb/gitweb.min.js
 endif
+ifdef CSSMIN
+GITWEB_PROGRAMS += gitweb/gitweb.min.css
+endif
+OTHER_PROGRAMS +=  gitweb/gitweb.cgi  $(GITWEB_PROGRAMS)
+gitweb/gitweb.cgi: gitweb/gitweb.perl $(GITWEB_PROGRAMS)
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
 
 ifdef JSMIN
 gitweb/gitweb.min.js: gitweb/gitweb.js
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
 endif # JSMIN
+ifdef CSSMIN
+gitweb/gitweb.min.css: gitweb/gitweb.css
+	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
+endif # CSSMIN
 
 
 git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/gitweb.js
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index b76a0cf..b75a90b 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -66,6 +66,11 @@ file for gitweb (in gitweb/README).
   build configuration variables. By default gitweb tries to find them
   in the same directory as gitweb.cgi script.
 
+- You can optionally generate a minified version of gitweb.css by defining
+  the CSSMIN build configuration variable. By default the non-minified
+  version of gitweb.css will be used. NOTE: if you enable this option,
+  substitute gitweb.min.css for all uses of gitweb.css in the help files.
+
 Build example
 ~~~~~~~~~~~~~
 
diff --git a/gitweb/Makefile b/gitweb/Makefile
index c9eb1ee..fffe700 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -6,13 +6,17 @@ all::
 # Define JSMIN to point to JavaScript minifier that functions as
 # a filter to have gitweb.js minified.
 #
+# Define CSSMIN to point to a CSS minifier in order to generate a minified
+# version of gitweb.css
+#
 
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
 RM ?= rm -f
 
-# JavaScript minifier invocation that can function as filter
+# JavaScript/CSS minifier invocation that can function as filter
 JSMIN ?=
+CSSMIN ?=
 
 # default configuration for gitweb
 GITWEB_CONFIG = gitweb_config.perl
@@ -26,7 +30,11 @@ GITWEB_STRICT_EXPORT =
 GITWEB_BASE_URL =
 GITWEB_LIST =
 GITWEB_HOMETEXT = indextext.html
+ifdef CSSMIN
+GITWEB_CSS = gitweb.min.css
+else
 GITWEB_CSS = gitweb.css
+endif
 GITWEB_LOGO = git-logo.png
 GITWEB_FAVICON = git-favicon.png
 ifdef JSMIN
@@ -84,13 +92,14 @@ endif
 
 all:: gitweb.cgi
 
+FILES = gitweb.cgi
 ifdef JSMIN
-FILES=gitweb.cgi gitweb.min.js
-gitweb.cgi: gitweb.perl gitweb.min.js
-else # !JSMIN
-FILES=gitweb.cgi
-gitweb.cgi: gitweb.perl
-endif # JSMIN
+FILES += gitweb.min.js
+endif
+ifdef CSSMIN
+FILES += gitweb.min.css
+endif
+gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
 
 gitweb.cgi:
 	$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -123,6 +132,11 @@ gitweb.min.js: gitweb.js
 	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif # JSMIN
 
+ifdef CSSMIN
+gitweb.min.css: gitweb.css
+	$(QUIET_GEN)$(CSSMIN) <$ >$@
+endif
+
 clean:
 	$(RM) $(FILES)
 
diff --git a/gitweb/README b/gitweb/README
index ad6a04c..71742b3 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -80,7 +80,8 @@ 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]
+   the gitweb config file.  [Default: gitweb.css (or gitweb.min.css if the
+   CSSMIN variable is defined / CSS minifier is used)]
  * GITWEB_LOGO
    Points to the location where you put git-logo.png on your web server
    (or to be more generic URI of logo, 72x27 size, displayed in top right
-- 
1.7.0.3.519.g7e0613
Jakub Narebski· Apr 1, 2010, 08:51 UTC · re: Mark Rada · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Thu, 1 April 2010, Mark Rada wrote:
Show 11 quoted lines
> The build system added support minifying gitweb.js through a
> JavaScript minifier, but most minifiers come with support to
> minify CSS files as well, so we should use it if we can.
> 
> This patch will add the same facilities to gitweb.css that
> gitweb.js has for minification. That does not mean that they
> will use the same minifier though, as it is not safe to assume
> that all JavaScript minifiers will also minify CSS files.
> 
> Though the bandwidth savings will not be as dramatic as with
> the JavaScript minifier, every byte saved is important.

That's not everything this patch does, isn't it? I think you should have added to your commit message the following paragraph:

  While at it, introduce GITWEB_PROGRAMS variable to Makefile and
  use it for gitweb instead of adding to OTHER_PROGRAMS, to keep
  (possible) gitweb dependencies separate.
Or something like that.
> 
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
I think it is good idea.
[...]
Show 14 quoted lines
>  ifdef JSMIN
> -OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
> -gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
> -else
> -OTHER_PROGRAMS += gitweb/gitweb.cgi
> -gitweb/gitweb.cgi: gitweb/gitweb.perl
> +GITWEB_PROGRAMS += gitweb/gitweb.min.js
>  endif
> +ifdef CSSMIN
> +GITWEB_PROGRAMS += gitweb/gitweb.min.css
> +endif
> +OTHER_PROGRAMS +=  gitweb/gitweb.cgi  $(GITWEB_PROGRAMS)
> +gitweb/gitweb.cgi: gitweb/gitweb.perl $(GITWEB_PROGRAMS)
>  	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
-- 
Jakub Narebski
Poland
Charles Bailey· Apr 13, 2010, 20:28 UTC · re: Mark Rada · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On 01/04/2010 06:36, Mark Rada wrote:
Show 19 quoted lines
> @@ -84,13 +92,14 @@ endif
>
>   all:: gitweb.cgi
>
> +FILES = gitweb.cgi
>   ifdef JSMIN
> -FILES=gitweb.cgi gitweb.min.js
> -gitweb.cgi: gitweb.perl gitweb.min.js
> -else # !JSMIN
> -FILES=gitweb.cgi
> -gitweb.cgi: gitweb.perl
> -endif # JSMIN
> +FILES += gitweb.min.js
> +endif
> +ifdef CSSMIN
> +FILES += gitweb.min.css
> +endif
> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>

I have a question about this last line of the patch. Are GITWEB_JS and GITWEB_CSS supposed to be a source path or a URI?

The documentation for install (and my previous assumption) was that they represented the path on the target web server. I'm used to overriding them so that gitweb.cgi can live in my /cgi-bin directory, but the static files are served from /gitweb which is readable but not executable.

After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from the list of dependencies for gitweb.cgi otherwise make failed.

Have I got the wrong end of the stick?
Thanks,
Charles.
Jakub Narebski· Apr 13, 2010, 22:30 UTC · re: Charles Bailey · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Tue, 13 April 2010, Charles Bailey wrote:
Show 33 quoted lines
> On 01/04/2010 06:36, Mark Rada wrote:
> > @@ -84,13 +92,14 @@ endif
> >
> >   all:: gitweb.cgi
> >
> > +FILES = gitweb.cgi
> >   ifdef JSMIN
> > -FILES=gitweb.cgi gitweb.min.js
> > -gitweb.cgi: gitweb.perl gitweb.min.js
> > -else # !JSMIN
> > -FILES=gitweb.cgi
> > -gitweb.cgi: gitweb.perl
> > -endif # JSMIN
> > +FILES += gitweb.min.js
> > +endif
> > +ifdef CSSMIN
> > +FILES += gitweb.min.css
> > +endif
> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >
> 
> I have a question about this last line of the patch. Are GITWEB_JS and 
> GITWEB_CSS supposed to be a source path or a URI?
> 
> The documentation for install (and my previous assumption) was that they 
> represented the path on the target web server. I'm used to overriding 
> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
> static files are served from /gitweb which is readable but not executable.
> 
> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
> the list of dependencies for gitweb.cgi otherwise make failed.
> 
> Have I got the wrong end of the stick?
Thanks a lot for noticing this bug.

GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with gitweb JavaScript code and default gitweb stylesheet,... but during work on minification of JavaScript code and CSS file it somehow got confused to mean source path.

If I remember correctly the original patch, before adding required support for minified gitweb.js and gitweb.css to git-instaweb script, and before support for CSS minification had

   ifdef JSMIN
   gitweb.cgi: gitweb.perl gitweb.min.js
   else
   gitweb.cgi: gitweb.perl
   endif
which should probably be replaced in current situation by
   ifdef JSMIN
   gitweb.cgi : gitweb.min.js
   endif
   ifdef CSSMIN
   gitweb.cgi : gitweb.min.css
   endif
just adding prerequisites to gitweb.css target in gitweb/Makefile

I guess that support for adding minifiction support to git-instaweb would need to be more complicated. Perhaps

  $(notdir $(GITWEB_JS))   # Makefile function
or
  $(basename $GITWEB_JS)   # shell command
But I guess that it wouldn't work for all cases...
-- 
Jakub Narebski
Poland
Mark Rada· Apr 14, 2010, 05:40 UTC · re: Jakub Narebski · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On 10-04-13 6:30 PM, Jakub Narebski wrote:
Show 76 quoted lines
> On Tue, 13 April 2010, Charles Bailey wrote:
>> On 01/04/2010 06:36, Mark Rada wrote:
>>> @@ -84,13 +92,14 @@ endif
>>>
>>>   all:: gitweb.cgi
>>>
>>> +FILES = gitweb.cgi
>>>   ifdef JSMIN
>>> -FILES=gitweb.cgi gitweb.min.js
>>> -gitweb.cgi: gitweb.perl gitweb.min.js
>>> -else # !JSMIN
>>> -FILES=gitweb.cgi
>>> -gitweb.cgi: gitweb.perl
>>> -endif # JSMIN
>>> +FILES += gitweb.min.js
>>> +endif
>>> +ifdef CSSMIN
>>> +FILES += gitweb.min.css
>>> +endif
>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>
>>
>> I have a question about this last line of the patch. Are GITWEB_JS and 
>> GITWEB_CSS supposed to be a source path or a URI?
>>
>> The documentation for install (and my previous assumption) was that they 
>> represented the path on the target web server. I'm used to overriding 
>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>> static files are served from /gitweb which is readable but not executable.
>>
>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>> the list of dependencies for gitweb.cgi otherwise make failed.
>>
>> Have I got the wrong end of the stick?
> 
> Thanks a lot for noticing this bug.
> 
> 
> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> gitweb JavaScript code and default gitweb stylesheet,... but during work
> on minification of JavaScript code and CSS file it somehow got confused
> to mean source path.
> 
> If I remember correctly the original patch, before adding required
> support for minified gitweb.js and gitweb.css to git-instaweb script,
> and before support for CSS minification had
> 
>    ifdef JSMIN
>    gitweb.cgi: gitweb.perl gitweb.min.js
>    else
>    gitweb.cgi: gitweb.perl
>    endif
> 
> which should probably be replaced in current situation by
> 
>    ifdef JSMIN
>    gitweb.cgi : gitweb.min.js
>    endif
>    ifdef CSSMIN
>    gitweb.cgi : gitweb.min.css
>    endif
> 
> just adding prerequisites to gitweb.css target in gitweb/Makefile
> 
> 
> I guess that support for adding minifiction support to git-instaweb
> would need to be more complicated.  Perhaps
> 
>   $(notdir $(GITWEB_JS))   # Makefile function
> 
> or
> 
>   $(basename $GITWEB_JS)   # shell command
> 
> But I guess that it wouldn't work for all cases...
>

Aw, frig, never thought of using gitweb like that so I made some assumptions to make things cleaner looking.

I think this can be fixed by just using different variable names? Or perhaps some nested ifdef's? I'm not sure which will be better.

I wasn't at the computer today so I'm just getting to it now, I'll try to have something when in the next day, going to bed now. Good night.

-- 
Mark Rada
Jakub Narebski· Apr 14, 2010, 17:22 UTC · re: Mark Rada · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Wed, 14 Apr 2010, Mark Rada wrote:
> On 10-04-13 6:30 PM, Jakub Narebski wrote:
>> On Tue, 13 April 2010, Charles Bailey wrote:
>>> On 01/04/2010 06:36, Mark Rada wrote:
[...]
Show 57 quoted lines
>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>>
>>>
>>> I have a question about this last line of the patch. Are GITWEB_JS and 
>>> GITWEB_CSS supposed to be a source path or a URI?
>>>
>>> The documentation for install (and my previous assumption) was that they 
>>> represented the path on the target web server. I'm used to overriding 
>>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>>> static files are served from /gitweb which is readable but not executable.
>>>
>>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>>> the list of dependencies for gitweb.cgi otherwise make failed.
>>>
>>> Have I got the wrong end of the stick?
>> 
>> Thanks a lot for noticing this bug.
>> 
>> 
>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>> on minification of JavaScript code and CSS file it somehow got confused
>> to mean source path.
>> 
>> If I remember correctly the original patch, before adding required
>> support for minified gitweb.js and gitweb.css to git-instaweb script,
>> and before support for CSS minification had
>> 
>>    ifdef JSMIN
>>    gitweb.cgi: gitweb.perl gitweb.min.js
>>    else
>>    gitweb.cgi: gitweb.perl
>>    endif
>> 
>> which should probably be replaced in current situation by
>> 
>>    ifdef JSMIN
>>    gitweb.cgi : gitweb.min.js
>>    endif
>>    ifdef CSSMIN
>>    gitweb.cgi : gitweb.min.css
>>    endif
>> 
>> just adding prerequisites to gitweb.css target in gitweb/Makefile
>> 
>> 
>> I guess that support for adding minifiction support to git-instaweb
>> would need to be more complicated. [...]
> 
> Aw, frig, never thought of using gitweb like that so I made some
> assumptions to make things cleaner looking.
> 
> I think this can be fixed by just using different variable names? Or
> perhaps some nested ifdef's? I'm not sure which will be better.
> 
> I wasn't at the computer today so I'm just getting to it now, I'll try
> to have something when in the next day, going to bed now. Good night.

I think the best solution for prerequisites would be to have multiple target-only (without commands) rules, which according to make documentation would get concatenated. This means the following code in gitweb/Makefile:

    ifdef JSMIN
    gitweb.cgi : gitweb.min.js
    endif
    ifdef CSSMIN
    gitweb.cgi : gitweb.min.css
    endif
in place of
   gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)

For git-instaweb I think that best solution would be to introduce new variables holding _source_ of gitweb JavaScript code and CSS, e.g.

            -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
in place of
            -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \

...although GITWEB_CSS might mean something different for Makefile and git-instaweb than for gitweb/Makefile and gitweb itself.

-- 
Jakub Narebski
Poland
Mark Rada· Apr 14, 2010, 19:17 UTC · re: Jakub Narebski · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On 10-04-14 1:22 PM, Jakub Narebski wrote:
Show 80 quoted lines
> On Wed, 14 Apr 2010, Mark Rada wrote:
>> On 10-04-13 6:30 PM, Jakub Narebski wrote:
>>> On Tue, 13 April 2010, Charles Bailey wrote:
>>>> On 01/04/2010 06:36, Mark Rada wrote:
> 
> [...]
>>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>>>
>>>>
>>>> I have a question about this last line of the patch. Are GITWEB_JS and 
>>>> GITWEB_CSS supposed to be a source path or a URI?
>>>>
>>>> The documentation for install (and my previous assumption) was that they 
>>>> represented the path on the target web server. I'm used to overriding 
>>>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>>>> static files are served from /gitweb which is readable but not executable.
>>>>
>>>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>>>> the list of dependencies for gitweb.cgi otherwise make failed.
>>>>
>>>> Have I got the wrong end of the stick?
>>>
>>> Thanks a lot for noticing this bug.
>>>
>>>
>>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>>> on minification of JavaScript code and CSS file it somehow got confused
>>> to mean source path.
>>>
>>> If I remember correctly the original patch, before adding required
>>> support for minified gitweb.js and gitweb.css to git-instaweb script,
>>> and before support for CSS minification had
>>>
>>>    ifdef JSMIN
>>>    gitweb.cgi: gitweb.perl gitweb.min.js
>>>    else
>>>    gitweb.cgi: gitweb.perl
>>>    endif
>>>
>>> which should probably be replaced in current situation by
>>>
>>>    ifdef JSMIN
>>>    gitweb.cgi : gitweb.min.js
>>>    endif
>>>    ifdef CSSMIN
>>>    gitweb.cgi : gitweb.min.css
>>>    endif
>>>
>>> just adding prerequisites to gitweb.css target in gitweb/Makefile
>>>
>>>
>>> I guess that support for adding minifiction support to git-instaweb
>>> would need to be more complicated. [...]
>>
>> Aw, frig, never thought of using gitweb like that so I made some
>> assumptions to make things cleaner looking.
>>
>> I think this can be fixed by just using different variable names? Or
>> perhaps some nested ifdef's? I'm not sure which will be better.
>>
>> I wasn't at the computer today so I'm just getting to it now, I'll try
>> to have something when in the next day, going to bed now. Good night.
> 
> I think the best solution for prerequisites would be to have multiple 
> target-only (without commands) rules, which according to make 
> documentation would get concatenated.  This means the following code
> in gitweb/Makefile:
> 
>     ifdef JSMIN
>     gitweb.cgi : gitweb.min.js
>     endif
>     ifdef CSSMIN
>     gitweb.cgi : gitweb.min.css
>     endif
> 
> in place of
> 
>    gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> 

Hmm, I like this because it is clear (if you know that dependancies can be joined like that), I was thinking of trying to make a smaller fix using pathsubst or subst, but that seems to not be as simple as I wanted it to be.

Show 11 quoted lines
> For git-instaweb I think that best solution would be to introduce new
> variables holding _source_ of gitweb JavaScript code and CSS, e.g.
> 
>             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
> 
> in place of
> 
>             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
> 
> ...although GITWEB_CSS might mean something different for Makefile
> and git-instaweb than for gitweb/Makefile and gitweb itself.

Did you get those lines mixed up? I might be not understanding something here.

I was actually planning something along the lines of 
             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_NAME)' \
             -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS_NAME)|' \

where I introduce the GITWEB_CSS_NAME variable, to be consistent with the token in instaweb. This way we don't touch GITWEB_JS in the top level makefile.

Also, I should update dependancies for instaweb, since those were forgotten last time around. Just creating a short list of what the fix will need for when I get home tonight.

-- 
Mark Rada
Jakub Narebski· Apr 14, 2010, 20:04 UTC · re: Mark Rada · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Wed, 14 April 2010, Mark Rada wrote:
> On 10-04-14 1:22 PM, Jakub Narebski wrote:
Show 14 quoted lines
> > For git-instaweb I think that best solution would be to introduce new
> > variables holding _source_ of gitweb JavaScript code and CSS, e.g.
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
> > 
> > in place of
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
> > 
> > ...although GITWEB_CSS might mean something different for Makefile
> > and git-instaweb than for gitweb/Makefile and gitweb itself.
> 
> Did you get those lines mixed up? I might be not understanding something
> here.

Ah, I'm sorry, I mixed up those two lines. They should be in reverse direction:

             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
 
  in place of
 
             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \

Actually I'd like to rename @@GITWEB_CSS@@ placeholder etc. in git-instaweb.sh, as @@GITWEB_CSS@@ in git-instaweb.sh means something quite different from ++GITWEB_CSS++ in gitweb/gitweb.perl...

Show 9 quoted lines
> 
> I was actually planning something along the lines of 
> 
>              -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_NAME)' \
>              -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS_NAME)|' \
> 
> where I introduce the GITWEB_CSS_NAME variable, to be consistent with the
> token in instaweb. This way we don't touch GITWEB_JS in the top level
> makefile.
Why not:
            -e '/@@GITWEB_CSS_SOURCE@@/r $(GITWEB_CSS_SOURCE)' \
            -e '/@@GITWEB_CSS_SOURCE@@/d' \
            ...
            -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS)|' \

(assuming that $(GITWEB_CSS) does not include '|' in it, I guess... but see below).

> 
> Also, I should update dependancies for instaweb, since those were
> forgotten last time around. Just creating a short list of what the fix will
> need for when I get home tonight.
Something like
   git-instaweb: git-instaweb.sh gitweb/gitweb.cgi $(GITWEB_CSS_SOURCE) $(GITWEB_JS_SOURCE)

P.S. I have noticed additional complication: git-instaweb really needs gitweb compiled with *specific* values of GITWEB_CSS and GITWEB_JS, so that they point to git-instaweb's installed files.

-- 
Jakub Narebski
Poland
Junio C Hamano· Apr 14, 2010, 23:58 UTC · re: Jakub Narebski · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

Jakub Narebski <jnareb@gmail.com> writes:
Show 19 quoted lines
> On Tue, 13 April 2010, Charles Bailey wrote:
>> On 01/04/2010 06:36, Mark Rada wrote:
>> > @@ -84,13 +92,14 @@ endif
>> >
>> >   all:: gitweb.cgi
>> >
>> > +FILES = gitweb.cgi
>> >   ifdef JSMIN
>> > +FILES += gitweb.min.js
>> > +endif
>> > +ifdef CSSMIN
>> > +FILES += gitweb.min.css
>> > +endif
>> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>
> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> gitweb JavaScript code and default gitweb stylesheet,... but during work
> on minification of JavaScript code and CSS file it somehow got confused
> to mean source path.

I am not touching instaweb part, but this would fix the build/clean side of the things, no?

-- >8 -- gitweb: simplify gitweb.min.* generation and clean-up rules

GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed cgi script use to refer to the stylesheet and JavaScript", never "this is the name of the file we are building".

Lose incorrect assignment to them.

While we are at it, lose FILES that is used only for "clean" target in a misguided way. "make clean" should try to remove all the potential build artifacts regardless of a minor configuration change. Instead of trying to remove only the build product "make clean" would have created if it were run without "clean", explicitly list the three potential build products for removal.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 gitweb/Makefile |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index ffee4bd..1787633 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -80,16 +80,7 @@ endif
 
 all:: gitweb.cgi
 
-FILES = gitweb.cgi
-ifdef JSMIN
-FILES += gitweb.min.js
-GITWEB_JS = gitweb.min.js
-endif
-ifdef CSSMIN
-FILES += gitweb.min.css
-GITWEB_CSS = gitweb.min.css
-endif
-gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
+gitweb.cgi: gitweb.perl
 
 gitweb.cgi:
 	$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -118,16 +109,18 @@ gitweb.cgi:
 	mv $@+ $@
 
 ifdef JSMIN
+all:: gitweb.min.js
 gitweb.min.js: gitweb.js
 	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif # JSMIN
 
 ifdef CSSMIN
+all:: gitweb.min.css
 gitweb.min.css: gitweb.css
 	$(QUIET_GEN)$(CSSMIN) <$ >$@
 endif
 
 clean:
-	$(RM) $(FILES)
+	$(RM) gitweb.cgi gitweb.min.css gitweb.min.js
 
 .PHONY: all clean .FORCE-GIT-VERSION-FILE
Charles Bailey· Apr 15, 2010, 00:18 UTC · re: Junio C Hamano · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Wed, Apr 14, 2010 at 04:58:12PM -0700, Junio C Hamano wrote:
Show 25 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > On Tue, 13 April 2010, Charles Bailey wrote:
> >> On 01/04/2010 06:36, Mark Rada wrote:
> >> > @@ -84,13 +92,14 @@ endif
> >> >
> >> >   all:: gitweb.cgi
> >> >
> >> > +FILES = gitweb.cgi
> >> >   ifdef JSMIN
> >> > +FILES += gitweb.min.js
> >> > +endif
> >> > +ifdef CSSMIN
> >> > +FILES += gitweb.min.css
> >> > +endif
> >> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >
> > GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> > gitweb JavaScript code and default gitweb stylesheet,... but during work
> > on minification of JavaScript code and CSS file it somehow got confused
> > to mean source path.
> 
> I am not touching instaweb part, but this would fix the build/clean side
> of the things, no?
> 

Yes, this certainly fixes make and make clean for me with overridden GITWEB_JS and GITWEB_CSS paths.

Charles.
Jakub Narebski· Apr 15, 2010, 00:25 UTC · re: Junio C Hamano · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On Thu, 15 Apr 2010, Junio C Hamano wrote:
Show 23 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Tue, 13 April 2010, Charles Bailey wrote:
>>> On 01/04/2010 06:36, Mark Rada wrote:
>>>> @@ -84,13 +92,14 @@ endif
>>>>
>>>>   all:: gitweb.cgi
>>>>
>>>> +FILES = gitweb.cgi
>>>>   ifdef JSMIN
>>>> +FILES += gitweb.min.js
>>>> +endif
>>>> +ifdef CSSMIN
>>>> +FILES += gitweb.min.css
>>>> +endif
>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>
>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>> on minification of JavaScript code and CSS file it somehow got confused
>> to mean source path.
> 
> I am not touching instaweb part, but this would fix the build/clean side
> of the things, no?
Close, see below.
Show 9 quoted lines
> 
> -->8 --
> gitweb: simplify gitweb.min.* generation and clean-up rules
> 
> GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed
> cgi script use to refer to the stylesheet and JavaScript", never "this is
> the name of the file we are building".
> 
> Lose incorrect assignment to them.

Actually the assignment was intended to provide correct *default* values for GITWEB_CSS and GITWEB_JS, so that if e.g. JSMIN is defined gitweb, in absence of build time configuration, would link gitweb.min.js instead of gitweb.js.

Show 7 quoted lines
> 
> While we are at it, lose FILES that is used only for "clean" target in a
> misguided way.  "make clean" should try to remove all the potential build
> artifacts regardless of a minor configuration change.  Instead of trying
> to remove only the build product "make clean" would have created if it
> were run without "clean", explicitly list the three potential build
> products for removal.
Good.
Show 23 quoted lines
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  gitweb/Makefile |   15 ++++-----------
>  1 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index ffee4bd..1787633 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -80,16 +80,7 @@ endif
>  
>  all:: gitweb.cgi
>  
> -FILES = gitweb.cgi
> -ifdef JSMIN
> -FILES += gitweb.min.js
> -GITWEB_JS = gitweb.min.js
> -endif
> -ifdef CSSMIN
> -FILES += gitweb.min.css
> -GITWEB_CSS = gitweb.min.css
> -endif

I wonder about removing assigmnet to GITWEB_JS and GITWEB_CSS. Without it "make gitweb" (in top dir) would create gitweb/gitweb.cgi and gitweb/gitweb.min.js etc.... but generated gitweb/gitweb.cgi would refer to gitweb.js, not gitweb.min.js. Unless of course one provides values for GITWEB_JS during build time.

Show 19 quoted lines
> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> +gitweb.cgi: gitweb.perl
>  
>  gitweb.cgi:
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
> @@ -118,16 +109,18 @@ gitweb.cgi:
>  	mv $@+ $@
>  
>  ifdef JSMIN
> +all:: gitweb.min.js
>  gitweb.min.js: gitweb.js
>  	$(QUIET_GEN)$(JSMIN) <$<>$@
>  endif # JSMIN
>  
>  ifdef CSSMIN
> +all:: gitweb.min.css
>  gitweb.min.css: gitweb.css
>  	$(QUIET_GEN)$(CSSMIN) <$>$@
>  endif

That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css. It might be right... and I think the rightness or wrongness might be tied with values of GITWEB>JS and GITWEB_CSS.

Show 7 quoted lines
>  
>  clean:
> -	$(RM) $(FILES)
> +	$(RM) gitweb.cgi gitweb.min.css gitweb.min.js
>  
>  .PHONY: all clean .FORCE-GIT-VERSION-FILE
> 
-- 
Jakub Narebski
Poland
Junio C Hamano· Apr 15, 2010, 00:46 UTC · re: Jakub Narebski · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

Jakub Narebski <jnareb@gmail.com> writes:
>> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>> +gitweb.cgi: gitweb.perl
> ...
> That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
That is what I inteded to do.

Unless you are including gitweb.cgi (iow, the contents of the generated file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi does _not_ depend on these files. Of course if you generate gitweb.cgi out of gitweb.perl with one setting of GITWEB_JS and then change your mind, then you need to regenerate it, but that is not something you can do by comparing file timestamp of gitweb.cgi and the file timestamp of $(GITWEB_JS) anyway. You would need to imitate something like how GIT-BUILD-OPTIONS is used by the primary Makefile.

Jakub Narebski· Apr 15, 2010, 01:02 UTC · re: Junio C Hamano · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

Junio C Hamano wrote:
Show 17 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> >> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >> +gitweb.cgi: gitweb.perl
> > ...
> > That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
> 
> That is what I inteded to do.
> 
> Unless you are including gitweb.cgi (iow, the contents of the generated
> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
> does _not_ depend on these files.  Of course if you generate gitweb.cgi
> out of gitweb.perl with one setting of GITWEB_JS and then change your
> mind, then you need to regenerate it, but that is not something you can do
> by comparing file timestamp of gitweb.cgi and the file timestamp of
> $(GITWEB_JS) anyway.  You would need to imitate something like how
> GIT-BUILD-OPTIONS is used by the primary Makefile.
Right.  I agree.

P.S. This is nt the case with git-instaweb, which literally include gitweb.js or gitweb.min.js...

-- 
Jakub Narebski
Poland
Mark Rada· Apr 15, 2010, 01:21 UTC · re: Jakub Narebski · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

On 10-04-14 9:02 PM, Jakub Narebski wrote:
Show 24 quoted lines
> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>>> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>> +gitweb.cgi: gitweb.perl
>>> ...
>>> That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
>>
>> That is what I inteded to do.
>>
>> Unless you are including gitweb.cgi (iow, the contents of the generated
>> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
>> does _not_ depend on these files.  Of course if you generate gitweb.cgi
>> out of gitweb.perl with one setting of GITWEB_JS and then change your
>> mind, then you need to regenerate it, but that is not something you can do
>> by comparing file timestamp of gitweb.cgi and the file timestamp of
>> $(GITWEB_JS) anyway.  You would need to imitate something like how
>> GIT-BUILD-OPTIONS is used by the primary Makefile.
> 
> Right.  I agree.
> 
> P.S. This is nt the case with git-instaweb, which literally include 
> gitweb.js or gitweb.min.js...
> 

I believe we can just change the variable names as planned before as well as the dependancies, and that should fix up instaweb as far as will need to be fixed, the rest has to happen at gitweb.cgi generation time.

My understanding (please correct me if I am wrong) is that comparing the mtimes of the files is not reliable. So can't we just make gitweb.cgi depend on source .js and .css files, since any modifications there will also cause the minified versions to be regenerated? Can Junio's patch just be updated to subst the suffix of GITWEB_JS and GITWEB_CSS, which makes sure the correct version is being used?

-- 
Mark Rada
Junio C Hamano· Apr 15, 2010, 01:42 UTC · re: Junio C Hamano · lore

Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Unless you are including gitweb.cgi (iow, the contents of the generated
> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
> does _not_ depend on these files.  Of course if you generate gitweb.cgi
> out of gitweb.perl with one setting of GITWEB_JS and then change your
> mind, then you need to regenerate it, but that is not something you can do
> by comparing file timestamp of gitweb.cgi and the file timestamp of
> $(GITWEB_JS) anyway.  You would need to imitate something like how
> GIT-BUILD-OPTIONS is used by the primary Makefile.
Here is a minimally tested patch.

In addition to the points raised by the proposed log message for the previous one, this tries to make sure that the scripts are regenerated whenever the replacement variables are modified. For a good measure, if you used different JSMIN/CSSMIN since the last time you produced minified version of these files, they are regenerated.

If this seems to work Ok, please send it back to me with a proper commit
log message (concat of the above and the previous) with Tested-by: or
Acked-by: as appropriate, so that we can fix it at the tip of 'master'
before we tag 1.7.1-rc2.
Thanks.
 gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index ffee4bd..f2e1d92 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -80,54 +80,55 @@ endif
 
 all:: gitweb.cgi
 
-FILES = gitweb.cgi
 ifdef JSMIN
-FILES += gitweb.min.js
 GITWEB_JS = gitweb.min.js
+all:: gitweb.min.js
+gitweb.min.js: gitweb.js GITWEB-BUILD-OPTIONS
+	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif
+
 ifdef CSSMIN
-FILES += gitweb.min.css
 GITWEB_CSS = gitweb.min.css
+all:: gitweb.min.css
+gitweb.min.css: gitweb.css GITWEB-BUILD-OPTIONS
+	$(QUIET_GEN)$(CSSMIN) <$ >$@
 endif
-gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
 
-gitweb.cgi:
+GITWEB_REPLACE = \
+	-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
+	-e 's|++GIT_BINDIR++|$(bindir)|g' \
+	-e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \
+	-e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \
+	-e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \
+	-e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \
+	-e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \
+	-e 's|"++GITWEB_PROJECT_MAXDEPTH++"|$(GITWEB_PROJECT_MAXDEPTH)|g' \
+	-e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \
+	-e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
+	-e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
+	-e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
+	-e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
+	-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
+	-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
+	-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+	-e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
+	-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
+	-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g'
+
+GITWEB-BUILD-OPTIONS: FORCE
+	@rm -f $@+
+	@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
+	@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@
+
+gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-	    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
-	    -e 's|++GIT_BINDIR++|$(bindir)|g' \
-	    -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \
-	    -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \
-	    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \
-	    -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \
-	    -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \
-	    -e 's|"++GITWEB_PROJECT_MAXDEPTH++"|$(GITWEB_PROJECT_MAXDEPTH)|g' \
-	    -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \
-	    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
-	    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
-	    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
-	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
-	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
-	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
-	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
-	    -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
-	    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-	    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
-	    $< >$@+ && \
+		$(GITWEB_REPLACE) $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
-ifdef JSMIN
-gitweb.min.js: gitweb.js
-	$(QUIET_GEN)$(JSMIN) <$< >$@
-endif # JSMIN
-
-ifdef CSSMIN
-gitweb.min.css: gitweb.css
-	$(QUIET_GEN)$(CSSMIN) <$ >$@
-endif
-
 clean:
-	$(RM) $(FILES)
+	$(RM) gitweb.cgi gitweb.min.js gitweb.min.css GITWEB-BUILD-OPTIONS
+
+.PHONY: all clean .FORCE-GIT-VERSION-FILE FORCE
 
-.PHONY: all clean .FORCE-GIT-VERSION-FILE

← back to recent threads