🔍 Upgrade only to newer versions

[chris-araman]

Also:
🏃 Throw any errors from Process.run

[chris-araman]

This addresses an issue where mas info, mas outdated and mas upgrade sometimes report an older version is available in the App Store than what was reported in a previous mas invocation, or, potentially, older than what is currently installed.

The App Store search API is behind an Akamai CDN with a caching mechanism that sometimes reports (slightly) out-of-date information. Requesting the same information multiple times might yield dissimilar results while the CDN reaches eventual consistency. (Example here.) Before #345, this was ameliorated by the URLSesssion cache. With that cache, mas frequently reported cached results that were older than what was shown in the App Store app. Without the client cache, mas is exposed to the eventual consistency problem.

In an effort to avoid inadvertently "upgrading" to an older version, I thought it might be best to start comparing version numbers instead of checking for version string equality.

One wrinkle with that plan is that the App Store does not enforce semantic versioning. However, we will assume most apps are sane and follow versioning schemes that increase numerically over time.

Another wrinkle is that the App Store seems to allow vendors to publish a new release with a lower-numbered version than a previous release. In this case, the App Store app would update to the new, lower-numbered version while mas upgrade would remain on the older, higher-numbered version. I believe this is rare enough that the benefits of this change outweigh the risk.

As seen in #331, the App Store seems to allow vendors to publish an app bundle with a CFBundleShortVersionString that doesn't match the corresponding version from the App Store release. That issue is orthogonal to this one, though. Perhaps we can resolve that by querying the "installed version" from CommerceKit/StoreFoundation instead.

If a version string from a bundle or from the App Store can't be parsed as a Semantic Version, our best effort is to revert to checking for string equality. The only version that matters is the one in the App Store.

Note that this change doesn't fix the eventual consistency problem. mas outdated and mas upgrade may still intermittently not see a newer version when one exists. However, mas will no longer oscillate between versions N and N-1 when invoked repeatedly.

Note also that this change does not address #331, where an app will always appear to be outdated because its bundle version does not match its App Store version.

[chris-araman]

This last change should address the specific case outlined in #331, in that Messenger's bundle version 97.11.116 is currently "newer" than its App Store version 96.0. We will now treat 96.0 as if it were 96.0.0 and comparable semantically to 97.11.116.

However, this would not address a hypothetically possible case where a vendor publishes a release to the App Store with a version that is "newer" than its bundle version. In that case, mas would continue to report the installed app as outdated.

[chris-araman]

I discovered that the app pages frequently list versions that are newer than what the search API returns. It seems that there's a delay before the search API is updated with newly published releases, unrelated to the client cache or the Akamai CDN cache. To work around this, I added logic to scrape the current version from the app page.

[phatblat]

Sorry for the delay in looking at this, @chris-araman. Fantastic research!

I'm not surprised by these issues as the related commands were broken for so long. I'll start looking closer at these changes tonight.

[chris-araman]

I snuck one more in to reduce the public surface area (and binary size).

I'm done for the night. All yours!

[phatblat]

This is such great work! I have mostly style questions and suggestions.

[phatblat]

Ending the method on an error case makes me a bit twitchy,. Could you switch this up and have the happy/golden path end the method?

Suggested change

	throw MASError.noData
	guard let data = data else {
	throw MASError.noData
	}
	return data

[chris-araman]

I agree it's awkward. I went back and forth on this. The problem is that this definition would conflict with the definition of data on L42.

I've committed a force-unwrap instead as a85d4e8. Is that less twitchy?

[chris-araman]

Regarding the future direction of async code in mas-cli, I'm not thrilled with the current pyramid-of-doom style. We can't just use Swift Combine, because it requires macOS 10.15 or later.

I could see a few options:

• Take a dependency on PromiseKit. We already have a dependency on another mxcl project in this PR. However, the future of PromiseKit is not clear to me now that Combine exists. I agree with the author that PromiseKit and Combine are complementary in part, but there does seem to be some overlap.
• OpenCombine exists but hasn't yet reached a stable release.
• CombineX is another option, but it seems like it might be more difficult to integrate.

I think any of the above options would let us simplify the async code in MasKit a bit, at the cost of adding to our list of dependencies. Do you have any thoughts on this, @phatblat?

Also: Unrelated, but I'm curious, what was the motivation behind breaking MasKit out into its own framework? Is there a goal to publish it as an SPM package? It seems like we could simplify the install a bit by linking MasKit and Commandant as libraries into the executable, instead of laying them down as frameworks.

[chris-araman]

RE: Async patterns/frameworks. Maybe I'll just post PRs for a couple of options and see what you think.

Did you need anything else from me on this PR, @phatblat?

[phatblat]

Sorry for the delay. I like your thinking on the management of async code but I haven't had time to digest the options yet. Let me approve this PR so we can move forward.