Skip to content

Conversation

@provokateurin
Copy link
Member

Split of #30 (exactly: #30 (comment)).
Moves packaging to containers for packaging to work on all OSs. Requires Docker to be installed.

@GeertJohan
Copy link
Member

GeertJohan commented Sep 30, 2019

Nice, I like the refactoring, code separated into specific files and packages etc.

Is it possible to keep docker optional when targetOS == runtime.GOOS? Then users don't have to install docker if they already have (e.g.) snap installed.
On the other hand, this creates situations where different approaches are used based on OS, more code to maintain...

@Drakirus thoughts?

edit: I haven't done a complete code-review of this PR yet

@pchampio
Copy link
Member

I'll also prefer to keep Docker optional for packaging.
As a Linux developer, when targeting windows, (the most used platform on the desktop), having the possibility to generate a windows installer is a necessity. In this case I see why I would need Docker.
But, as a windows developer, I usually only care about windows, and the Docker requirement becomes a painful point.

In the same way we have the --docker flag for hover run, I would also like to have it on packaging.

@GeertJohan
Copy link
Member

GeertJohan commented Sep 30, 2019

Agree with @Drakirus

I also see how the --docker flag can be a bit confusing in combination with packaging. A user may read it as: package as a docker container image.

Perhaps --use-docker better describes what's going on?
If so, the flag name should also be changed for cross-compilng so it's a consistent experience for users. The flag --docker may be kept as hidden flag for a few months for backwards-compatibility, or just removed straight away.

@provokateurin
Copy link
Member Author

The packaging for windows uses a tool called wixl which is only for linux. The original closed source program is called wixtoolset. My approach is to create true cross-packaging. wixl should be compatible with the wixtoolset, but maintaining 2 different versions of the code for windows / non-windows is not great for the overall maintainability.

@pchampio
Copy link
Member

pchampio commented Oct 1, 2019

The packaging is a one-time process, and I don't think adding one or 2 other packaging systems hurts that much the maintainability. (And if not happy, user can still zip the build dir..)

Regarding this particular case of 'wixl' (linux tool to generate windows MSI) and 'wixtoolset' (window tool to generate windows MSI), I think it should be run by independent hover command, e.g.: hover build window-msi-onlinux and hover build window-msi.

IMHO: The docker feature can be added after each OS has a working packaging system. Because for now, we are only assuming that it is a valid demand. (Still, I'm a fan of hover build window-msi-onlinux and hover build window-msi, but not docker, at least for now)

@provokateurin
Copy link
Member Author

My plan was to approach it like the cross-compiling:
Package from any OS for any OS using Docker.
For cross-compiling I could just have checked if the cross-compiling tools are available, but that would be just a mess.
So doing all cross-compiling and eventually all cross-packaging in Docker containers is the only possibility.
I think sticking with my current approach would be the best for the future of packaging.

@provokateurin
Copy link
Member Author

The packaging formats I created which work on all OSs using Docker:
linux-deb: https://github.com/jld3103/hover/tree/feature/cross-packaging
linux-snap: https://github.com/jld3103/hover/tree/feature/cross-packaging
linux-appimage: https://github.com/jld3103/hover/tree/feature/packaging/linux-appimage
windows-msi: https://github.com/jld3103/hover/tree/feature/packaging/windows-msi
darwin-bundle: https://github.com/jld3103/hover/tree/feature/packaging/darwin-bundle
darwin-pkg: https://github.com/jld3103/hover/tree/feature/packaging/darwin-pkg

They all have only Docker as their dependency at work cross platform. I don't see a reason to not use only Docker for packaging.

@GeertJohan
Copy link
Member

I'm convinced about going docker-only for now, these are good arguments @jld3103

@GeertJohan
Copy link
Member

GeertJohan commented Oct 3, 2019

I'm attempting merge with master (there are some conflicts).

@GeertJohan GeertJohan merged commit f447c9f into go-flutter-desktop:master Oct 3, 2019
runCmd.Flags().StringVarP(&buildTarget, "target", "t", "lib/main_desktop.dart", "The main entry-point file of the application.")
runCmd.Flags().StringVarP(&buildBranch, "branch", "b", "", "The 'go-flutter' version to use. (@master or @v0.20.0 for example)")
runCmd.Flags().StringVarP(&buildCachePath, "cache-path", "", "", "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll (defaults to the standard user cache directory)")
runCmd.Flags().StringVar(&buildOpenGlVersion, "opengl", "3.3", "The OpenGL version specified here is only relevant for external texture plugin (i.e. video_plugin).\nIf 'none' is provided, texture won't be supported. Note: the Flutter Engine still needs a OpenGL compatible context.")
Copy link
Member

Choose a reason for hiding this comment

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

We should keep this flag on hover run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants