pgrx icon indicating copy to clipboard operation
pgrx copied to clipboard

Move unnecessary deps out of critical build path

Open workingjubilee opened this issue 2 years ago • 2 comments

The pgx-utils crate, or more specifically the SQL generator logic inside it, is a "core" dependency of PGX. This means that it forms part of the critical path to get PGX building in any case, and everything not part of that critical path would ideally be set aside in another crate, or perhaps feature-flagged out of default existence. The main concern is making sure it doesn't expand the dependency graph (as reckoned by Cargo.toml and Cargo.lock, mostly) when it isn't needed. Perhaps a new pgx-sqlgen crate or something similar is called for?

I glanced at the code in pgx-utils/src/pg_config.rs and spotted mod rss, which doesn't seem essential to running the SQL generator if PGX is being used in an offline-centric manner. It seems to be mostly used to provide a more friendly default, which is valuable, but optional.

https://github.com/tcdi/pgx/blob/1a89778046547c52c714fe6885069eab5638f965/pgx-utils/src/pg_config.rs#L413-L423

Any other cases we can move dependencies towards leaves in the build tree without loss of maintainability would also be good, if for no other reason than the sake of build times.

workingjubilee avatar Jun 20 '22 20:06 workingjubilee

Aside: Concerning build times in particular, and to a lesser extent dependency resolution, the reverse of this is also true. Anything that is very well-populated throughout the dependency tree may as well be deep in the trunk of the build. There are ways to arrange for that using "workspace hack" crates, however, that are less... "principled", but also more effective for pure deps manipulation.

workingjubilee avatar Jun 20 '22 21:06 workingjubilee

#607 moved the rss module into cargo pgx, since that was its only use site, but I am not yet satisfied that we have accounted for everything else in pgx-utils that could be removed.

workingjubilee avatar Jul 15 '22 13:07 workingjubilee

@workingjubilee Should this be closed as a result of https://github.com/tcdi/pgx/pull/645 being merged? Or is this going to be a placeholder for possible other dependencies that could be removed in the future?

BradyBonnette avatar Aug 31 '22 17:08 BradyBonnette

I think it's done! Thank you!

workingjubilee avatar Sep 02 '22 05:09 workingjubilee