pkgxdev / libpkgx

`import`… but with pkging powers
https://npmjs.com/libpkgx
Apache License 2.0
62 stars 12 forks source link

{{deps}} moustaches should work in runtime.env: keys #24

Closed jhheider closed 1 year ago

jhheider commented 1 year ago

https://github.com/teaxyz/pantry/actions/runs/4581371758/jobs/8090857190

For now, I'll just export this from ca-certs, so git will work.

mxcl commented 1 year ago

Unfortunately, there's plenty of places where the {{moustaches}} aren't fully hydrated. It mostly depends on what data the function has to hand.

We are taking a “wait for a user to report the bug” approach since otherwise you can waste a bunch of time examining all cases for scenarios that never actually show up in real life.

I think here the information about what dependencies a specific package has is lost. However we could just pass all the packages in the fully hydrated dependency graph. It's more than needed but that does include the deps.

jhheider commented 1 year ago

Since we don't hydrate multiple versions currently (right? just conflict them out?), all deps should have a unique identity for a given dep.

Possibly we could hydrate this envvar using $SSL_CERT_FILE, but we'd have to know that would populate first...

mxcl commented 1 year ago

Since we don't hydrate multiple versions currently (right?)

Yes this is correct. So far we haven't needed more granularity than that.

Possibly we could hydrate this envvar using $SSL_CERT_FILE, but we'd have to know that would populate first...

I added env.FOO to the moustaches recently so that should work. Though I recall humming and harring about whether this was a good idea. So maybe I didn't and only added env.HOME.

jhheider commented 1 year ago

It seems versatile, but we should aspire to never need arbitrary env vars. Unless/until we want optional build features; then we're recreating emerge.

mxcl commented 1 year ago

indeed. This was my reasoning and you spelling it out makes me think that yeah it's a bad call. Let me go check what I did.

jhheider commented 1 year ago

teaxyz/teash

mxcl commented 1 year ago

I just tried GIT_SSL_CAINFO: ${{deps.curl.se/ca-certs.prefix}}/ssl/cert.pem in git’s package.yml and it works…

Do you have any concrete example of this not working?

edit: potentially wouldn’t work for transitive deps…?

jhheider commented 1 year ago

That sounds like the use case I needed. Transitive sounds like you'd be doing it wrong anyway; should be explicit.

mxcl commented 1 year ago

my bad, I was misreading the output, let me see why it doesn't work