OpenGen / GenSQL.query

42 stars 5 forks source link

build: enable opt-in readonly deps #104

Closed ships closed 6 months ago

ships commented 6 months ago

This PR splits the current, strict devshell experience (depsCache is set to a readonly nix-store directory, and is therefore very stable, but harder to iterate on) to .#strict devshell; the default shell, used with direnv, still downloads the existing deps but sets maven and clojure to use a temp directory, and copies the deps directly into that location.

Along the way, two fixes are included:

  1. use MAVEN_OPTS instead of JAVA_TOOL_OPTS user.home, becuase it is more exact.
  2. stop using the tag reference for test-runner dependency, which was breaking the use of depsCache for unknown reasons.

@KingMob , (2) implies that in the short term, it will be necessary to refer by full sha instead of tag and short sha to git dependencies. I am unclear why this is the case, as it seems related but not identical to https://github.com/jlesquembre/clj-nix/issues/88 . I hope you are okay with this adaptation in the short term while we figure out the cause of that behavior; please share any concerns here.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.60%. Comparing base (1e7e2dd) to head (011f3ff).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #104 +/- ## ======================================= Coverage 75.60% 75.60% ======================================= Files 30 30 Lines 1496 1496 Branches 64 64 ======================================= Hits 1131 1131 Misses 301 301 Partials 64 64 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KingMob commented 6 months ago

I'm not sure why use of the git tag was breaking anything, but if you look here, it explains what :git/tag is doing. It's optional, but if you don't use one, you have to use the full SHA and not a shortened one. And if you do add the tag, it has to resolve to the same commit as the SHA. (The whole situation is awkward, but I'm not sure how best to improve it. Maybe allow tag-less commits to still match, if the first ancestor commit with a tag is the right tag? Dunno.)

Perhaps the deps cache issue was tag/SHA mismatch, or upgrading to commits that lacked a tag?

KingMob commented 6 months ago

I'm confused about the switch to MAVEN_OPTS; afaik, Clojure doesn't use Maven to run/build Clojure code (tho it uses it internally to build the compiler itself). I could be wrong, though.

Does MAVEN_OPTS work in and of itself, or is the value in no longer using JAVA_TOOL_OPTS?

ships commented 6 months ago

The only Java-related thing present in the deps cache is the maven .m2 cache. We used to set -Duser.home java option because it will use this setting to find the maven cache in absence of more specific MAVEN setting. It is probably a negligible functional change, but this is more precise and expressive, and prevents a user who calls java in the shell who needs a writablee user.home from having unrelated problems and having to find this solution later.

KingMob commented 6 months ago

We used to set -Duser.home java option because it will use this setting to find the maven cache in absence of more specific MAVEN setting.

I'm still confused here. Does the java binary pay attention to Maven env vars? I assumed the m2 libs are found by adding them to the classpath, and that java doesn't care, or know, about Maven otherwise.

ships commented 6 months ago

You're right, and I noticed at least one setting where running clj plainly in the devShell tried to write to the java user.home . So i am inclined to resume that setting.