pack.alpha icon indicating copy to clipboard operation
pack.alpha copied to clipboard

Add support for using the CLJ_CONFIG env var.

Open sundbp opened this issue 5 years ago • 10 comments

This is very helpful for e.g. certain monorepo workflows.

sundbp avatar Apr 02 '19 14:04 sundbp

I'm not sure about merging this.

I know some people are (ab)using CLJ_CONFIG for monorepo setups, but I haven't actually found it necessary at all with Edge. If someone actually were using CLJ_CONFIG (eg for development purposes) then this would cause dev dependencies created on their machine to differ from other people's.

I expect @alexmiller will create an alternative solution to solve the monorepo problems more generally in the future.

SevereOverfl0w avatar Apr 02 '19 15:04 SevereOverfl0w

I think there are a few things here:

  • not using CLJ_CONFIG makes it inconsistent with the rest of the tools.deps ecosystem, and hence one gets unexpected behaviour
  • I do (ab)use CLJ_CONFIG to unify versions across a monorepo. I wrap my tasks (e.g. repl, uberjar'ing etc) in a way to ensure consistent and reproducible behaviour (incl CLJ_CONFIG). That seems a reasonable trade-off - I get version consistency, I give up other use of CLJ_CONFIG until there is a better monorepo story around.
  • Edge has version duplication pretty much everywhere, one of the main reasons I don't find it appealing to work with, while I do share a lot of other ideas it uses. I basically use something taking ideas from edge and from seancorfield's CLJ_CONFIG "trick/hack/workaround" for version handling.

As part of this discussion I feel like the first point is the most objective one - inconsistency is never a good thing. I personally then feel the other 2 points relate to the subjective view of allowing users to shoot themselves in the foot and make their own risk/return decisions.

Even if we had some other magical feature for better monorepo support I'd argue it makes sense for consistent use of CLJ_CONFIG to avoid surprises. I'm sure there will be better support for monorepos, but I don't think that makes this PR void.

sundbp avatar Apr 02 '19 17:04 sundbp

I believe that packs behaviour is consistent with -Srepro, which is the intention.

SevereOverfl0w avatar Apr 02 '19 20:04 SevereOverfl0w

It is - but without the user having passed -Srepro asking for it. That choice was taken away from from the user. My patch should be updated to suppress CLJ_CONFIG when -Srepro is indeed passed. Would that be a good trade-off? Or if you really hate using the same semantics leading to it being on by default, making it off by default but adding a pack flag to enable it?

sundbp avatar Apr 02 '19 20:04 sundbp

The thought process is that release artefacts are a sensitive output. Reproducibility is an important property. So factoring in the user deps.edn file is a no no.

I don't have a good idea yet of what I think should happen here. Maybe supporting -Sdeps would resolve this?

Alternatively, maybe this is an opportunity to expose more of an API interface from pack? Then users can layer on as many deps.edn files as they like.

SevereOverfl0w avatar Apr 02 '19 20:04 SevereOverfl0w

I agree reproducibility is an important feature. I enforce mine through a pack script (e.g. I use no pack alias or anything like that at all, it's all in the pack script), which will indeed make my builds reproducible. In my setup the CLJ_CONFIG is not a user file, it's a monorepo file under control of the monorepo. -Sdeps is in principle an escape hatch, but it wont be smooth as it'd require e.g. reading in a deps.edn file and pass via -Sdeps (with potential quoting issues etc). Some similar in spirit flag to indeed add in "any" deps.edn file would solve my situation.

In general, for clj itself as well, if I had a flag to merge in another arbitrary deps.edn file the monorepo case would "work". I use CLJ_CONFIG as my "extra" deps.edn, and I'd gladly use a different way to get the extra deps.edn into the mix without "borrowing" CLJ_CONFIG to be a monorepo setting.

sundbp avatar Apr 02 '19 21:04 sundbp

Fwiw, quoting isn't too difficult here because bash takes care of that.

Generally, clj -Sdeps "$(< ../extra-deps.edn)" works with clj itself without issue, nothing scarier than standard bash escapes.

SevereOverfl0w avatar Apr 02 '19 21:04 SevereOverfl0w

To make matters worse I need both linux and windows support :/ I still haven't worked out the cmd/powershell stuff properly. But that's encouraging.

A flag to add a deps.edn file feels like it could be a decent way forward. I'll mention it to Alex and see if it's something considered or worthy of consideration for clj itself.

sundbp avatar Apr 02 '19 22:04 sundbp

What was the response from Alex regarding a flag for providing a file to clj?

SevereOverfl0w avatar Jul 24 '19 14:07 SevereOverfl0w

https://clojure.atlassian.net/projects/TDEPS/issues/TDEPS-115?filter=allopenissues&orderby=priority%20DESC&keyword=file someone else might have reported this.

SevereOverfl0w avatar Jul 24 '19 14:07 SevereOverfl0w