Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

+pkgm #8419

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

+pkgm #8419

wants to merge 2 commits into from

Conversation

jhheider
Copy link
Contributor

closes #8405

 closes #8405
@jhheider
Copy link
Contributor Author

@mxcl odd. i wonder if this is because both v1 and v2 are present?

  ▌ pantry script start
+ cd /Users/jacob/pkgx/pantry/testbeds/pkgx.sh__pkgm-0.3.0
+ pkgm --version
pkgm 0.3.0
+ pkgm install dua
┏ ⚠️  Deno requests run access to "pkgx".
┠─ Requested by `Deno.Command().spawn()` API.
┠─ To see a stack trace for this prompt, set the DENO_TRACE_PERMISSIONS environmental variable.
┠─ Learn more at: https://docs.deno.com/go/--allow-run
┠─ Run again with --allow-run to bypass this prompt.
┗ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all run permissions) > 

^local. on CI it just fails.

@jhheider
Copy link
Contributor Author

adding:

  - sed -i 's|--allow-run=pkgx,/usr/bin/sudo|--allow-run|' pkgm

"fixes" that, but then i get:

  ▌ pantry script start
+ cd /Users/jacob/pkgx/pantry/testbeds/pkgx.sh__pkgm-0.3.0
++ pkgm --version
+ test 'pkgm 0.3.0' = 'pkgm 0.3.0'
+ pkgm install dua
 × usage error: no such arg: --json
error: Uncaught (in promise) Error: UR TEST FAILED WITH CODE 1 & SIGNAL null
if (!rv.success) throw new Error(`UR TEST FAILED WITH CODE ${rv.code} & SIGNAL ${rv.signal}`)
                       ^
    at file:///Users/jacob/.pkgx/pkgx.sh/brewkit/v1.14.3/bin/bk-test:106:24
    at eventLoopTick (ext:core/01_core.js:182:7)

which means it's finding v1 first. so... maybe this can't be done until we move brewkit to v2?

@felipecrs
Copy link
Contributor

felipecrs commented Jan 15, 2025

Maybe we should deno compile pkgm.ts during build time to avoid a hard runtime dependency on pkgx?

@jhheider
Copy link
Contributor Author

possibly, but a) deno compile binaries are huge, and b) it requires pkgx to functional in any case.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 15, 2025

I see. Maybe you should remove pkgx from the list of dependencies? There's no way this will be executed without pkgx installed in the system anyway, lol.

@mxcl
Copy link
Member

mxcl commented Jan 15, 2025

there's a bug somewhere if it finds v1 when the package depends on v2.

@jhheider jhheider force-pushed the +pkgm branch 4 times, most recently from a612c55 to 91bec03 Compare January 15, 2025 20:02
@jhheider
Copy link
Contributor Author

that's... what it seems like, even though path inspection shows v2. and the darwin version gets can't spawn. so, maybe it just needs to be on-hold.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 15, 2025

What's the benefit of keeping pkgx in the dependencies list? It would trigger pkgx to redundantly install another pkgx. Also, if users installed pkgx in their machine, they would expect pkgm to reuse that installation. It doesn't make sense to break that expectation, does it?

@jhheider
Copy link
Contributor Author

it strictly requires pkgx^2. and may, in the future, strictly require higher versions. so, same benefits of mapping dependencies.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 15, 2025

Regardless of which version of pkgx pkgm requires, I would expect pkgm to use the version of pkgx which I installed to my system, even if it is not compatible and fails.

Maybe pkgm should be an exception, as it manages stuff installed at system level, not at pkgx dir level.

Well, maybe I'm going too crazy. lol

@felipecrs
Copy link
Contributor

Maybe this is like mvn, which I would expect to use my system's Java, but instead it downloads its own Java.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 16, 2025

Maybe my concern will be "fixed" by: pkgxdev/pkgx#971 (comment)

I.e. would allow my system installed pkgx to satisfy the (companion) dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

+pkgm
3 participants