From: Omar Polo Subject: Re: -portable gotd progress To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Sun, 20 Aug 2023 21:32:45 +0200 On 2023/08/20 14:28:55 +0200, Stefan Sperling wrote: > The following patches provide some progress for gotd in -portable. > The patches are based on the ta/gotd branch, not the 'portable' branch! > > I am restoring chroot support where possible. This requires the main > process to keep running as root. But that is probably better than not > chrooting. > > A blocking issue I found during testing is that 'got send' never > exits after uploading the pack file. On the server side I see gotsh > hanging in poll on stdin, still waiting for more pack file data to > arrive. Any ideas how that can be fixed? We seem to be relying on > some OpenBSD-specific behaviour on the server side? > > To test the changes, add a gotd user to the system, create an empty > directory at /var/run/gotd, put gotsh and gitwrapper in /usr/bin > with appropriate symlinks for gitwrapper. > > Try a gotd.conf such as: > [[[ > user gotd > > repository "test" { > path "/git/test.git" > permit rw stsp > } > ]]] > > I don't want to commit the debug diff (#4). OK for any others? it reads fine to me; haven't been able to test yet. > Should I put patches on ta/gotd or do we create a separate branch? (no comment on this) > Diffs #1 and #2 could probably go directly to the 'portable' branch? IMHO yes. > ----------------------------------------------- > commit fdd7ffc98a9c9a26d83d115d398c1cd1e609e7cd > from: Stefan Sperling > date: Sun Aug 20 10:32:34 2023 UTC > > make sure program_invocation_short_name gets defined in getprogname.c > > This is needed for gitwrapper which will fail if its program name > cannot be detected properly. > > diff 497c206b2e1425e9a667723222a167d4f3b1dae6 fdd7ffc98a9c9a26d83d115d398c1cd1e609e7cd > commit - 497c206b2e1425e9a667723222a167d4f3b1dae6 > commit + fdd7ffc98a9c9a26d83d115d398c1cd1e609e7cd > blob - 1dc2f493577b073fa65b7544928ff6ff61c1c2e6 > blob + 975dd4f84e4975bf7c53aaa058770a333a96cfd4 > --- compat/getprogname.c > +++ compat/getprogname.c > @@ -14,10 +14,15 @@ > * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > +#define _GNU_SOURCE > + > #include > > #include > +#include > > +#include "got_compat.h" > + I'd avoid explicitly defining _GNU_SOURCE and instead move the include of got_compat.h so it's the first header. we're already using AC_USE_SYSTEM_EXTENSIONS which defines _GNU_SOURCE at the top of got_compat.h on GNU systems, so we're already covered. (We have many files that don't include got_compat.h and not all have got_compat.h at the very start, but this is a separate issue.) also, is argp.h standard? Don't see it on OpenBSD. We should check its presence in configure.ac (add an entry to the AC_CHECK_HEADERS macro) and guard the include with #ifdef HAVE_ARGP_H or something along these lines. The manpage of program_invocation_short_name(3) on GNU/Linux only lists errno.h as required header moreover. > #if defined(HAVE_PROGRAM_INVOCATION_SHORT_NAME) > const char * > getprogname(void) > > ----------------------------------------------- > commit 13c8c4fddb5a359fdfb781028cfaa3529ae65043 > from: Stefan Sperling > date: Sun Aug 20 10:32:34 2023 UTC > > use a workng git libexec directory default on Linux > > Tested on ubuntu 22.04 > > diff fdd7ffc98a9c9a26d83d115d398c1cd1e609e7cd 13c8c4fddb5a359fdfb781028cfaa3529ae65043 > commit - fdd7ffc98a9c9a26d83d115d398c1cd1e609e7cd > commit + 13c8c4fddb5a359fdfb781028cfaa3529ae65043 > blob - 2b1255118b91d4c7edef0a6016130ed9de07bfcd > blob + af3cf15215d61e94a762d704ddca0ecf37080994 > --- gitwrapper/gitwrapper.c > +++ gitwrapper/gitwrapper.c > @@ -43,8 +43,12 @@ > #include "log.h" > > #ifndef GITWRAPPER_GIT_LIBEXEC_DIR > +#ifdef __linux__ > +#define GITWRAPPER_GIT_LIBEXEC_DIR "/usr/lib/git-core/" > +#else > #define GITWRAPPER_GIT_LIBEXEC_DIR "/usr/local/libexec/git" > #endif > +#endif not to block progress but these should also not be hardcoded because 1. it's ugly to #ifdef __linux__; this seems to affect other systems as well 2. different systems/distro install different things in different places. (I won't be surprised if DragonflyBSD has git in /usr/bin for example.) so this could be fine for a first stab, but would then need some work to allow changing these paths at configure-time (at least.) One way would be to detect at configure time where the git libexecs are installed and provide a --with flag to override the automatic guessing game. > #ifndef GITWRAPPER_MY_SERVER_PROG > #define GITWRAPPER_MY_SERVER_PROG "gotsh" > > ----------------------------------------------- > commit bf9a5775e4bca9539f7e1064a16fe20f611028ef > from: Stefan Sperling > date: Sun Aug 20 10:32:34 2023 UTC > > re-enable chroot in -portable gotd > > Reads (git clone) are working but writes (git push) run into an error > > diff 13c8c4fddb5a359fdfb781028cfaa3529ae65043 bf9a5775e4bca9539f7e1064a16fe20f611028ef > commit - 13c8c4fddb5a359fdfb781028cfaa3529ae65043 > commit + bf9a5775e4bca9539f7e1064a16fe20f611028ef > blob - d847e923a6c4676e526a8171f6813cb9686f9698 > blob + 4fd361f420a38ab5a9dfe6a68516b93b5d1f9d2c > [...] > --- gotd/gotd.h > +++ gotd/gotd.h > @@ -20,7 +20,11 @@ > #define GOTD_UNIX_SOCKET_BACKLOG 10 > #define GOTD_USER "_gotd" > #define GOTD_CONF_PATH "/etc/gotd.conf" > +#ifdef __linux__ > +#define GOTD_EMPTY_PATH "/var/run/gotd" > +#else > #define GOTD_EMPTY_PATH "/var/empty" > +#endif same thing here, these should be changeable from the configure script IMHO. It's fine for me if you want to land these as-is, I can add the autoconf bits later (assuming thomas agrees.) > #define GOTD_MAXCLIENTS 1024 > #define GOTD_MAX_CONN_PER_UID 4 > [...]