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

Add Github action for building EXE #173

Closed
wants to merge 5 commits into from

Conversation

sebastian-berlin-wmse
Copy link
Contributor

Bug: #152

@sebastian-berlin-wmse sebastian-berlin-wmse changed the title Add Github action for building EXE (#3) Add Github action for building EXE Mar 21, 2023
Copy link
Collaborator

@Abbe98 Abbe98 left a comment

Choose a reason for hiding this comment

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

It looks good overall!

I would like to see to what extend the dependencies on Github and Windows are actually needed. Given that it's based on SFX archives I don't see why the approach shouldn't work on Linux.

Aside from the cost and dependency aspects, a future opportunity if it works on Linux could be to make all builds with a single script or in a container if need be.

README.md Outdated

----
* Download the EXE file from downloads linked above.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This type of installation instructions currently live on Commons:Pattypan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean they shouldn't be in the repo? If so I can remove them and add them to the wiki once this is merged. Looks like https://commons.wikimedia.org/wiki/Commons:Pattypan/Simple_manual is a natural place to have them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

README.md Outdated Show resolved Hide resolved
.github/workflows/ant.yml Outdated Show resolved Hide resolved

jobs:
build:
runs-on: windows-latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks to me like it should run on UNIX(with some minor changes) as long as the Java distribution doesn't strip Windows specific stuff? 7-zip SFX build should be fine on Linux.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In case it runs on Linux we could move the various steps to a Shell file so that we don't end up with proprietary dependencies and gets local reproducibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was there a review action here or are these just notes for how this could be developed further?

.github/workflows/ant.yml Show resolved Hide resolved
@Abbe98
Copy link
Collaborator

Abbe98 commented Apr 12, 2023 via email

@sebastian-berlin-wmse
Copy link
Contributor Author

OK, so that's a possible further development and not something to do for this PR, right? This is specifically for Windows.

@Abbe98
Copy link
Collaborator

Abbe98 commented Apr 12, 2023 via email

@lokal-profil
Copy link

Hi @Abbe98,
Sorry for the long silence on this issue. Finally found the time to come back to it.

It's a bit unclear to me what exactly the issues are. If I’m understanding them correctly there are two separate ones.

To build the Windows stand-alone .exe file the Action job needs to run on Windows (which is proprietary).

Using Github Actions costs minutes (and Windows even more so) which is a limited resource for you (per #152 (comment)).

Is this understanding correct?

For 1) I believe @sebastian-berlin-wmse answered this in #152 (comment). I.e. the .exe needs to be created in the same OS as the result should be run on (since it doesn’t create an installer but a directly executable version of Pattypan). This would mean that even were you to run this locally you would need Windows.

For 2) This Action should only ever run when there is a new release. Which historically is not very often. The job takes around 2 minutes (so 4 minutes with the Windows multiplier). If it is run on this repo then those minutes should be taken from Yarl’s pool of free minutes right?

Either way we’d like to find a way forward on this since there is now a real use case for these executables and the most natural place for them to live is attached to the official repo.

Cheers,
André

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