cleanup qsrelease: Make POSIX, fail on errors #2582
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
n8henrie
commented
Nov 13, 2021
n8henrie
commented
- If errors are acceptable, explicity ignore them.
- Move cleanup to a trap instead of a long conditional
- Add and remove quoting where it can / can't make a difference
- Break long lines for readability
- Enable users to pass in a custom signing identity for local testing
- If errors are acceptable, explicity ignore them. - Move cleanup to a trap instead of a long conditional - Add and remove quoting where it can / can't make a difference - Break long lines for readability - Enable users to pass in a custom signing identity for local testing
n8henrie
changed the title
|
Fixes #2581 |
2 tasks
|
Status: WIP, need review |
|
This looks OK to me so far. My biggest concern initially was the one-liner used for signing, but it looks like that’s been replaced with something a little smarter. Not that signing does anything, but… (I’ve never liked using backslashes to break up long lines, but it’s not like double-quotes in Python. Just leave it. 😀) |
|
Not that signing does anything
Still looking into this -- there are a few SO posts saying that they
couldn't get it working without using a real apple-provided certificate.
Using a self-signed cert, it seems that the `spcl --add` adds the "execute"
privileges (i.e. for an app) but not the `open` privileges (i.e. for a
dmg), (when you look at `spctl --list | grep myLabel`). And `spctl --add`
doesn't accept a `--type open` flag like `spctl --assess` does, which is
frustrating.
I’ve never liked using backslashes to break up long lines
Me neither, which is why I end all I can in `|` instead (haha). I do much
of my coding on pretty small screens with way too many tmux panes open, so
I wrap pretty aggressively. Glad it looks okay-ish.
I imagine performance may be better if using `codesign --force --continue
--sign "FOO" "${args_in_proper_order[@]}"` but was trying to stick with
POSIX compliant (I've never used zsh, always stick with bash since it comes
with basically all flavors of linux). Could also be faster with e.g. `find
stuff | xargs codesign` but then I feel like the inside-out order may not
be reproducible.
Also trying to make sure these work identically with the MacOS BSD tooling
(`/usr/bin/{awk,sed,grep}`) as well as gnu tools from homebrew (which I
imagine many others, like me, have first on their `$PATH`)
Can you pull this locally and see if (by change) spctl has any better of an
opinion using an official cert?
Sorry the markdown isn't going to render since this is an email, hope it's
still readable.
|
|
Wait, what!? $ spctl -avv Quicksilver.app
Quicksilver.app: accepted
source=Developer ID
origin=Developer ID Application: Rob McBroom (ATVCND8EAP)
$ spctl -avt open --context context:primary-signature 'Quicksilver 1.6.1.dmg'
Quicksilver 1.6.1.dmg: rejected
source=Unnotarized Developer IDSo now the app is OK, but not the DMG? I mean, if I had to pick one to work, it would be the app. I wonder why the DMG is suddenly unhappy. I’ll mess with it a bit. |
|
Yup -- sorry, I thought that's what you'd been seeing as well. Didn't read carefully enough.
You're probably well aware, but to simulate the error / scary warning on opening you can run a local web server (`python3 -m http.server`) and download from yourself and then open the "downloaded" dmg.
|
Add comments to give examples for those using self-signed certs.
|
Instead of the big |
Closed
|
I’m trying this out, but something is making |
|
All of the `No space left on device` -- I assume that's referring to the dmg volume and not your filesystem?
|
|
Until we can work out #2576 I've just bypassed the codesigning and spctl checks with |
Yes, it’s the DMG. I have 2.82 TB free on the actual disk. I’ll try to do some trial and error to identify the meaningful differences between the old and new scripts’ environments. |
skurfer
added a commit
that referenced
this pull request
Dec 6, 2021
|
I did some poking around and found that most of the app does get copied to the temporary volume. The help for Anyway, adding Are we keeping this PR, or do you want to close it and just work from #2587? |
|
Are we keeping this PR, or do you want to close it and just work from
#2587 <#2587>?
If they are both looking ok it doesn't matter much, I had them split so
that this one could be merged while we investigate if the signing is going
to work or not. Some of the notes here might be nice to have but are
referenced from the other issue and PR, so I don't see a good reason to
prefer one over the other.
… |
|
I added the |
skurfer
added a commit
that referenced
this pull request
Dec 12, 2021
This was referenced Feb 12, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.