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

keychain_mac: fix build on < 10.9 #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

barracuda156
Copy link

@hrantzsch
Copy link
Owner

Hi, thanks for the report and the fix! I'll have to look into testing it, because GitHub Actions doesn't offer such old versions of macOS. And apparently I need to fix GH Actions anyway.

One question though: you commented on #33 that a fallback would be nice, but are you sure the problem is related to the change from that PR? The problematic errSecUserCanceled was already used before that.

@barracuda156
Copy link
Author

@hrantzsch At the moment I can only say that 1.2.1 built for me, but 1.3.0 did not. To find out which commit introduced the issue, I would need either to bisect or go through history. (So no, I am not sure whether that specific PR introduced the problem.)

@hrantzsch
Copy link
Owner

That's alright, I'll figure that out if I can find some old version of macOS somewhere.

@hrantzsch
Copy link
Owner

I looked into this for a while but unfortunately I did not find a way to get my hands on an installation of OS X 10.8 (virtual or otherwise). Without any chance to test it, I don't think I can support this old version.

The change in this PR is simple enough, but given that the enum value in question has already been used in Keychain since the very beginning, I'm not convinced that this is the what introduced the issue for you.

Do you have any suggestions how I could test this? Otherwise I don't know how to move forward with this.

@barracuda156
Copy link
Author

@hrantzsch Could you say what was the issue with the older OS? I think UTM and some other virtualization software are free and support macOS back to 10.5. (I mean, if the problem was technical, it is likely solvable. I do realize that doing this requires a bit of time to have it set up.)

@hrantzsch
Copy link
Owner

I tried 10.8 and got it to a point where it started the installation in a VM on a Linux host. But then it failed with "Can't download the additional components needed to install Mac OS X.". I found some suggestion in an Apple forum to set the system clock to an earlier date because apparently Apple let the certificates of the download servers expire, but that didn't help either.
There's no feedback what went wrong beyond that error message, or at least I didn't know where to find it.

@barracuda156
Copy link
Author

I tried 10.8 and got it to a point where it started the installation in a VM on a Linux host. But then it failed with "Can't download the additional components needed to install Mac OS X.". I found some suggestion in an Apple forum to set the system clock to an earlier date because apparently Apple let the certificates of the download servers expire, but that didn't help either.
There's no feedback what went wrong beyond that error message, or at least I didn't know where to find it.

I never used Linux myself, but 10.5 and 10.7, for example, are known to install in UTM: d99kris/nmail#153 (reply in thread)
And UTM is basically an interface to Qemu, so that should work with similar settings on Linux, I think.

I have 10.8 installed in Parallels, AFAIR, it did not require any downloads to be installed. Possibly, there are somewhat different install images available, something like “Retail install DVD” ones should be complete and installable offline.

@RJVB Maybe you could help us here a bit, I recall you also use Linux?

@RJVB
Copy link

RJVB commented Apr 25, 2024 via email

Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.83%. Comparing base (63682e3) to head (29218c6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #38   +/-   ##
=======================================
  Coverage   28.83%   28.83%           
=======================================
  Files           5        5           
  Lines        4546     4546           
  Branches      555      554    -1     
=======================================
  Hits         1311     1311           
  Misses       3141     3141           
  Partials       94       94           
Flag Coverage Δ
unittests 28.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

None yet

3 participants