git, gitweb: Fix git-instaweb #53494
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Git’s build system does two things with the
gitwebdirvariable:git-instawebBefore this change
git-instawebwouldn’t work as it was looking for gitweb in git’ssharedirectory, but it was moved away into a separate output by the nix expression.This PR sets the
gitwebdirvariable in the Makefile and points it to thegitweboutput. As a result, a path to this output is now embedded intogit-instaweb, which will affect the closure size, but, AFAIU, only ifperlSupportis enabled, so this is probably not a problem? I also had to move the fixups from thegitwebexpression togititself to make sure thatgitweb.cgithat is referenced fromgit-instawebis functional.Fixes #53491.
@gnidorah @peti @the-kenny @wmertens
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)gitas its tests take too long, but I am relatively confident that it works as its tests were running successfully for quite a while.git instawebnow worksgitweb(asgitnever finished building), but I don’t think this change could break something in it.