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

Adds support for "fyne release" #3

Merged
merged 12 commits into from Dec 2, 2020
Merged

Conversation

lucor
Copy link
Member

@lucor lucor commented Nov 1, 2020

This PR adds support for "fyne release"

Note: since "release" for windows, ios and macOS requires native tools, it is supported only on the relative hosts via the fyne-cli tool.

Fixes #1

@lucor lucor changed the title Feature/release Adds support for "fyne release" Nov 1, 2020
@lucor lucor marked this pull request as ready for review November 4, 2020 20:01
@lucor lucor requested a review from andydotxyz November 4, 2020 20:01
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Great work getting this together thanks.
I see that iOS and Windows release build is called on the host, so I guess this means that they can lookup the keystore / key file for the signing.
However the Android build asks for a keystore path - is that accessible from the runtime ? (It may be outside the project) - and it will ask for the password on the command line, does that work in this model?

I was a little confused as fyneRelease vs fyneReleaseHost seems to have duplicated code but as I understand it the former is just android? In which location do linux/bsd releases run?

@lucor
Copy link
Member Author

lucor commented Nov 5, 2020

Great work getting this together thanks.
I see that iOS and Windows release build is called on the host, so I guess this means that they can lookup the keystore / key file for the signing.

Right, it is basically a wrapper for fyne release on the host for Windows and iOS. This is needed since the tools used signign are not available into the docker image.

However the Android build asks for a keystore path - is that accessible from the runtime ? (It may be outside the project)

Yes, but it can only access files inside the project. Mounting of external files or directories is not supported yet.

  • and it will ask for the password on the command line, does that work in this model?

Good catch, no won't work. We need to specify the password with a flag like for Windows. If this is not supported by fyne release we could always allow the Andoid release only in host mode for now.

I was a little confused as fyneRelease vs fyneReleaseHost seems to have duplicated code but as I understand it the former is just android? In which location do linux/bsd releases run?

Yep, I was unsure about the naming too. Tempted to add the Docker suffix but at the end I kept fyneRelease for consintence with the fynePackage. Anyway renaming to fyneReleaseDocker and fynePackageDocker is an easy fix if it helps to improve the code readability.

TD;DR
fynePackage and fyneRelease run the fyne tool inside the container.
fynePackageHost and fyneReleaseHost run the fyne tool from the host.
Only Windows (release command) and iOS (package and release) are executed on the host.

@andydotxyz
Copy link
Member

Testing locally, did a --pull, but I don't think the bundled fyne is new enough?

❯ fyne-cross darwin -release
[i] Target: darwin/amd64
[i] Cleaning target directories...
[✓] "bin" dir cleaned: /Users/andy/Go/src/github.com/fyne-io/life/fyne-cross/bin/darwin-amd64
[✓] "dist" dir cleaned: /Users/andy/Go/src/github.com/fyne-io/life/fyne-cross/dist/darwin-amd64
[✓] "temp" dir cleaned: /Users/andy/Go/src/github.com/fyne-io/life/fyne-cross/tmp/darwin-amd64
[i] Checking for go.mod: /Users/andy/Go/src/github.com/fyne-io/life/go.mod
[✓] go.mod found
[i] Packaging app...
flag provided but not defined: -category
Usage: fyne [command] [parameters], where command is one of:
...

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

👍

@andydotxyz
Copy link
Member

flag provided but not defined: -category
Usage: fyne [command] [parameters], where command is one of:

For the record this was an out of date local fyne, which is used by darwin release.

@lucor lucor merged commit b66faed into fyne-io:develop Dec 2, 2020
@lucor lucor deleted the feature/release branch December 2, 2020 12:48
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.

None yet

2 participants