Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

support building on more platforms #58

Conversation

jcgruenhage
Copy link

@jcgruenhage jcgruenhage commented Aug 17, 2021

This adds support for building on a lot more linux platforms. Supported
architectures now include x86_64, i686, powerpc64le and
aarch64. In addition to that, this adds the ability to use musl on each
of these as well, in addition to glibc.

@jcgruenhage
Copy link
Author

cc @q66

This adds support for building on a lot more linux platforms. Supported
architectures now include x86_64, i686, powerpc64le and aarch64. In
addition to that, this adds the ability to use musl on each of these as
well, in addition to glibc.
@jcgruenhage jcgruenhage force-pushed the compile-on-less-common-platforms branch from 7d48521 to c1e1916 Compare August 17, 2021 15:30
@q66
Copy link

q66 commented Sep 13, 2021

lgtm, also works for me (tested element 1.8.4, electron 13.x, ppc64le-musl)

@jcgruenhage jcgruenhage marked this pull request as ready for review September 14, 2021 08:10
@jryans jryans requested a review from a team September 21, 2021 10:52
@dbkr
Copy link
Member

dbkr commented Sep 22, 2021

closing & re-opening to try & wake the linter job up...

@dbkr dbkr closed this Sep 22, 2021
@dbkr dbkr reopened this Sep 22, 2021
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Looks ok. If this gets much more complex it's probably going to need something a bit more sophisticated than just this file.

Bit weird that these effectively do nothing & have to be enabled by editing the code. Main issue here is that we have no means of maintaining this as we wouldn't be building for these platforms. Perhaps we could assign a maintainer for some 'community maintained' platforms?

@jcgruenhage
Copy link
Author

I could be assigned as community maintainer for i686, x86_64 and aarch64, maybe @q66 could do ppc64?

@q66
Copy link

q66 commented Sep 22, 2021

there is a bunch more stuff needed to get things built on ppc64le, but this part is the only one that's in element itself (the rest is workarounds for electron-builder, so that's a different upstream to address)

I test things fairly regularly, so this should be no problem

@jcgruenhage
Copy link
Author

@dbkr anything that's needed to move this forward?

@dbkr
Copy link
Member

dbkr commented May 12, 2022

I asked the team about this and the conclusion is that this seems fine. Currently the only 'dead' target in this file is x86 windows (which we previously built but don't anymore) so if we could just add some comments to explain why these targets are sitting around in the file without being enabled, that would be good (and add the semicolons that the linter wants).

@t3chguy
Copy link
Member

t3chguy commented May 12, 2022

Currently the only 'dead' target in this file is x86 windows (which we previously built but don't anymore)

I have plans to resurrect it

element-hq/element-web#13175

@jcgruenhage
Copy link
Author

I was under the impression that this bit of code was moved into element-desktop itself, which previously was holding a ts->js transpiled artefact, see https://github.com/vector-im/element-desktop/blob/develop/scripts/hak/target.ts. In which of these two places is the code maintained now?

@dbkr
Copy link
Member

dbkr commented May 26, 2022

Good point - we should update both if we're changing it, possibly making builder refer to version in element-desktop

@t3chguy
Copy link
Member

t3chguy commented May 26, 2022

#67 makes the reference happen, please move these changes to element-desktop in their entirety where they'll be accepted with open arms!
All the target specs will live in element-desktop, element-builder will have the list of "ENABLED_TARGETS" (now just targets in index.ts)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants