Bug 208422 - Eclipse project download page needs love
Summary: Eclipse project download page needs love
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Releng (show other bugs)
Version: 4.2   Edit
Hardware: All All
: P2 normal (vote)
Target Milestone: ---   Edit
Assignee: David Williams CLA
QA Contact:
URL: http://download.eclipse.org/eclipse/d...
Whiteboard:
Keywords: bugday, helpwanted
: 219618 220182 381257 (view as bug list)
Depends on:
Blocks: 205205 208381
  Show dependency tree
 
Reported: 2007-11-01 10:38 EDT by Denis Roy CLA
Modified: 2012-06-19 09:59 EDT (History)
13 users (show)

See Also:


Attachments
Diff (9.09 KB, patch)
2012-06-14 13:33 EDT, Denis Roy CLA
no flags Details | Diff
nova theme patch (83.97 KB, patch)
2012-06-14 14:23 EDT, Christopher Guindon CLA
no flags Details | Diff
Latest nova patch (18.30 KB, patch)
2012-06-14 14:28 EDT, Christopher Guindon CLA
no flags Details | Diff
Nova theme integration patch (17.43 KB, patch)
2012-06-14 14:40 EDT, Christopher Guindon CLA
no flags Details | Diff
Updated nova theme integration patch (6.46 KB, patch)
2012-06-15 10:23 EDT, Christopher Guindon CLA
no flags Details | Diff
Latest nova patch. (17.28 KB, patch)
2012-06-18 10:22 EDT, Christopher Guindon CLA
no flags Details | Diff
Nova theme integration patch for download site (5.29 KB, patch)
2012-06-18 10:35 EDT, Christopher Guindon CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Denis Roy CLA 2007-11-01 10:38:51 EDT
The Eclipse project download page should be updated to look like the rest of the Eclipse website and to be more user-friendly while catering to the committer and developer communities who know what the page is about.

I know you guys are under strain, so I'm adding the helpwanted keyword here.
Comment 1 Kim Moir CLA 2007-11-01 11:30:37 EDT
Thanks Denis.  Yes, it definitely needs to be updated.  I would truly welcome contributions in this area and be happy to help people test changes.  It seems I'm always too busy adding stuff to the build to find time to make the build page look pretty.
Comment 2 Nick Boldt CLA 2007-11-02 16:48:36 EDT
Would you be cool with something similar to the way we do things in Modeling?

http://www.eclipse.org/modeling/emf/downloads/
http://www.eclipse.org/modeling/emft/downloads/
http://www.eclipse.org/modeling/mdt/downloads/
Comment 3 Kim Moir CLA 2007-11-02 17:54:11 EDT
Nick, 

