sparkle-project / Sparkle

A software update framework for macOS
https://sparkle-project.org
Other
7.42k stars 1.05k forks source link

Does not compile on Xcode 8 and code missing #1373

Closed hofman closed 5 years ago

hofman commented 5 years ago

Description of the problem

Sparkle does not compile in Xcode 8 on 10.11. The problem, at least, is in the SPU downloader classes. The availability directives and macros don't work there. I seriously doubt now whether Sparkle will even be backwards compatible with 10.7, which I need (when compiled in a newer Xcode version). Also, the content of the ed55519 directory is missing in the code of the latest release distributions. Is there any review of releases? This problem must have been present already for several releases. The website does say it is compatible with Xcode 8 with 10.10 SDK, and runtime 10.7. At least 2 out of three are not true anymore, what about 3? I need all to be true. Which version of Sparkle satisfies these requirements?

Do you use Sandboxing in your app?

No.

Version of Sparkle.framework in the latest version of your app

1.21.3

Version of Sparkle.framework in the old version of app that your users have (or N/A)

NA

Sparkle's output from Console.app

NA

Steps to reproduce the behavior

Just try to compile the sources from 1.21.3 in Xcode 8 on 10.11.

hofman commented 5 years ago

It looks like the compile problem is due to the use of @available in SPUDownloadData. It should check the OS version instead and ignore warnings, like for the touchbar code.

I also saw that the ed25519 code from the master branch give a lot of warnings due to implicit type casts between signed/unsigned types.

kornelski commented 5 years ago

Thanks for the report.

I've bumped minimum requirement to Xcode 9. That's what we have in CI, so earlier versions don't get tested and rot like that.

I'd rather keep using @available since that's an easy and reliable runtime check, but it could be OK to hide these completely with #ifdefs from old Xcode/SDKs. Would you like to make a PR for that?

hofman commented 5 years ago

I don't like that, I unfortunately need Xcode 8.

Why keep @available? All the rest of the code checks the OS version using SUOSOperatingSystem.

hofman commented 5 years ago

Why not define a macro that can be defined as @available when this is supported, for instance:

#if __clang_major__ >= 9
#define SUAVAILABLE(major, minor) @available(macOS major##.##minor, *)
#else 
#define SUAVAILABLE(major, minor) (floor(NSFoundationVersionNumber) >= NSFoundationVersionNumber##major##_##minor)
#fi 
hofman commented 5 years ago

Sorry, I guess the fallback should use SUOperatingSystemVersion as NSFoundationVersionNumber constants may not be available.

#if __clang_major__ >= 9
#define SUAVAILABLE(major, minor) @available(macOS major##.##minor, *)
#else 
#define SUAVAILABLE(major, minor) [SUOperatingSystem isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){major, minor, 0}]
#fi 
hofman commented 5 years ago

Added a PR to do this