Oh wow - I never would have thought that the presence of this symbol would affect linking behavior. But I suppose it makes sense. The functions are probably left as unresolved references but the static variable reference forces early binding or something. I'm not an expert on how PE does symbol binding/resolution. But this patch shouldn't change behavior and if it fixes the problem, it fixes the problem.

In D10689#162672, @mharbison72 wrote:

running light to produce C:\Users\mharbison\mercurial\build\pyoxidizer\x86_64-pc-windows-msvc\release\msi\mercurial-5.8.msi
C:\Users\mharbison\mercurial\build\pyoxidizer\wxs\mercurial.wxs(55) : warning LGHT1076 : ICE40: REINSTALLMODE is defined in the Property table. This may cause difficulties.
C:\Users\mharbison\mercurial\build\pyoxidizer\wxs\mercurial.wxs(147) : warning LGHT1076 : ICE61: This product should remove only older versions of itself. No Maximum version was detected for the current product. (INSTALLEDMERCURIALPRODUCTS)

IDK if the `REINSTALLMODE` warning matters, or what it is alluding to.

I'll need to release a new version of PyOxidizer to fix wonky behavior due to interaction of file mtimes and pip (https://github.com/indygreg/PyOxidizer/commit/a0bdde179aaed8521d3a668e518554814bcffdba).

Yeah, I was hitting this zip file error too. I'm not sure what's causing it. But I'm actively looking into fixing it!

Turns out Python's distutils isn't smart enough to use Visual Studio 2019, even though its toolchain is ABI compatible with 2017. Boo.

@mharbison72 You may be interested in this series starting at approximately this patch. Version 0.15 of PyOxidizer supports building MSIs "natively." I implemented enough features to port Mercurial's MSI generation to it. I need to have a go at porting TortoiseHg to use PyOxidizer. But I'm optimistic it is doable. Note that you don't need to use the "build a Python binary" pieces of PyOxidizer with MSI generation: they are completely separate.

I'm not going to review this right now, but I have prior art at https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/093657.html and https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-May/097960.html that might be worth a read. I believe the wiki page on this format only contains a subset of the deficiencies I outlined in the 1st link.

The -- also bothers me and I came here to see if it had been discussed before accepting this patch.

Shouldn't this patch be checking for the existence of the windows_curses package instead?

I'm -0 on this change. Caches are caches and IMO should only be populated on demand.

I mostly support this change. However, a supposed no-op upgrade may still result in differences! For example, if we change the algorithm for computing how revlog deltas are computed without actually changing the storage semantics, performing a no-op upgrade would migrate the store to use the new code.

In D9825#149333, @durin42 wrote:

Do you think we should go ahead and apply the local patch then until we can update the vendored copy?

In D9825#149257, @durin42 wrote:

Yes, please remove the python-zstandard changes from here, and send them directly to @indygreg upstream for that.