Adding normalization to osx32, osx64, win32, and win64.

[theiawolfe]

Here are the changes I made to get Several changes to help with adding 64bit support. #117 ready to (hopefully) be accepted:

• Change osx to osx32 and osx64, and win to win32 and win64.
• Since we're working with one platform at a time I'm using detection on architecture to determine which one to use is a string like 'osx' or 'win' is passed.
• In the case of this being called from node-webkit-builder, the list containing the platforms has already been modified if it contains 'osx' and/or 'win' to contain 'osx32' and 'osx64' and/or 'win32' and 'win64' respectively.
  • Tests have been updated and should all be passing.

[adam-lynch]

😍 thanks!

Correct me if I'm wrong here but... It should still be possible for a user to put just win in their JSON and that would mean win32 and win64. But auto-detection should only use either 32 or 64 (like you have).

I guess it should be possible for a user to have this in their JSON (if they wanted):

{
    "a": 1,
    "b": 2,
    "platformOverrides": {
        "win": {
            "a": 2
        },
        "win64": {
            "b": 64
        }
    }
}

So then on Windows 32-bit, it would return {a:2, b:2} and on 64-bit: {a:2, b:64}.

Does that make sense?

---

Also, when a platform is passed, osx and win can no longer be passed with this PR, right? And that's OK? I think it is but I'm worried there's an edge case (which probably doesn't exist) 😄.

[theiawolfe]

Yeah, that makes sense, but I have no idea on how to accomplish that 😞

With this line

platform = if args.platform then normalizePlatform args.platform else normalizePlatform detectPlatform()

and

darwin: -> 'osx' + if process.arch is 'ia32' then 32 else 64
osx: -> 'osx' + if process.arch is 'ia32' then 32 else 64
osx32: -> 'osx32'
osx64: -> 'osx64'
win: -> 'win' + if process.arch is 'ia32' then 32 else 64
win32: -> 'win32'
win64: -> 'win64'

Users should still be able to pass in 'osx' or 'win', but it will get normalized to 'osx32' / 'osx64' or 'win32' / 'win64' based on the architecture they're running. (Which makes sense on auto detected platforms, but admittedly, might be a bit weird when building cross platform.)

[theiawolfe]

Also, consider the cases:

"platformOverrides": {
        "win": {
            "a": 2
        },
        "win64": {
            "a": 64
        }

and

"platformOverrides": {
        "win64": {
            "a": 64
        },
        "win": {
            "a": 2
        }

Should the 32/64 version always take precedence, or should the last one in the list take precedence? I think there's might be enough ambiguity there to justify making the json require either 32 or 64 to be in the platformOverride.

Just a thought.

[adam-lynch]

Yeah, that makes sense, but I have no idea on how to accomplish that

Yeah I was thinking that logic would have to be re-written; auto-detection versus specifying a platform will be very different now.

Users should still be able to pass in 'osx' or 'win', but it will get normalized to 'osx32' / 'osx64' or 'win32' / 'win64' based on the architecture they're running. (Which makes sense on auto detected platforms, but admittedly, might be a bit weird when building cross platform.)

This does seem weird. On auto-detection, nothing is passed but when a platform is passed (i.e. it is a different platform to the current one) then the architecture should never be detected. So yeah, maybe we should no longer accept osx or win.

Should the 32/64 version always take precedence, or should the last one in the list take precedence? I think there's might be enough ambiguity there to justify making the json require either 32 or 64 to be in the platformOverride.

Order in the JSON file shouldn't matter and I think specifity should win always. So if you have:

{
    "a": 0,
    "b": 1,
    "platformOverrides": {
        "win": {
            "a": 100,
            "b": 999
        },
       "win32": {
           "b": 101
       }
    }
}

Then on win32 (or if win32 is passed), then it will return { a: 100, b: 101 }.

---

If you want, I can create a new branch for this and you can make a PR from your branch to that new branch. Then I can merge right away and we can work out the rest of the details, like re-working that logic. But I guess we might as well start with tests defining exactly what's expected.

[theiawolfe]

If you want, I can create a new branch for this and you can make a PR from your branch to that new branch. Then I can merge right away and we can work out the rest of the details, like re-working that logic. But I guess we might as well start with tests defining exactly what's expected.

That sounds like a good idea. Let me know where to PR to.

[adam-lynch]

There's a new branch called 64-bit

[theiawolfe]

Closing in favor of PR #3