The modeling pages make great use of Phoenix and is very well organized.  I personally find (don't be offended :-)) that they display a lot of information on a single page.  But I am a bit of a minimalist :-). 

Note that this bug only refers to the main download page for our project.  Not the subpages for each build.  This is a separate issue and there is certainly also room for improvement in that area.

I'd like to have a page that clearly identifies what a new user should download versus a developer getting the latest I-build.  Also, the page should reflect new build content dynamically as it does today.

Sonia and I previously discussed having a separate download page for regular users versus committers to hide some of the details.  But in reality I don't know how many joe six-pack users actually use this page as opposed to downloading eclipse from http://www.eclipse.org/downloads/.  I'd have to take a look at the stats to see if this would be worth the effort.
Comment 4 Nick Boldt CLA 2007-11-03 00:19:42 EDT
Well, if you take what we have in Modeling and ...

... instead of having expand/collapse links to the information that is each build's "page" ...

... link to that build's page (a separate document, as you have it, rather than a nested section)... 

... then all tht's required is a new skin on an existing useful page. 

I'd be +1 for this idea, and for possibly contributing a first pass at it come next BugDay.

---

As to the stats for usage of the page... there's two communities to consider, and based on my experience in #eclipse, here's what I'd suggest:

* End Users -- often ask "where can I find the download for my platform?". Apparently no one sees the link on http://eclipse.org/downloads/ to http://www.eclipse.org/downloads/moreinfo/classic.php on "Find out more..." which in turn provides a link for "All versions" (http://download.eclipse.org/eclipse/downloads/). This route-to-market could be simpler.

* Developers -- I use http://download.eclipse.org/eclipse/downloads/ at least once a week. We point #eclipse people there too when the EPP and Eclipse Classic options don't work for them, or they're looking for something newer than the latest R build. To encourage early adoption of 3.4, we send people there all the time.

---

I suppose another approach would be to set up vanity URLs (as Denis once suggested)... so that you could provide an ever-moving target link to:

: "the latest Eclipse 3.4 S build -- http://www.eclipse.org/downloads/stable/S/
: "the latest Eclipse 3.4 I build -- http://www.eclipse.org/downloads/stable/I/
: "the latest Eclipse Release -- http://www.eclipse.org/downloads/release/
: "the latest Eclipse 3.2  -- http://www.eclipse.org/downloads/release/3.2/
etc.

Then these index.php pages would just look at the filesystem on download1.eclipse and pull up the latest n' greatest directory (based on pattern match) and morph that into a URL link, eg., 

"latest S" -> get all S dirs, then sort by date and grab latest -> S-3.4M3-200711012000 -> http://download.eclipse.org/eclipse/downloads/drops/S-3.4M3-200711012000/ or "latest R" -> get all R dirs, then sort by date and grab latest -> R-3.3.1.1-200710231652 -> http://download.eclipse.org/eclipse/downloads/drops/R-3.3.1.1-200710231652/

Comment 5 Ian Skerrett CLA 2007-11-28 11:51:10 EST
Kim, can Nathan help update this download page to use the Phoenix skin?
Comment 6 Kim Moir CLA 2007-11-28 13:46:38 EST
Certainly :-).  One note - this page is automatically replicated from our build machine via rsync.  Something to keep in mind if you are trying it out the changes directly on the eclipse.org filesystem.
Comment 7 Denis Roy CLA 2007-11-28 13:57:58 EST
(In reply to comment #6)
> directly on the eclipse.org filesystem.

No way, we would never do that. Besides, Nathan doesn't have write access there and webmasters never abuse our r00tness that way.

My suggestion would be to set up a mirror environment for Nathan where he can write and test the page.  When it's all good, he can attach a patch to this bug -- or a diff -u -- for your consideration.  

Kim, Nathan, does that sound ok?

For all intents and purposes, Phoenix is aliased and available on download.eclipse.org, and just include() 'ing the header would already be a giant step forward:

http://download.eclipse.org/eclipse.org-common/themes/Phoenix/header.php
Comment 8 Kim Moir CLA 2007-11-28 14:30:10 EST
Yes, that sounds good.  I wasn't suggesting that you update the main file, but rather test a copy.  But the mirrored enviroment seems like an even better idea :-)
Comment 9 Nick Boldt CLA 2007-11-28 17:01:20 EST
You might enjoy this -- it's how I test *.eclipse.org pages on localhost before publishing them to the server: 

http://wiki.eclipse.org/Eclipse_Server_Sandbox_Setup
Comment 10 Denis Roy CLA 2008-02-20 16:48:59 EST
*** Bug 219618 has been marked as a duplicate of this bug. ***
Comment 11 Denis Roy CLA 2008-02-20 16:49:48 EST
From Karl @ bug 219618:

Many of the important pages on the eclipse.org site use the word 'here' as a
link name instead of a meaningful description of where the link leads to. 

This is very bad style, as experienced web users will let their eyes flow from
link to link until they find what they are looking for -- if one can avoid
reading all the text in between, then all the better; this is impossible when
many important links are all called 'here'.
Comment 12 Kim Moir CLA 2008-04-09 16:13:12 EDT
*** Bug 220182 has been marked as a duplicate of this bug. ***
Comment 13 Kim Moir CLA 2008-04-09 16:14:00 EDT
From bug 220182, link to archives should be more prominent.
Comment 14 John Arthorne CLA 2012-05-31 15:21:00 EDT
*** Bug 381257 has been marked as a duplicate of this bug. ***
Comment 15 Denis Roy CLA 2012-05-31 15:26:36 EDT
I was just hit by a cloud of dust.
Comment 16 John Arthorne CLA 2012-05-31 15:39:41 EDT
Just to give some technical information, this page is a plain html file (index.html) in the directory you would expect. However the page is generated from a PHP script during the build. See createIndex4x.php in the same directory. You can view the dynamic page here:

http://download.eclipse.org/eclipse/downloads/createIndex4x.php

Personally I think we could do away with this generation during the build and just always use a dynamic index.php, but there are build changes needed to support that.

I'm feeling a bit dizzy from the blistering rate of change here though. I mean, we only just updated parts of our download page to Phoenix in bug 58101 and now here comes Nova already!
Comment 17 Nathan Gervais CLA 2012-05-31 15:42:48 EDT
(In reply to comment #16)

> 
> I'm feeling a bit dizzy from the blistering rate of change here though. I mean,
> we only just updated parts of our download page to Phoenix in bug 58101 and now
> here comes Nova already!

Well played Sir, well played.

I'm happy to provide any help in terms of getting Nova setup on these pages.
Comment 18 David Williams CLA 2012-05-31 15:58:51 EDT
(In reply to comment #16)

> 
> http://download.eclipse.org/eclipse/downloads/createIndex4x.php
> 
> Personally I think we could do away with this generation during the build and
> just always use a dynamic index.php, but there are build changes needed to
> support that.

Actually, we convert to HTML (primarily) for simply good-citizen web server efficiency :) ... the "dynamic" parts of PHP do not change very frequently. 

Plus, I suspect, there is a little funky history of how "we" evolved to have 4.2 be primary build (and build download page) while under the covers we have just one download site .... just two download pages (one for 3.8 and one for 4.2). 

I've always suspected we could "mimic" how equinox does it 
http://download.eclipse.org/equinox/
but have not looked at details.
Comment 19 Denis Roy CLA 2012-05-31 16:12:55 EDT
Simply wrapping the current index.html inside the Phoenix/Nova header and footer make a world of difference, so I don't think an overhaul here will be a significant investment of time:

http://download.eclipse.org/eclipse/downloads/index_denis.php

index_denis.php simply contains:

<?php
include($_SERVER['DOCUMENT_ROOT'] . "/eclipse.org-common/themes/Nova/header.php");
include("index.html");
include($_SERVER['DOCUMENT_ROOT'] . "/eclipse.org-common/themes/Nova/footer.php");
?>
Comment 20 Denis Roy CLA 2012-06-08 13:56:16 EDT
I just gave a copy of the files to Chris!
Comment 21 David Williams CLA 2012-06-10 22:23:13 EDT
(In reply to comment #20)
> I just gave a copy of the files to Chris!

If you mean 
createIndex4x.php
and 
eclipse3x.php

There are copies of these files in our git repo named
eclipse.platform.releng.eclipsebuilder

under 
downloadsites/eclipse/downloads

There's nothing "automatic" about publishing those files from git to the downloads server, but ... patches preferred. (And, I just made changes this weekend). 


I should mention, one problem "we" have had, is getting a version that "works" on "download.eclipse.org" but which also works (to some degree at least) on other servers, either my own test machines, or even build.eclipse.org. 

For example, I had to "jump through hoops" to get a page such as 
http://build.eclipse.org/eclipse/eclipse4I/siteDir/eclipse/downloads/drops4/I20120608-1400/
to half-way display something, even though its "not quite right" (nor does it need to be perfect). 
The template for that files is in same repo, but different directory: 
/eclipse/publishingFiles/templateFiles/index.php.template
and it does go through some changes "during the build" (hence, "template").

But, the point is, ideally these files should "work" on any server, at least display approximation, to display something, so we can build and/or test pages, without them literally being on "downloads.eclipse.org". It is a constraint you may not be normally aware of. Suggestions/help in that regard appreciated. (I've not looked or studies closely, but think some of the normal templates or libraries don't help, since they often assume questionable relative directory locations, such as "somethingStandard" refers to "../../somethingElseStandard" which would not be true on the build server. 


The other place I've put in effort in the past few months, is to get the page to be "clean" in w3c validator: 
try 
http://download.eclipse.org/eclipse/downloads/index.html
in URL field of 
http://validator.w3.org/
How many pages at Eclipse are clean? (I've not looked lately, myself, maybe lots) and I am not sure how many people think that is important ... but, I do, and I think everyone should :) 

Third, 

I put in a lot of work to make the PHP to HTML complete without errors/warnings. 

such as running
php createIndex4x.php > index.php 
now runs with no errors or warnings. 
We all know that PHP, web servers, and web browsers, are very forgiving of errors ... much more forgiving than I am :) [and, I am concerned about such issues for the simple reason if there are lots of "warnings", its impossible to tell which are significant and with are not]. Even if we do not literally do the php to html conversion on a regular basis, whenever there is a serious problem (e.g. page doesn't display at all) that is the first thing to check ... if there are routinely scores of errors/warnings, then it is hard to tell what's changed that suddenly made the page go bad.

Fourth, I put in a lot of effort to make

createIndex4x.php
and 
eclipse3x.php
nearly identical. 

While still an afternoon or two of work to do there, the idea was to get to the point where "the real work" was does by a few common "functions" and the stream specific files would be just a 10 lines or so. 


I mention all this partially to brag about all the work I've done :) but mostly to let you know that there are constraints than "style" going on here and even more so to let you know the pages are being worked on, and not just sitting there "stagnate". 

Hope that clarifies the current state and situation.
Comment 22 Denis Roy CLA 2012-06-11 11:29:02 EDT
(In reply to comment #21)
> (In reply to comment #20)
> > I just gave a copy of the files to Chris!
> 
> If you mean 
> createIndex4x.php
> and 
> eclipse3x.php

We weren't looking at those, but thanks for the pointers. I just went over them with Chris.
Comment 23 Christopher Guindon CLA 2012-06-14 13:18:14 EDT
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #20)
> > > I just gave a copy of the files to Chris!
> > 
> > If you mean 
> > createIndex4x.php
> > and 
> > eclipse3x.php
> 
> We weren't looking at those, but thanks for the pointers. I just went over them
> with Chris.

I did some changes to the createIndex4x.php that will output the content in 3 different format. My modified version for createIndex4x.php is here :

http://download.eclipse.org/technology/phoenix/eclipse/downloads/index_chris.php

This url will print the content inside the nova theme. Just like any other page on eclipse.org. The content is computed on every page load.

I also added a view to see the old html format :
http://download.eclipse.org/technology/phoenix/eclipse/downloads/index_chris.php?layout=html

Finally I added a php format. In case you don't want php to do all the computing on every page load this create the code that you need to copy inside a php file. (From what I can see, this is very similar to how the index.html is created from createIndex4x.php)

You will need to view the source code. The page will include all the php code to load the nova theme from eclipse-common but the actual content will be static.

http://download.eclipse.org/technology/phoenix/eclipse/downloads/index_chris.php?layout=php

Here how it all looks once the source code is copied over to a php file:
http://download.eclipse.org/technology/phoenix/eclipse/downloads/index_test.php
Comment 24 Denis Roy CLA 2012-06-14 13:33:19 EDT
Created attachment 217373 [details]
Diff

Attached is a diff -u of the current createIndex4.php and Chris' version.
Comment 25 Christopher Guindon CLA 2012-06-14 13:38:02 EDT
(In reply to comment #24)
> Created attachment 217373 [details]
> Diff
> 
> Attached is a diff -u of the current createIndex4.php and Chris' version.

(In reply to comment #24)
> Created attachment 217373 [details]
> Diff
> 
> Attached is a diff -u of the current createIndex4.php and Chris' version.

I think you compared the wrong files. My script is here :
http://download.eclipse.org/technology/phoenix/eclipse/downloads/index_chris.php
Comment 26 Christopher Guindon CLA 2012-06-14 14:23:16 EDT
Created attachment 217375 [details]
nova theme patch
Comment 27 Christopher Guindon CLA 2012-06-14 14:28:52 EDT
Created attachment 217376 [details]
Latest nova patch
Comment 28 Christopher Guindon CLA 2012-06-14 14:40:39 EDT
Created attachment 217377 [details]
Nova theme integration patch

Removed a few unnecessary lines.
Comment 29 David Williams CLA 2012-06-14 21:32:21 EDT
Chris, 

Thanks for the patches! In general the kind of changes you are suggesting look reasonable and I think the "prototype" page looks suitable (though, it is a good illustration of how beauty is in the eye of the beholder :) ... not that ours was "pretty" ... but Nova doesn't exactly look state of the art (no offence) ... and I strongly agree it is important to have the consistency, standard links at top, etc. so am quite happy to get the help to accomplish that. 

I'm guessing ... hoping? ... you would like to learn in this process, as well as teach us? Based on that assumption, I'll give you feedback, and go through this process like I would any other contributor submitting patches, and hopefully what you learn will serve you for years to come. 

First, the patch does not apply cleanly to the version in our repository. It appears the version you were working with was several weeks (10 days?) old, so I think there have been some recent changes, not in your version, hence doesn't apply cleanly and hence, as we'd ask any contributor, can you you please provide a new patch that is against the current version in 'master'? 

Second, you made some changes that appear unrelated to what this bug is about. The largest of those was changing all the "echo's" to "Prints's" ... unless I'm missing something that's unrelated to the theme issue, and just your style ... and, even if there are good reasons for it, that should be addressed in a separate bug. The simple reason is that if there are fewer changes, it makes the "theme" changes much easier to "see" and evaluate. As a busy committer, evaluating contributor's patches, such unnecessary changes look like "noise" and slows us down. [I'm not being critical, and really do appreciate the patches and improvements ... just trying to explain -- in extra detail even -- ... and, I know, it's a pain :) ] 

Third, your options of providing URL parameters to control PHP vs. HTML, etc., seems like over-engineering, and just confuses the issue. Or, I'm missing something? Not sure if you thought that was required? Or ... if you just thought it was interesting? Which it is! ... but ... I personally try to stay away from "get parameters" since then I don't need to worry if they are secure or not ... I am sure yours are ... but ... I'd have to "read up" on it and study the code a bit more than usual to see it for myself. So, unless they are needed for reasons I don't understand, we can skip that feature.

If in fact you don't care much about learning how to make great contributed patches ... that is, this is sort of a "one off" task for you and you are plenty busy with other things (as I'm  sure you are) ... then that's fine too, just say so, and I'm sure eventually I can eventually tease apart your code to make the changes we need. 

All those feedback comments are mostly to help or teach you (not me being critical of your help, style, designs, etc. ... all of which we greatly appreciate). 

Thanks again,
Comment 30 Christopher Guindon CLA 2012-06-15 10:23:02 EDT
Created attachment 217429 [details]
Updated nova theme integration patch

(In reply to comment #29)
> Chris, 
> 
> I'm guessing ... hoping? ... you would like to learn in this process, as well
> as teach us? Based on that assumption, I'll give you feedback, and go through
> this process like I would any other contributor submitting patches, and
> hopefully what you learn will serve you for years to come. 

Definitely, I really appreciate that you are taking some of your time to go trough this patch with me!

> 
> First, the patch does not apply cleanly to the version in our repository. It
> appears the version you were working with was several weeks (10 days?) old, so
> I think there have been some recent changes, not in your version, hence doesn't
> apply cleanly and hence, as we'd ask any contributor, can you you please
> provide a new patch that is against the current version in 'master'? 

My latest attachment is a patch that I created with the latest version of createIndex4x.php. I got this file from cloning this repo. I am hoping that I am using the right one : git.eclipse.org/gitroot/platform/eclipse.platform.releng.eclipsebuilder.git 

> 
> Second, you made some changes that appear unrelated to what this bug is about.
> The largest of those was changing all the "echo's" to "Prints's" ... unless I'm
> missing something that's unrelated to the theme issue, and just your style ...
> and, even if there are good reasons for it, that should be addressed in a
> separate bug. The simple reason is that if there are fewer changes, it makes
> the "theme" changes much easier to "see" and evaluate. As a busy committer,
> evaluating contributor's patches, such unnecessary changes look like "noise"
> and slows us down. [I'm not being critical, and really do appreciate the
> patches and improvements ... just trying to explain -- in extra detail even --
> ... and, I know, it's a pain :) ]

I totally understand, print and echo basically are the same thing. I personally prefer to use print but my latest patch that i am submitting now using echo!  
> 
> Third, your options of providing URL parameters to control PHP vs. HTML, etc.,
> seems like over-engineering, and just confuses the issue. Or, I'm missing
> something? Not sure if you thought that was required? Or ... if you just
> thought it was interesting? Which it is! ... but ... I personally try to stay
> away from "get parameters" since then I don't need to worry if they are secure
> or not ... I am sure yours are ... but ... I'd have to "read up" on it and
> study the code a bit more than usual to see it for myself. So, unless they are
> needed for reasons I don't understand, we can skip that feature.

The reason I did that is because I am not really sure how you guys actually plan in using this script. From what I saw from the code, (Please correct me if I am wrong here since this is only my assumption) I think that createIndex4x.php was used to generate a static html file where the source code was then transferred to the index.html. 

So if this is the case I am presuming that you are doing this because you need to create a static version of this page instead of just using createIndex4x.php to generate the content on each page load.

This is why I created the php display. This view generate the code needed to load the nova themplate with php but having the content of this page static the same way it was done in the past.

Since I was doing this I decided to add the html view in there in case you still need to generate the old version of this page.

My $_GET variable is being sanitize on this line:

#OPTIONS: php, html. If the value of the $_GET['layout'] is something else the value of $layout is 'default';
  $layout = (!isset($_GET['layout']) || ($_GET['layout'] != "php" && $_GET['layout'] != "html")) ? 'default' : $_GET['layout'];

the $layout variable can only be 1 of the 3 possibilities. If something else that php or html is set in that variable then $layout is set to default.

The default layout is basically a page with the nova theme. The content here is not static and is calculated on each page load.

If you can give me more information on how you plan to use this page I will gladly update my patch and remove the unnecessary views. I basically did that to have my patch be more flexible and useful no mater how you where planning to use this page.

> 
> If in fact you don't care much about learning how to make great contributed
> patches ... that is, this is sort of a "one off" task for you and you are
> plenty busy with other things (as I'm  sure you are) ... then that's fine too,
> just say so, and I'm sure eventually I can eventually tease apart your code to
> make the changes we need. 
> 
> All those feedback comments are mostly to help or teach you (not me being
> critical of your help, style, designs, etc. ... all of which we greatly
> appreciate). 
> 
> Thanks again,

Don't get me wrong I really appreciate you doing this! I really try and want to do the right thing. I appreciate all type of feedback, it can only help me!
thanks!!
Comment 31 David Williams CLA 2012-06-15 14:12:02 EDT
Thanks again. These patches applied cleanly. (But apologies, in the light of day, realized I had changes committed locally, but not pushed to repo ... but, apparently, without all the echo->print conversions the other changes could still be automatically merged ... or what ever else you did). 

First what are the changes in default_style.css for? Are they required? I assume those changes would not hurt anything if other files used it? Would it change their appearance? [I don't really care, just want to understand.]. 

I'll comment on use-cases in a bit.
Comment 32 Christopher Guindon CLA 2012-06-15 14:20:30 EDT
(In reply to comment #31)
> Thanks again. These patches applied cleanly. (But apologies, in the light of
> day, realized I had changes committed locally, but not pushed to repo ... but,
> apparently, without all the echo->print conversions the other changes could
> still be automatically merged ... or what ever else you did). 

That would explain why we were out of sync!
> 
> First what are the changes in default_style.css for? Are they required? I
> assume those changes would not hurt anything if other files used it? Would it
> change their appearance? [I don't really care, just want to understand.]. 
> 
> I'll comment on use-cases in a bit.

The changes in in the css are required they allow me to add some padding in the around the content. (white space left & right) for the php and default view. This is necessary since those view used the nova theme.

Here is what I am adding : 	

.container_php, .container_default{ padding:0 20px}

This is applied to the container div. The container div class is dynamic since it use the $layout variable to complete the name of the div.

<div class="container_<?php echo $layout;?>">

Because of this naming convention container_<?php echo $layout;?> it shouldn't affect any other file that is using this style-sheet.
Comment 33 David Williams CLA 2012-06-15 15:08:46 EDT
Here's the two use main use cases we need/use, and current version (with your patches) still works for those (without any parameters on URL): 

1. like any other index.php in browser
http://download.eclipse.org/eclipse/downloads/createIndex4xB.php

We will rename this one to index.php after Juno, but we'll still have eclipse3x.php served from same directory so, obviously, can't name them both index.php. 

2. We do create HTML versions, using this "trick": 

wget -O createIndex4xB.html http://download.eclipse.org/eclipse/downloads/createIndex4xB.php

scp createIndex4xB.html build:/home/data/httpd/download.eclipse.org/eclipse/downloads/
 
I suspect one of your parameters, is meant to do something similar, but, our current "trick" is fine by me. 

[Note: as I mentioned in previous note, the main reason we convert to HTML is that we know PHP files which "read directories" to decide their content is not a great thing ... perhaps someday we'll have XML :) ... but, until then we convert.

So far, so good. 

3. run, from command line, from SSH shell, in "downloads" directory: 
 
   php createIndex4xB.php > createIndex4xB.html
   (meant to "do same thing" as the wget/copy-back trick). 

   This no longer works, since there is no "_SERVER"  when running from command line ... so, results in fatal error. 


4. Other machines. '3' wouldn't be that big of a deal ... I only use it when something odd happens and I want to regenerate that page so I could always resort to "wget trick" pattern ... but not having the "right" server also prevents me from using this same file on my "test server" (i.e. my big lap top mostly :) when I'm testing new (potential) changes to build, etc. I prefer to use same files as much as possible, rather than have my own test setup, since a) easier and b) provides a bit more of an accurate test in some cases. 

So, any recommendations on how to "loosen" the dependence on _SERVER array and/or those "standard" Eclipse files not being available? In some other PHP files, I've used things like 

if (array_key_exists("SERVER_NAME", $_SERVER)) {
    $servername = $_SERVER["SERVER_NAME"];
    if ($servername === "build.eclipse.org") { 
....

To do one thing when accessed directly on "build.eclipse.org", but then the more standard thing when accessed on "download.eclipse.org". 

We can open a separate bug if these _SERVER issues are that separate, but your current solution accomplishes the main goal of this bug and we can use as is ... except I still don't see a reason for the request variables, so, you can resubmit patch without them and we'd be good to go ... except .... 

If anything, perhaps you could still have some different "cases" but just decide "automatically" and not depend on request parameters. 

Perhaps, if _SERVER array doesn't exist or "servername != "downloads.eclipse.org" then use (your current) HTML mode ... or .... something better? 

I have always assume those require statements, 
require_once($_SERVER['DOCUMENT_ROOT'] ....

used that form ($_SERVER['DOCUMENT_ROOT']) for some security or performance reason? I assume (and think I've tried :) using some full "http://download.eclipse.org/... " reference would not work? 

So, in summary, as-is, without the "request parameters" or ... some cool twist to still allow display on other build server or machines.
Comment 34 David Williams CLA 2012-06-15 15:15:44 EDT
Oh, I also see running your current page through validator, it displays 6 errors. 

I didn't look at them closely, but you might, to see if anything that could be easily fixed (or, cause for concern). 

http://validator.w3.org/
Comment 35 Christopher Guindon CLA 2012-06-15 15:25:15 EDT
(In reply to comment #34)
> Oh, I also see running your current page through validator, it displays 6
> errors. 
> 
> I didn't look at them closely, but you might, to see if anything that could be
> easily fixed (or, cause for concern). 
> 
> http://validator.w3.org/

I know... I already did the test yesterday :) These errors are in nova template and if i remember correctly it's because there is <div inside a list (Visit other eclipse site box at the top right). I will open a separate bug for this because I don't have commit access to nova theme files and also once this is fix it will validate most of the pages on eclipse.org.
Comment 36 Christopher Guindon CLA 2012-06-15 15:32:45 EDT
(In reply to comment #33)
> Here's the two use main use cases we need/use, and current version (with your
> patches) still works for those (without any parameters on URL): 
> 
> 1. like any other index.php in browser
> http://download.eclipse.org/eclipse/downloads/createIndex4xB.php
> 
> We will rename this one to index.php after Juno, but we'll still have
> eclipse3x.php served from same directory so, obviously, can't name them both
> index.php. 
> 
> 2. We do create HTML versions, using this "trick": 
> 
> wget -O createIndex4xB.html
> http://download.eclipse.org/eclipse/downloads/createIndex4xB.php
> 
> scp createIndex4xB.html
> build:/home/data/httpd/download.eclipse.org/eclipse/downloads/
> 
> I suspect one of your parameters, is meant to do something similar, but, our
> current "trick" is fine by me. 
> 
> [Note: as I mentioned in previous note, the main reason we convert to HTML is
> that we know PHP files which "read directories" to decide their content is not
> a great thing ... perhaps someday we'll have XML :) ... but, until then we
> convert.
> 
> So far, so good. 
> 
> 3. run, from command line, from SSH shell, in "downloads" directory: 
> 
>    php createIndex4xB.php > createIndex4xB.html
>    (meant to "do same thing" as the wget/copy-back trick). 
> 
>    This no longer works, since there is no "_SERVER"  when running from command
> line ... so, results in fatal error. 
> 
> 
> 4. Other machines. '3' wouldn't be that big of a deal ... I only use it when
> something odd happens and I want to regenerate that page so I could always
> resort to "wget trick" pattern ... but not having the "right" server also
> prevents me from using this same file on my "test server" (i.e. my big lap top
> mostly :) when I'm testing new (potential) changes to build, etc. I prefer to
> use same files as much as possible, rather than have my own test setup, since
> a) easier and b) provides a bit more of an accurate test in some cases. 
> 
> So, any recommendations on how to "loosen" the dependence on _SERVER array
> and/or those "standard" Eclipse files not being available? In some other PHP
> files, I've used things like 
> 
> if (array_key_exists("SERVER_NAME", $_SERVER)) {
>     $servername = $_SERVER["SERVER_NAME"];
>     if ($servername === "build.eclipse.org") { 
> ....
> 
> To do one thing when accessed directly on "build.eclipse.org", but then the
> more standard thing when accessed on "download.eclipse.org". 
> 
> We can open a separate bug if these _SERVER issues are that separate, but your
> current solution accomplishes the main goal of this bug and we can use as is
> ... except I still don't see a reason for the request variables, so, you can
> resubmit patch without them and we'd be good to go ... except .... 
> 
> If anything, perhaps you could still have some different "cases" but just
> decide "automatically" and not depend on request parameters. 
> 
> Perhaps, if _SERVER array doesn't exist or "servername !=
> "downloads.eclipse.org" then use (your current) HTML mode ... or .... something
> better? 
> 
> I have always assume those require statements, 
> require_once($_SERVER['DOCUMENT_ROOT'] ....
> 
> used that form ($_SERVER['DOCUMENT_ROOT']) for some security or performance
> reason? I assume (and think I've tried :) using some full
> "http://download.eclipse.org/... " reference would not work? 
> 
> So, in summary, as-is, without the "request parameters" or ... some cool twist
> to still allow display on other build server or machines.

O.k I understand now! My php view is totally unnecessary here so I will remove that.  I will also update my patch to see if the file is on build if not it will display the old format (HTML). 

I will also remove the request parameters.

What would be best here? Should I prepare a patch based on the last patch I did or should I create a patch based off the file that is currently in git?
Comment 37 Christopher Guindon CLA 2012-06-15 15:54:23 EDT
(In reply to comment #34)
> Oh, I also see running your current page through validator, it displays 6
> errors. 
> 
> I didn't look at them closely, but you might, to see if anything that could be
> easily fixed (or, cause for concern). 
> 
> http://validator.w3.org/

I created this bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=382770 regarding the wc3 validation test!
Comment 38 David Williams CLA 2012-06-15 15:57:39 EDT
> What would be best here? Should I prepare a patch based on the last patch I did
> or should I create a patch based off the file that is currently in git?

Latest (current) from git.
Comment 39 Christopher Guindon CLA 2012-06-18 10:22:05 EDT
Created attachment 217495 [details]
Latest nova patch.

This is my latest patch for the downloads page. 

I've removed the php view and made a change on how the $layout variable is set.

  $layout = (array_key_exists("SERVER_NAME", $_SERVER) && ($_SERVER['SERVER_NAME'] == "download.eclipse.org")) ? "default" : "html";

Basically here the nova theme ("default") is set if the file is hosted on build only.
Comment 40 Christopher Guindon CLA 2012-06-18 10:35:24 EDT
Created attachment 217496 [details]
Nova theme integration patch for download site
Comment 41 David Williams CLA 2012-06-18 17:06:57 EDT
At long last .... thanks for all the love'n!
Comment 42 Denis Roy CLA 2012-06-19 09:59:57 EDT
Thanks!