-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
git, gitweb: Fix git-instaweb #53494
Conversation
* Make the build system embed the correct path to gitweb into git-instaweb * Move gitweb fixups to the git expression, to make sure that gitweb used by git-instaweb is functional * This will increase the closure size of git, but only with perlSupport
Oh, ok, never mind. Skipped tests and it doesn’t work:
Not sure what to do now. Probably unsplit |
This partially reverts 9029ed9 as `git-instaweb`, which comes with git, needs on gitweb and having them in separate outputs results in a cycle.
For reference, I partially reverted #38918. |
@kirelagin sorry, i don't use gitweb atm. Feel free to do whatever you find an appropriate. |
Since you're going from not working to working, I think you can merge if it works and if closure size is not impacted overly much. |
@kirelagin from your perspective what is the status on this? |
I don’t see any reasons not to merge it. |
@wmertens @peti @the-kenny final approval before merging please? |
Thanks, looking forward to trying it out! |
Motivation for this change
Git’s build system does two things with the
gitwebdir
variable:git-instaweb
Before this change
git-instaweb
wouldn’t work as it was looking for gitweb in git’sshare
directory, but it was moved away into a separate output by the nix expression.This PR sets the
gitwebdir
variable in the Makefile and points it to thegitweb
output. As a result, a path to this output is now embedded intogit-instaweb
, which will affect the closure size, but, AFAIU, only ifperlSupport
is enabled, so this is probably not a problem? I also had to move the fixups from thegitweb
expression togit
itself to make sure thatgitweb.cgi
that is referenced fromgit-instaweb
is functional.Fixes #53491.
@gnidorah @peti @the-kenny @wmertens
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)git
as its tests take too long, but I am relatively confident that it works as its tests were running successfully for quite a while.git instaweb
now worksgitweb
(asgit
never finished building), but I don’t think this change could break something in it.