-
Notifications
You must be signed in to change notification settings - Fork 17
pkg build
fixes
#188
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
pkg build
fixes
#188
Conversation
libexec/fixup.ts
Outdated
@@ -20,31 +20,31 @@ const pkg_prefix = new Path(unknown[0]) | |||
|
|||
switch (host().platform) { | |||
case 'darwin': { | |||
const { success } = await Deno.run({ | |||
cmd: [ | |||
const { output } = new Deno.Command(Deno.execPath(), { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be fix-machos.rb
.
It's interesting this works, I guess that deno re-execs shebangs? To be clear this runs the ruby script through deno
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry not following, what should be fix-machos.rb?
fix-machos.rb is run on line 25 no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are executing deno
, this is what Deno.execPath()
returns. Replace it with fix-machos.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see.
Couldn't tell in deno.run docs if line 25 and 26 are args or not but saw my mistake with the actual command. Built a package with this and it completely successfully
share/brewkit/fix-machos.rb
Outdated
@@ -1,8 +1,6 @@ | |||
#!/usr/bin/env ruby | |||
#!pkgx +gem ruby |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be #!/usr/bin/env -S pkgx +gem ruby
. Shebangs have to be full paths if you are running them directly which with the above change will be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol. you beat me by seconds.
const { success } = await Deno.run({ | ||
cmd: [ | ||
'fix-machos.rb', | ||
const { output } = new Deno.Command('fix-machos.rb', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn’t run the command: it instantiates it and then exits.
It's worrying that CI passes
I think this PR is not an actual fix for your issues. I am closing it. We need to look deeper into why you are having issues. Rationale: the only reason it seems like it fixes it is because actually you are no longer running |
based on the knowledge shared here: #185 (comment)
I was able to get pantry packages building with the couple small changes here.
Using shebang #!pkgx +gem ruby did not initially fix the errors I was seeing. I then updated the deprecated deno.run commands being used and that together with the new shebang brought success.