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

AppVeyor CI: Update to Visual Studio 2022 image #355

Closed
wants to merge 1 commit into from

Conversation

EwoutH
Copy link
Contributor

@EwoutH EwoutH commented Oct 20, 2022

This PR updates the AppVeyor configuration to use the Visual Studio 2022 image.

Update 2022-11-07: The Python 3.11 part of this PR has moved to a new PR, #360.

@EwoutH
Copy link
Contributor Author

EwoutH commented Oct 23, 2022

It looks like the AppVeyor build has been failing for quite some time. I openend a bug report at https://bugs.launchpad.net/lxml/+bug/1993962.

@scoder Do you have any idea what's going on?

@EwoutH
Copy link
Contributor Author

EwoutH commented Oct 30, 2022

After the fix in 77413c2, it processes further, and it looks like it now fails with this error:

fatal error C1047: The object or library file 'libs\iconv-1.14.win64\lib\iconv_a.lib' was created by a different version of the compiler than other objects like 'build\temp.win-amd64-cpython-310\Release\src\lxml\etree.obj'; rebuild all objects and libraries with the same compiler

@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 2, 2022

It looks like it's blocked on libiconv being compiled with a compiler that's too old. libiconv was last updated in June 2013, and thus unmaintained.

@scoder Do you know how essential libiconv is to building lxml? Could it be replaced with another, maintained package?

@scoder
Copy link
Member

scoder commented Nov 3, 2022

libiconv is maintained (latest release was last May) and used by libxml2.

@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 3, 2022

Thanks for linking to the right repository. It looks like the latest stable libiconv version is 1.17, and we’re using 1.14. How can we upgrade?

@scoder
Copy link
Member

scoder commented Nov 3, 2022

How can we upgrade?

There's a repository with git subrepos for the dependencies. But libiconv remains at 1.14 from where it's forked. So it won't be trivial, I guess. Not sure if we must upgrade, though. It might be enough to rebuild the libs in their current state with the right compiler.
https://github.com/lxml/libxml2-win-binaries

@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 3, 2022

Would you be able to take a look at that?

@jchalupka-pan
Copy link

I am not an expert in building stuff (for long time got stuck with pure Python), but maybe it will help at least a little bit. I know that it is not a "production solution", but maybe something can be used, since I have built the lxml wheel for Windows and Python 3.11 with BuildTools v143 somewhere last week.

  1. I used master branch from https://github.com/lxml/libxml2-win-binaries.
  2. Applied the patch provided on repo.
  3. Find and replace the build tools version in file libiconv\MSVC16\libiconv_static\libiconv_static.vcxproj from v142 to v143.
  4. Changed the path to vcvars.bat in build.ps1
  5. Build libraries, which can be used while building lxml wheel.

Building the lxml wheel required a hax to hardcode paths to zips and not use those from https://api.github.com/repos/lxml/libxml2-win-binaries/releases. I just modified the function download_and_extract_windows_binaries from buildlibxml.py.

@EwoutH EwoutH changed the title AppVeyor: Update to VS 2022 image, prepare for Python 3.11 AppVeyor CI: Update to Visual Studio 2022 image Nov 7, 2022
@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 7, 2022

I split the Python 3.11 part off to a new PR, #360. I would like to request to merge and backport that one first, since it already works on the VS 2019 image.

It would still be nice to get libiconv updated and move to the Visual Studio 2022 image, especially because it has better Arm support. However, that can wait until #360 is merged and Python 3.11 wheels are on PyPI.

@cclauss
Copy link
Contributor

cclauss commented Dec 6, 2022

Do these tests pass now that #365 has landed?

@bastimeyer
Copy link

Just to add some info to the failed tests on AppVeyor: some of them fail due to rate-limiting of the GitHub API when querying the pre-built Windows libs from the releases page of the lxml/libxml2-win-binaries repo.
https://ci.appveyor.com/project/scoder/lxml/builds/45588135/job/v6baefedljix5rcl#L71

According to

lxml/buildlibxml.py

Lines 34 to 36 in e8f088a

def download_and_extract_windows_binaries(destdir):
url = "https://api.github.com/repos/lxml/libxml2-win-binaries/releases"
releases, _ = read_url(url, accept="application/vnd.github+json", as_json=True)

lxml/buildlibxml.py

Lines 172 to 181 in e8f088a

def read_url(url, decode=True, accept=None, as_json=False):
if accept:
request = Request(url, headers={'Accept': accept})
else:
request = Request(url)
with closing(urlopen(request)) as res:
charset = _find_content_encoding(res)
content_type = res.headers.get('Content-Type')
data = res.read()

no User-Agent header gets set
https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#user-agent-required

You may also want to add a PAT (private access token) for the AppVeyor CI runners, to be able to increase the rate limiting.
https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting

This commit updates the AppVeyor configuration to use the Visual Studio 2022 image. An updated compiler and toolchain helps for Arm64 support, among other things.

See https://www.appveyor.com/docs/windows-images-software/
@scoder
Copy link
Member

scoder commented Dec 13, 2022

The iconv issue is probably related to this:
lxml/libxml2-win-binaries@3fb0529
Not sure what needs to be done – we could undo that change, make it version dependent, or add another build setup for the newer MSVC. What do you think?

@EwoutH
Copy link
Contributor Author

EwoutH commented Dec 13, 2022

I’m not sure. Maybe get Python 3.11 wheels out first, and then look at this.

This AppVeyor build passed, so I would suggest tagging e8f088a as 4.9.2, and uploading these artifacts to PyPI.

@schlenk
Copy link
Contributor

schlenk commented May 26, 2023

This may be related to the binary compatibility rules for static libraries and different Visual Studio versions:

https://learn.microsoft.com/en-us/cpp/porting/binary-compat-2015-2017?view=msvc-170#restrictions

Static libraries or object files compiled using the /GL (Whole program optimization) compiler switch or linked using /LTCG (Link-time code generation) aren't binary-compatible across versions, including minor version updates.

So if the prebuilt static libraries of iconv/zlib do not match the compiler used for the final linkage it breaks. You would need to provide static libs for each compiler/toolset version that you want to support.

@scoder
Copy link
Member

scoder commented Jan 19, 2024

Closing this PR since we're no longer using appveyor.

@scoder scoder closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants