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

Electron 11.0.0-beta.11 produces invalid builds #103

Closed
Julusian opened this issue Apr 14, 2021 · 6 comments
Closed

Electron 11.0.0-beta.11 produces invalid builds #103

Julusian opened this issue Apr 14, 2021 · 6 comments

Comments

@Julusian
Copy link

I have been digging into an issue with a nan and prebuild based library that was throwing an error only when run in electron 11 serialport/node-serialport#2191

My conclusion is that electron v11.0.0-beta.11...v11.0.0-beta.12 is a breaking abi change and should have changed the abi version number. I haven't raised anything upstream, as I don't see anything they can do other than say to not use that version.

So to make everything happy, prebuild (and others?) need to build for electron 11 against v11.0.0-beta.12 or newer. As they get the version number from this library, it would be nice to fix it here, but I don't know how the data should be modelled, as a single version essentially needs to be ignored.

@dunkmann00
Copy link

There is a similar problem connected to electron 9 prebuilds, also coincidentally relating to serialport. In this case the electron 9 betas up to beta 9 are producing the problem, but from beta 10 and up it seems to work just fine.

@MarshallOfSound Looking at how the update-abi-registry.js script handles electron releases, would you be open to reversing the list when processing? That way instead of an ABI version linking to the earliest target (which in the case of electron builds is always an early beta), it would link to the latest released version that uses that particular ABI.

Since the update script is run automatically that wouldn't create any extra work for anyone, and I think it would solve both of these problems with prebuild.

@Julusian
Copy link
Author

@dunkmann00 that will break various methods exposed by this library, such as https://github.com/lgeiger/node-abi/blob/master/index.js#L16.

I believe the primary purpose of this data is to figure out what abi version to use for a process, and not for prebuild to choose a version to build against.

@dunkmann00
Copy link

@Julusian I'm not sure why it would break something?

What I was suggesting was:

update-abi-registry.js line 42

function electronReleasesToTargets (releases) {
  const versions = releases.map(({ version }) => version)
  const versionsByModules = releases
    .filter(release => release.deps && Number(release.deps.modules) >= 70)
++  .reverse()
    .map(({ version, deps: { modules } }) => ({
      version,
      modules,
    }))
    .reduce(
      (acc, { modules, version }) => ({
        ...acc,
        [modules]: version,
      }),
      {}
    )
...
}

Just adding the reverse array method call on the line marked wiht the ++. I could be missing something but I don't think that would break anything?

@Julusian
Copy link
Author

That results in the abi_registry.json containing this:

{
    "abi": "80",
    "future": false,
    "lts": false,
    "runtime": "electron",
    "target": "9.4.4"
  },
  {
    "abi": "82",
    "future": false,
    "lts": false,
    "runtime": "electron",
    "target": "12.0.0-nightly.20200914"
  },
  {
    "abi": "85",
    "future": false,
    "lts": false,
    "runtime": "electron",
    "target": "11.4.8"
  },
  {
    "abi": "87",
    "future": false,
    "lts": false,
    "runtime": "electron",
    "target": "13.0.0-nightly.20201123"
  },

Which means that a call to getAbi('11.0.0', 'electron') gives back 80, instead of the expected 85

@dunkmann00
Copy link

@Julusian 👍 I see now.

@MarshallOfSound
Copy link
Member

Electron 11 should produce valid builds nowadays, the ABI registry lookup logic has been tweaked slightly. Passing the correct electron version into node-abi results in the updated ABI version being used even if the ABi version during an Electron major line

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

No branches or pull requests

3 participants