canonical / pebble

Take control of your internal daemons!
GNU General Public License v3.0
137 stars 52 forks source link

fix(cli): pass PebbleDir and SocketPath through RunOptions/ParserOptions/CmdOptions #388

Closed thp-canonical closed 3 months ago

thp-canonical commented 4 months ago

This builds on top of #383, and allows for a custom PebbleDir to be passed to cli.Run() in RunOptions to make sure all parts of the codebase use the passed-in values and not fall back on environment variables.

The default behavior of cli.Run(nil) stays unchanged.


Multiple parts of the codebase currently use getEnvPaths() directly to access PEBBLE and PEBBLE_SOCKET:

With #383, it is possible to pick a different socket path by passing it into cli.Run()'s options. This socket path currently only applies to Pebble client, it does not apply to the daemon part started by pebble run (because of the separate getEnvPaths() in cmd_run.go's runDaemon).

For loadCLIState and saveCLIState (both used by pebble notices and its acknowledgement command pebble okay) this PR makes sure the passed-in socket path is used (otherwise it would default to the environment variable even when a non-nil options value is passed to cli.Run()).

While "Pebble as a server" (pebble run) and "Pebble as a client" can be seen as conceptually different things, both share cli.Run as entry point.

This doesn't affect uses of getEnvPaths (and its exported name GetEnvPaths) in the unit tests.

benhoyt commented 4 months ago

Tests are failing on this one too. :-) It's end of day for me now, but I'll review this tomorrow.

thp-canonical commented 3 months ago

The PR description needs updating once the code changes are approved.