Skip to content
/ lxml Public

add win-arm64 support#326

Merged
scoder merged 2 commits intolxml:masterfrom
niyas-sait:enable_arm64_build
Oct 17, 2021
Merged

add win-arm64 support#326
scoder merged 2 commits intolxml:masterfrom
niyas-sait:enable_arm64_build

Conversation

@niyas-sait
Copy link
Copy Markdown
Contributor

@niyas-sait niyas-sait commented Oct 7, 2021

This patch enables building lxml package for win/arm64 platforms.

The change expects win/arm64 libraries to be available in repository currently used for getting x86 and x64 dependencies. The packages are not yet available in the repository but a PR has been raised to add arm64 libraries.

Unofficial release of win/arm64 libraries from PR are available here

Test Result


TESTED VERSION: 4.6.3
    Python:           sys.version_info(major=3, minor=10, micro=0, releaselevel='candidate', serial=1)
    lxml.etree:       (4, 6, 3, 0)
    libxml used:      (2, 9, 5)
    libxml compiled:  (2, 9, 5)
    libxslt used:     (1, 1, 30)
    libxslt compiled: (1, 1, 30)
    FS encoding:      utf-8
    Default encoding: utf-8
    Max Unicode:      1114111

----------------------------------------------------------------------
Ran 1937 tests in 7.474s

OK

@niyas-sait
Copy link
Copy Markdown
Contributor Author

@scoder This is probably not quite ready yet considering the library dependency is not yet available in the libxml2-win-binaries repository. But would be nice to hear your thoughts on how to resolve the dependency etc. to get this sorted for win/arm64 targets.

@scoder
Copy link
Copy Markdown
Member

scoder commented Oct 17, 2021

It doesn't hurt to merge it already.

Regarding Windows library building, I actually don't know how they were previously built. Having a build script for Github Actions would certainly be very much appreciated.

@scoder scoder merged commit 5d7d69d into lxml:master Oct 17, 2021
@niyas-sait
Copy link
Copy Markdown
Contributor Author

Thanks, @scoder.

As the external dependencies for windows were pulled from libxml2-win-binaries, I've created a PR to extend the appveyor build scripts and related files in libxml2-win-binaries to add win-arm64 packages.

Unfortunately, the maintainer of libxml2-win-binaries doesn't have time to maintain the repository and he left a comment for you to look for an owner for that repository. Could you suggest what would be the best way to resolve this issue?

I've also created a full library release through appveyor here which includes packages for all supported platforms including win-arm64.

scoder pushed a commit that referenced this pull request Nov 1, 2021
Comment on lines 41 to +48
if sys.version_info < (3, 5):
arch = 'vs2008.' + arch
elif platform.machine() == 'ARM64':
arch = "win-arm64"
elif sys.maxsize > 2**32:
arch = "win64"
else:
arch = "win32"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff breaks the arch reference in the first if-block and breaks the py27 builds on appveyor:

Not sure how relevant this is considering you're currently trying to sort out the dependency builds in the remote repository, but just wanted to let you know in case you haven't noticed already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think the statements in lines 41 and 42 should be moved after line 48

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #331 to fix it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, yeah, I noticed this, too, when I was trying to set up an automated wheel build.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry for breaking your original PR)

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

Successfully merging this pull request may close these issues.

3 participants