Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Provide a working out of the box instaweb #11

Closed
ciaranj opened this issue May 23, 2012 · 30 comments
Closed

Provide a working out of the box instaweb #11

ciaranj opened this issue May 23, 2012 · 30 comments
Assignees

Comments

@ciaranj
Copy link
Member

ciaranj commented May 23, 2012

Related to issue #9 and in particular comment #9 (comment) . It would be great if msysgit supported (out of the box) an implementation of the instaweb command that worked without external (where external is, external to the provided git installation) dependencies.

There are it seems various instructions on the web as to how one might set about achieving this, for example ( http://asimilatorul.com/index.php/2009/10/12/git-instaweb-using-mongoose-and-msysgit/ ) however looking at the documentation for instaweb it looks as though we should just provide a msys-happy install of lighthttpd, for example: https://github.com/mkottman/lighttpd-1.5-mingw looks like a potential candidate for inclusion.

I could probably take a look at this, but it may turn out to be beyond my means (hopefully not though.)

@ghost ghost assigned ciaranj May 23, 2012
@ciaranj
Copy link
Member Author

ciaranj commented May 23, 2012

lighttpd would appear to be a royal PiTA to get compiled (requires cmake which is a 10 meg set of binaries I figure won't be popular, unless I'm missing something) will look at mongoose which seems to have less dependencies, but would change msysgit's git's instaweb to default to use mongoose rather than lighttpd ?

@hvoigt
Copy link
Member

hvoigt commented May 23, 2012

Yes if you can get it working with mongoose more easily I do not see a downside. You probably want to make it the default configuration on msysgit using /etc/gitconfig if you do so though.

@ciaranj
Copy link
Member Author

ciaranj commented May 23, 2012

Ah, ok if this is possible then yes that is a much better solution, I'm currently in some sort of file-path hell, I can fire up instaweb running mongoose which I compiled successfully (and trivially) from within the msysgit environment, but I can't seem to make the file munging play nicely... I'm fairly close, but not without changing the way some things are passed to mongoose (likely *nix incompatible) ... also remembering how bad I am at perl :(

@ciaranj
Copy link
Member Author

ciaranj commented May 24, 2012

Sweet, thanks to some help from @harlandpaul (some abs_path calls seem to be required) I've got a working git instaweb using mongoose with no special magic.

So I can do it ;) I'm just not sure how to package things successfully! I need to add the CGI perl module (as per Issue #9) think I can figure this one out, but I'm stuck on 3 other issues :

  1. I had to copy libiconv-2.dll to the git-core folder (there are various incidents on google suggesting this 'fix', but I'm hopeful for a better path-based solution.. not sure yet)
  2. I need to pull in the source code to mongoose into msysgit/msysgit which is a mercurial repository. What is the preference for doing this, should I just take a snapshot and document the current tagname/commit hash and commit that, or should I commit the whole repo yadda, yadda.
  3. I need to make a small change to Git's instaweb scripts, so presumably I also need to change msysgit/git, if I do do these changes do I just submit two pull requests for the separate projects, wait for the first to be consumed, before updating the second's request with the relevant submodule change? (Sorry if this is dumb)

@hvoigt
Copy link
Member

hvoigt commented May 24, 2012

Some answers:

  1. Yes there should be another solution
  2. Just write the release.sh script that downloads, compiles and installs the binaries into msysgit and commit the script (like e.g. /src/openssl/)
  3. Yes that should be submitted as a seperate pull request on msysgit/git possibly referencing this issue.

@dscho
Copy link
Member

dscho commented May 24, 2012

As for Hg repository: we now have a working git remote-hg, so in theory you can call "git clone hg::http://wherever.com/hg/mongoose". I say "in theory" because it requires a working Python, something I did not manage to build for MinGW (except cross-compiling from Linux).

You can see that things are working beautifully here: http://github.com/dscho/hg (updated via a cron job on a Linux machine I have access to).

As to 3) yes, two pull requests are appropriate, the msysgit one including an update to the /git submodule pointer (which might need to be updated if the pull request for the git repository needs to be adjusted).

@ciaranj
Copy link
Member Author

ciaranj commented May 24, 2012

