-
Notifications
You must be signed in to change notification settings - Fork 76
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
Grab proot from bootstrap zip rather than including its nix path #393
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,5 @@ | |||
# WARNING: This file is autogenerated by the deploy script. Any changes will be overridden | |||
{ | |||
url = "file:///data/local/tmp/n-o-d/bootstrap-aarch64.zip"; |
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.
I presume, the committed version will have my bootstrap url here instead?
Isn't there a dependency loop, BTW, with the bootstrap zipball containing the hash of itself?
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.
"will have my bootstrap url here instead" yes
"isn't there a dependency loop"
No, two things:
- the hash is only a hash of the proot binary
- during bootstrap building prootStatic is overridden, so this is never evaluated
6f38e94
to
b107a19
Compare
updated to fix formatting |
scripts/deploy.sh
Outdated
cat >$new_attrs_file <<EOF | ||
# WARNING: This file is autogenerated by the deploy script. Any changes will be overridden | ||
{ | ||
url = "$NIX_ON_DROID_BASE_URL/bootstrap-$arch.zip"; |
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 lacks a branch (github:owner/repo/branch case, see above)
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.
No, wait, it generally isn't inferrable. The interface will have to be extended to pass the public url of the zip tarball as well =(
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.
The use case I want covered is, basically, someuser
cloning nix-on-droid, creating a branch, changing it and then wanting to test it without adb and with a HTTP server. while we know where to pull the github source tarball from if it's on github (and non-github users assumed to know what they're doing), we don't know what URL will they place the bootstrap tarball at. previously that didn't matter to us, as long as they enter it correctly into the app; now it begins to matter
scripts/deploy.sh
Outdated
cat $attrs_file | ||
echo ">>>>>>" | ||
log "adding $attrs_file to git index" | ||
git add $attrs_file |
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.
I don't think we should.
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.
I think we have to, otherwise it's not included in the flake. I agree it's less than ideal but I don't see an obvious alternative.
And what's the other impurity source? |
|
082fe84
to
a18149c
Compare
I've made one reading pass through it, I am willing to explore that direction in general, but I'm afraid I'll make a million little opinionated edits over the course of the next ~10 days. |
Opinionated edits aren't necesarily bad, I believe I checked the box so that you can directly push changes to the branch if you want to. |
So far the only global consideration I've found for my workflows is that I can't update proot in between releases, lest we'll have master commits using a version of proot with no stable-path zipball to back them. |
I don't know what you want from me on this one. If you do it anyways it's no worse off than before (sometimes people will need to have the cachix substituter), or you can follow a nightly-style filename format (eg |
…ctly. This means that the cachix substituter (or already having the package in your nix store somehow) is no longer required to build. This required reworking the deploy script. As a bonus you can now omit the second argument and it will tell you what it would have copied instead of copying anything. This is fixes one source of impurity, but for now flake builds will still require the --impure flag
a18149c
to
c5324bc
Compare
This means that the cachix substituter (or already having the package in your nix store somehow) is no longer required to build.
This required reworking the deploy script. As a bonus you can now omit the second argument and it will tell you what it would have copied instead of copying anything.
This is fixes one source of impurity, but for now flake builds will still require the --impure flag