Skip to content

Conversation

@joshtynjala
Copy link
Contributor

My SDKs directory contains three entries:

  • MacOSX.sdk
  • MacOSX26.0.sdk
  • MacOSX26.sdk

The current SDK version detection in hxcpp is preferring MacOSX26.sdk over MacOSX26.0.sdk. This is causing MACOSX_VER to be set to 26. However, xcodebuild seems to want both major and minor parts of the version to be specified, so MACOSX_VER should be set to 26.0 instead.

This change checks if the current best's minor version is parsed as NaN when the major versions match, which results in a real float value for the minor version to be preferred.

My SDKs directory contains three entries:

MacOSX.sdk
MacOSX26.0.sdk
MacOSX26.sdk

The current SDK version detection in hxcpp is preferring MacOSX26.sdk over MacOSX26.0.sdk. This is causing MACOSX_VER to be set to 26. However, xcodebuild seems to want both major and minor parts of the version to be specified, so MACOSX_VER should be set to 26.0 instead.

This change checks if the current best's minor version is parsed as NaN when the major versions match, which results in a real float value for the minor version to be preferred.
var minor_best = Std.parseFloat(split_best[1]);
if (major_ver == major_best && Math.isNaN(minor_best))
best = ver;
else if (major_ver > major_best || minor_ver > minor_best)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't these two branches here be combined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, yes, but it felt like too much complexity for a single if condition. Separating them made them feel easier to understand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the logic be simpler if incomplete version numbers are just ignored altogether, since it will always lead to an invalid MACOSX_VER value?
.e.g

if (Math.isNan(major_ver) || Math.isNan(minor_ver))
   continue;

Also it seems the second condition would be more correct if it was major_ver > major_best || (major_ver == major_best && minor_ver > minor_best).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that makes sense to me. I think maybe I was trying to support MacOSX26.sdk as a valid fallback if there's some strange situation where MacOSX26.0.sdk didn't exist. However, maybe that wouldn't ever happen.

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.

2 participants