feat: check for @electron/build-tools updates

[dsanders11]

Soliciting feedback on this one. Got a chicken-and-egg problem and trying to minimize breakage for folks. electron/build-tools-installer#6 landed and adds the long-form electron-build-tools which is 'safer' to use for working with build-tools from scripts. I'm using that in electron/electron#26253 to get the Python linting back to working.

So merging electron/electron#26253 would cause breakage when linting for build-tools folks unless they have the latest @electron/build-tools. Could have tried to do an auto-update but that seemed headache worthy, and the need for updating @electron/build-tools should hopefully be uncommon enough that we can just tap people to do it. Explicitly specifying a minimum version so the feature can only be used when necessary, rather than firing on any update to @electron/build-tools which might be minor housekeeping.

I figure we could cut a new release of @electron/build-tools, set this feature to warn about it, give it a week, then merge electron/electron#26253. That would hopefully hit most people by then, and if they hit errors due to the linting fix then they have something actionable. But the update note may also go by unnoticed, so perhaps it's better to do a hard stop there until they update?

[ckerr]

I think I might be misunderstanding the problem. Might be that I have a disconnect because I've only ever installed build-tools from git, rather than from npm.

If someone installs it from npm, it looks like maybeCheckForUpdates() will still be called s.t. we try to pull new changes if it's been > 10 hours since the last update.

If so, doesn't that mean it'll be hard for users to fall out-of-sync with new changes?

And if that's the case, is this new test necessary?

Again, I'm not asking to grill you; I'm asking because I think I'm missing something and this PR doesn't make sense to me. :slightly_smiling_face:

[dsanders11]

Again, I'm not asking to grill you; I'm asking because I think I'm missing something and this PR doesn't make sense to me. 🙂

No worries, @codebytere had a similar question.

I think the disconnect is due to the naming, unfortunately.

The NPM package is called @electron/build-tools, and is published from the electron/build-tools-installer repository, not this repository. That package is what sets up .electron_build_tools and creates the e executable in the global package bin directory.

As such, that's the repository where the long-form name change was made with electron/build-tools-installer#6, adding electron-build-tools as an executable name.

No amount of updating from the code in this repository via maybeCheckForUpdates will pick up that change, since it's not in this repository, and exists outside the scope of .electron_build_tools. Only an update to the @electron/build-tools package will get that update.

So the issue here is that while the update functionality in this repository covers everything inside .electron_build_tools, it does not cover the @electron/build-tools package, which is installed in package manager directories. Any changes or fixes to those files, like the change I'm try to pick up, won't be covered by maybeCheckForUpdates.

[zcbenz]

Would love some reviews here as it is blocking https://github.com/electron/electron/pull/26253, /cc @electron/wg-ecosystem

[dsanders11]

@ckerr @codebytere @MarshallOfSound, this is all rebased and ready for review. Let's make a decision on this and pull the trigger before it goes stale again.

[ckerr]

Let's make a decision on this and pull the trigger before it goes stale again.

@dsanders11 :laughing:

I don't have strong feelings either way about the PR but since @zcbenz says it's blocking https://github.com/electron/electron/pull/26253 I'd be :+1: on going ahead and merging it ... @dsanders11 want to update the patch Yet Again?

[ckerr]

OK I'm taking crazy pills. I thought this had conflicts against main but apparently not.

@dsanders11 I'll ask differently then, since it's been so long since the last update -- are you OK with this landing as-is, or does it need any update work?

[dsanders11]

@ckerr, rebased this on main and bumped the minimum @electron/build-tools package version to 1.1.0. The code still looks good to me (as good as it ever did, at least) and I've confirmed I get the message on my machine which has an older @electron/build-tools. The message could perhaps be stronger, but we can discuss and that's an easy change.

[ckerr]

@dsanders11 we had mentioned giving this further discussion face-to-face but TBH I re-reviewed this last night and am good with merging this as-is.

OK to merge?

[dsanders11]

OK to merge?

OK to merge. 👍