@dscho Hi, sorry, I've done a stab at following the existing patterns that I can see for importing 3rd party dependencies in ( ciaranj/msysgit@e32bca6 ) but i'm not really quite sure how to proceed. ...

  1. Other dependencies appear to get checked in at the end of their release.sh script, is this the correct way to do it in this case as well?
  2. (possibly related to 1) I presume if 1 doesn't happen then I need to add a call to mongoose/release.sh to the WinGit/release.sh file, otherwise I just rely on the WinGit/copy_files.sh copying this (mongoose.exe) across. Which is the
    preferred approach? (presumably this depends on whether you want local 'instaweb' support working for msysgit's Git, as well as for the produced Git artefact?)

@dscho
Copy link
Member

dscho commented May 25, 2012

  1. yes!
  2. you need to adjust copy-files, too.

Great work!

@ciaranj
Copy link
Member Author

ciaranj commented May 27, 2012

So, Progress (but I'm afraid that I need help to even get close to finishing this off);

So, starting with a positive :) An example (no smoke or mirrors) release can be found here: https://docs.google.com/folder/d/0B44eAAsUHKHPMWJ1TnVrZE1NaEE/edit (you will still need to use git instaweb --httpd mongoose to launch it at the moment)

So there's a few steps outstanding;

  1. This pull request on msysgit/msysgit needs to be reviewed, refined and accepted : Added CGI.pm (3.59) to the default perl install msysgit#15 (I think this is required for Issue add support for gitweb integration with gerrit without apache #9 too)
  2. I need help to review the work in https://github.com/ciaranj/msysgit/tree/add_mongoose to determine if I'm doing things 'the right way'
  3. I need help to review the work in https://github.com/ciaranj/git/commits/instaweb_msysgit_changes (one of these commits may need to go upstream as it updates the mongoose config file syntax to the latest version, alternatively we could figure out (and package) whatever older version was supported) and the other commits are basically doing stuff to handle msysgit path mangling, so hopefully someone better than me can figure out a better/neater solution now I've a working proof-of-concept :)

I've create a branch https://github.com/ciaranj/msysgit/tree/issue_11 where the mongoose + cgi branches have been merged and I've re-enabled instaweb (Which is what I used to generate the binary above)

Other than the review process required, there are some functional issues remaining;

  1. The forking/process identifier handling appears broken as re-launching mongoose doesn't work
  2. The snapshot feature of gitweb appears not to do anything

But surprisingly thats all I've noticed :)

@dscho
Copy link
Member

dscho commented May 28, 2012

Probably forking Mongoose requires a properly working fork()? Windows does not have that.

Maybe 2) is exactly the same issue?

@dscho
Copy link
Member

dscho commented May 29, 2012

IOW I think that this code is failing: http://code.google.com/p/mongoose/source/browse/mongoose.c#1178

I could imagine that it tries to execute ".perl" scripts directly, which works on Unix-like systems like Linux because those systems interpret the first line of scripts when they begin with a so-called 'hash-bang', i.e. "#!/usr/bin/perl".

If that is indeed the case, we might need to interpret that line ourselves. Since Mongoose is BSD and Git is GPLv2, we cannot simply copy the relevant code from our compat/mingw.c, unfortunately (Hannes Sixt wrote the useful parse_interpreter() function which would come in handy here).

@ciaranj
Copy link
Member Author

ciaranj commented May 30, 2012

Help! (please) :) I've thrashed about for a few hours on this, but I'm struggling. I can get mongoose launching as is (not sure if this is related to the above 2 comments or not) when running as part of WinGit, and I can get it launching happily as part of MsysGit if I copy libiconv-2.dll from /mingw/bin to /bin ... but if I do this I can no longer switch msysgit branches as the git that executes has 'found' libiconv-2.dll in the /bin folder, and locked it so the checkout can't complete :(

So :) I figured I had to somehow get /mingw/bin on the path that belongs to the environment that git.exe is being executed in by gitweb.exe (when doing instaweb from within msysgit)

Using process explorer (the old sysinternals tool) I can see that I can succesfully pass the path with /mingw/bin pre-pended to the front into mongoose which then happily passes that onto the CGI script (gitweb.cgi) .. which then appears to re-execute perl (succesfully passing in the path) which then executes git.exe .. however at this point the PATH is set to '' (blank)

