search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
16.46k stars 12.95k forks source link

gogs: Non-NixOS installations need manual rewriting of git hooks on upgrade #30048

Open knedlsepp opened 6 years ago

knedlsepp commented 6 years ago

Issue description

During regular usage gogs creates git hooks that look like this:

#!/nix/store/p4csf5alczckcj6j4byc211kpw6ir0kz-coreutils-8.26/bin/env bash
"/nix/store/dbx6h0sa6z2jzd7jw2kyp5hygylhlv7p-gogs-0.9.113-bin/bin/gogs" update $1 $2 $3 --config=/home/gogs/custom/conf/app.ini'

This hard-coding of the installation directory of course leads to problems on an upgrade. While on NixOS this is solved by the gogs service module as discussed here, upgrading gogs on Non-NixOS means one manually has to run "Rewrite all update hooks of repositories (needed when custom config path is changed)" in the admin-Dashboard.

Technical details

bobvanderlinden commented 6 years ago

This is where the hooks are defined: https://github.com/gogits/gogs/blob/643c85e9c8fd237912652d14a0535dfddf614d2b/models/repo.go#L774-L776

This is where the hook patterns are filled in: https://github.com/gogits/gogs/blob/643c85e9c8fd237912652d14a0535dfddf614d2b/models/repo.go#L783

You can see it will use setting.AppPath, which in turn is the executable path: https://github.com/gogits/gogs/blob/ab2c6cb00870991c33a7e7c7a22c47db1f48a9bb/pkg/setting/setting.go#L340

Ssh needs to know what gogs to run, gogs will set its own executable path in hooks when it initializes repositories. Maybe a symlink of the gogs executable should be placed in the data-dir and have the hooks refer to that executable. This will probably require changes to gogs: using a different path compared to AppPath. And creating a symlink from the wrapper script. That last step has shown its downsides.

I'm not sure how to solve this properly for ad-hoc running of gogs.

I think recreating the hooks might be a good-ish option, but it'll require a cmdline option to do that.

orivej commented 6 years ago

Looking at those links it can be seen that the current version of gogs already does what we want: it writes the path that was used to launch it (rather than, say, os.Executable(), which effectively resolves symlinks). When I launched gogs installed with nix-env, it used a stable path /home/user/.nix-profile/bin/gogs.

bobvanderlinden commented 6 years ago

That's a very keen observation. So the only next addition would be to have a cli version of 'rewrite all hooks'. I guess that's an issue for gogs and not so much for nixos.

orivej commented 6 years ago

This is a one time operation (assuming a single gogs installation), so a cli version won't be very useful.

However, looking at git blame it seems that gogs behaved like that for a long time. @knedlsepp , could you launch gogs in a new directory, create a repository and see if hooks point to a nix profile or to /nix/store?

knedlsepp commented 6 years ago

Yes, it seems that gogs installed via nix-env -i actually uses the ~/.nix-profile link in the hooks, just as you predicted. Currently I however have a nix-installed wrapper for gogs on openSUSE that I basically use as a poor man's daemon which restarts it on failure. As this uses ${pkgs.gogs} as a dependency I get the /nix/store/... path in the hooks instead. I initially also did this to prevent myself from running gogs directly (creating all those symlinks in the current directory), and to enforce using my wrapper that cds to the right directory. While the symlink issue is gone, I still don't want to depend on it being installed in the ~/.nix-profile and want it to work when started using nix-shell -p gogs of via a wrapper-script that refers to ${pkgs.gogs}.

What would be the downsides of moving the sed based rewriting currently implemented in the NixOS module into the wrapper? (It would at least need some adapting, because it currently doesn't rewrite directories of the ~/.nix-profile kind)

orivej commented 6 years ago

This can be done in the wrapper, with care not to rewrite hooks when Gogs is called from the hooks. However, patching Gogs to always rewrite hooks when it starts the web server seems better.

knedlsepp commented 6 years ago

I made a PR to gogs that adds the gogs admin resync-hooks subcommand: https://github.com/gogits/gogs/pull/4813

bobvanderlinden commented 6 years ago

Yay for getting it merged! Now we wait for a next release...

stale[bot] commented 4 years ago

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.