Stefan Sperling <stsp@stsp.name> wrote:
> On Tue, Jul 05, 2022 at 09:55:51AM +0200, Omar Polo wrote:
> > agreed.  if we want to have the libexec helpers even more small maybe we
> > could split privsep.c into multiple privsep_*.c files, so that the
> > libexec helpers can link only to the bits they actually need?
> 
> Maybe. For now, keeping code that is only used by one libexec helper
> in the .c file of this helper contributes to the end goal of having
> minimal helper binaries in libexec.
> 
> > I did a quick search and just for giggles tried to build got-read-patch
> > with LTO.  It significantly reduces the sizes of the helpers (patch went
> > from 229K to 54K, and once stripped down to 30K) and seemed to strip out
> > all the unused functions, but admittedly is something we can't use.  Was
> > fun to try thought, last time i played with -flto i had clang dying of
> > OOM :)
> > 
> > 	% ls -lah got-read-patch
> > 	-r-xr-xr-x  1 op  op  54.3K Jul  5 09:54 got-read-patch*
> > 	% nm got-read-patch | grep got_privsep
> > 	00006b90 t got_privsep_flush_imsg
> > 	00006610 t got_privsep_recv_imsg
> > 	000069e0 t got_privsep_send_error
> 
> Oh, that is nice. This could help us to see the groups of functions
> needed by each helper. Once we have a grouping, we could move things
> around to ensure that the smallest required subset will be linked,
> regardless of toolchain tricks like LTO.
> 
> It would not hurt to do such cleanup on a semi-regular basis. But we
> can also do such work when most of Got is "done", i.e. before we finally
> ship a 1.0 release that comes with backwards compatibility promises.
> There is little reason to do a lot of such work now, unless you are curious.

well, I'm curious :)

I've built the libexecs with -flto and this is the result of what
symbols are needed by the helpers.  I'm filtering out
got_privsep_flush_imsg / recv_imsg / send_error / send_stop as they're
used almost everywhere and would only add noise to the list:

	==>  got-fetch-pack
	none
	==>  got-index-pack
	0000cc50 t got_privsep_wait_for_child
	==>  got-read-blob
	00006ed0 t got_privsep_send_blob
	==>  got-read-commit
	000075d0 t got_privsep_send_commit
	==>  got-read-gitconfig
	none
	==>  got-read-gotconfig
	none
	==>  got-read-object
	00007290 t got_privsep_send_obj
	00007090 t got_privsep_send_raw_obj
	==>  got-read-pack
	000170c0 t got_privsep_recv_object_idlist
	000167a0 t got_privsep_send_blob
	00016030 t got_privsep_send_commit
	00016eb0 t got_privsep_send_enumerated_commit
	00016c30 t got_privsep_send_enumerated_tree
	00015f30 t got_privsep_send_obj
	00016db0 t got_privsep_send_object_enumeration_done
	00016e30 t got_privsep_send_object_enumeration_incomplete
	00017450 t got_privsep_send_painted_commits
	00017730 t got_privsep_send_painting_commits_done
	00016fa0 t got_privsep_send_raw_delta
	00015dc0 t got_privsep_send_raw_obj
	00017260 t got_privsep_send_reused_deltas
	000173d0 t got_privsep_send_reused_deltas_done
	00016920 t got_privsep_send_tag
	000163c0 t got_privsep_send_tree
	00015640 t got_privsep_wait_for_child
	==>  got-read-patch
	none
	==>  got-read-tag
	000073c0 t got_privsep_send_tag
	==>  got-read-tree
	00007170 t got_privsep_send_tree
	==>  got-send-pack
	none

so at least 5 libexec helpers would benefit from a minimal privsep.c.
Trimming down privsep.c could also help gotweb/gotwebd, the less we put
in an executable reachable from the internet the better.

However, got-read-pack needs a big chunk of privsep.c anyway and it's a
very frequently used one, so trimming down the size of privsep.c helps
there too but not in a particularly significant way.

I might try to split privsep.c into subfiles and see how it goes.