I know nothing about perl :( but I figure it is something to do with the lines that look like:

open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash

Where git_cmd() returns the path to the git.exe and a git directory location. I guess my question is, how do get 'PATH' so it is set correctly when 'git_cmd()' is executed? .. or am I (still) barking up the wrong tree !

@dscho
Copy link
Member

dscho commented May 31, 2012

@ciaranj thank you for the thorough analysis! However, I am not quite sure where exactly PATH is ''. Is it already when the .cgi is run, or when the .cgi calls cat-file, or when Git starts up? I do not see any PATH mangling in gitweb.cgi here... so I suspect that something fishy is going on either in our compat/mingw.c or in Mongoose.

@ciaranj
Copy link
Member Author

ciaranj commented May 31, 2012

So, when I use process explorer ( http://technet.microsoft.com/en-us/sysinternals/bb896653.aspx ) and select the 'git.exe' process that is eventually launched by cgiweb and I inspect the environment through the tool I see that 'PATH' is set to blank (but other environemntals such as GITWEB_CONF are set!)

There is some path mangling (I think) in git-instaweb.sh where it executes '. git-sh-setup' ... but the git.exe process is '2 levels' in/down from that script and in-between scripts seem to have the correct path .. I imagine this will turn out to be something simple, but perl/cgi is not something I have much (any!) familiarity with sorry :(

@terminatorul
Copy link

Well I could run gitweb.cgi as a normal CGI under apache, but it needed the $GIT variable to be set to the git executable. Do you have it ? I actually set $GIT_BIN first, than GIT="$GIT_BIN/git.exe". Do you have these ?

And I also had to copy the .dll files to the bin directory, but if that is not good enough for this case, than try making an Windows NTFS hard-link than (with mklink as Administrator or just try with fsutil), and see what happens. Or try with a symbolic link, that might be better

I don't know if that will get you somewhere, but it is still something you can try

@ciaranj
Copy link
Member Author

ciaranj commented May 31, 2012

Well I could run gitweb.cgi as a normal CGI under apache, but it needed the $GIT variable to be set to the git executable.
Do you have it ? I actually set $GIT_BIN first, than GIT="$GIT_BIN/git.exe". Do you have these ?

Nope, that's fine git.exe is located and used just peachy :)

And I also had to copy the .dll files to the bin directory, but if that is not good enough for this case, than try making an Windows NTFS hard-link than (with mklink as Administrator or just try with fsutil), and see what happens. Or try with a symbolic link, that might be better

:) I guess I wasn't clear, but I've got everything working just A-OK with the WinGit that gets packaged by this project, the issue I'm having is actually when running in the msysgit environment which won't affect most users, but needs to be addressed (I would expect/imagine) before msysgit will be willing to accept my patches :)

I don't know if that will get you somewhere, but it is still something you can try

Thank you, all helps gratefully received!

@dscho
Copy link
Member

dscho commented May 31, 2012

@terminatorul I advise against using symbolic links, someone in the community had to spend serious time figuring out an issue with them. Hardlinks, however, continue to work as in the good old DOS times (yeah, the "good" was ironic).

@ciaranj I am afraid that I had problems when I tried to set up a system here, running from msysGit:

set_ports_option: cannot bind to 1234: No error

(This was in .git/gitweb/mongoose/error.log)

Speaking of the paths: Maybe we want to put the "pid" file into .git/gitweb/? Or is this git-instaweb in general that puts it into .git/? Likewise, the access.log and error.log might be better put into .git/gitweb/, no?

Since you have a working setup, though, can you insert debug statements into gitweb.cgi to find out where PATH is unset? Something like

`echo "line 1337: $(date) $PATH" >> C:/mongoose.log`;

should work (this breaks out of Perl, executes shell -- where this particular operation is simpler -- and appends to the file C:/mongoose.log (creating it if it is not yet there). By changing the line numbers accordingly when littering gitweb.cgi with those lines, it should be possible to debug this issue -- the old way.

Thanks!

@dscho
Copy link
Member

dscho commented May 31, 2012

Oops. I spoke too soon. Basically, I gave up installing this last night when my yawns threatened to swallow the laptop. The reason I thought it did not work is my famous impatience. git instaweb always kept on loading in a web browser and did not show anything.

I stupidly thought that instaweb would not search the complete subdirectory structure, so I had no problems testing this on /.git, i.e. msysGit itself. When I just looked at my VM, the page finally loaded!

And I clicked on a revision's ''snapshot'' link and it worked... ;-)

(FWIW I did set git config --system instaweb.httpd mongoose for my experiments.)

@dscho
Copy link
Member

dscho commented May 31, 2012

FWIW I fixed a bug in post-install and pushed it to ''devel'' while in the process of hacking a little on your ''add_mongoose'' branch. Can you have a look at https://github.com/msysgit/msysgit/commits/add_mongoose and in particular at msysgit/msysgit@eaa70e4 and msysgit/msysgit@ce66386? If you like them, you could squash the first into your initial commit and adjust the second after supporting the 'mongoose3' value of ''instaweb.httpd''.

Thank you, this is fun! (And distraction from work... ;-)

@ciaranj
Copy link
Member Author

ciaranj commented May 31, 2012

@dscho thank you for fixing that submodules behaviour, that was biting me a little (I forgot on numerous occasions to make sure I was 'up-to-date', and would inherit that in a commit)

In terms of msysgit/msysgit@ce66386 I already have that covered in ciaranj/msysgit@3659460 but I can cherry-pick yours across if you'd prefer?

About

`echo "line 1337: $(date) $PATH" >> C:/mongoose.log`;

This could be the answer I needed, I've been struggling getting output of the script to do this kind of diagnostic, made figuring stuff out pretty painful, I'll report back :)

@dscho
Copy link
Member

dscho commented May 31, 2012

@ciaranj you're welcome! (I like helping people who help me, and I did not know about Mongoose, that was a pleasant surprise, especially since it is so neatly small.)

As to your commit changing etc/gitconfig, I missed that, sorry. Just keep yours.

Thanks!

@ciaranj
Copy link
Member Author

ciaranj commented May 31, 2012

Ok, so I think I'm making progress, but very slowly :(
The following code at the start of gitweb.cgi (at line 23) :

my $key;
foreach $key (sort keys(%ENV)) {
    `echo "$key :: $ENV{$key}" >> C:/mongoose.log`; 
}

Then the output I see in the mongoose.log contains (amongst other lines) :

GATEWAY_INTERFACE :: CGI/1.1
GITWEB_CONFIG :: C:/msysgit/.git/gitweb/gitweb_config.perl
GIT_DIR :: C:/msysgit/.git
GIT_EXEC_PATH :: C:\msysgit/libexec/git-core
.....
HTTP_COOKIE :: gitweb_tz=utc
HTTP_HOST :: 127.0.0.1:1234
HTTP_USER_AGENT :: Mozilla/5.0 (Windows NT 5.2) AppleWebKit/536.5 (KHTML, like Gecko) Chrome/19.0.1084.52 Safari/536.5
PATH ::
PATH_TRANSLATED :: C:/msysgit/share/gitweb\gitweb.cgi
REDIRECT_STATUS :: 200
REMOTE_ADDR :: 127.0.0.1
REMOTE_PORT :: 3216
REQUEST_METHOD :: GET

So, The GITWEB_CONFIG is being passed from instaweb, GATEWAY_INTERFACE, REMOTE_PORT etc. clearly being passed from mongoose, but the PATH is blank!!

Off I pop to mongoose.c, specifically to prepare_cgi_environment(...) . If I re-compile mongoose to log heavily, and add a debug statement in at line 2873 (the next 2 lines are contextual) along the lines of:

if ((s = getenv("path")) != NULL)
    addenv(blk, "PATH=%s", s);
DEBUG_TRACE(("path: %s", getenv("path")));

It dumps out (correctly) the full path I expect, which suggest to me/puzzles me that somewhere the Path that has been added to that environment block has been stripped, but I can't see that in the code :( any ideas? anyone ? :)

@dscho
Copy link
Member

dscho commented May 31, 2012

@ciaranj I see that the 'path' tag is inconsistently lower-case and upper-case. Maybe that is the issue?

@ciaranj
Copy link
Member Author

ciaranj commented Jun 1, 2012

@dscho lunch hour progress ;) If I hack mongoose.c#prepare_cgi_environment so that;
a) I change line 2872 to be

addenv(blk, "PATH=%s", "c:/msysgit/mingw/bin");

and

b) Ignore the passed in path from the instaweb created config:

  // Add user-specified variables
  s = conn->ctx->config[CGI_ENVIRONMENT];
  while ((s = next_option(s, &var_vec, NULL)) != NULL) {
    if( !(var_vec.ptr[0] == 'P' && var_vec.ptr[1] == 'A') ) {
        addenv(blk, "%.*s", var_vec.len, var_vec.ptr);
    }
  }

Then it works ! ;) .. Just need to figure out the right change to make, ...if I only 'need' the latter, then I can just change instaweb not to specify a path at all to the interpreter (it will inherit mongoose.exe' which for our purpose is correct [I think] )

@dscho
Copy link
Member

dscho commented Jun 1, 2012

@ciaranj Oh, so instaweb's config sets the PATH?

@ciaranj
Copy link
Member Author

ciaranj commented Jun 1, 2012

@dscho Yes, it passes in various settings relating to GIT_WEB ... the end result of mongoose' setting up of the CGI environment is actually 2 PATH entries ... which (it seems) the perl gets (at some point) resolved to a single blank PATH entry ! :)

@dscho
Copy link
Member

dscho commented Jun 1, 2012

I see. My mongoose.conf has this line:

cgi_environment PATH=/usr/libexec/git-core:/usr/bin:/c/Users/VirtualBox/bin:.:/usr/local/bin:/usr/mingw/bin:/usr/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/,GIT_DIR=C:/msysgit/.git,GIT_EXEC_PATH=C:\msysgit/libexec/git-core,GITWEB_CONFIG=C:/msysgit/.git/gitweb/gitweb_config.perl

@ciaranj how does yours look like?

@ciaranj
Copy link
Member Author

ciaranj commented Jun 12, 2012

@dscho Sorry man I've been outta the country, I'll get back on this tomorrow :)

@dscho
Copy link
Member

dscho commented Jun 13, 2012

@ciaranj no worries. I'm outta the country myself ;-)

@dscho
Copy link
Member

dscho commented Dec 30, 2013

For people interested to pick up the ball: https://github.com/msysgit/msysgit/compare/add_mongoose

@dscho dscho closed this as completed Dec 30, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants