code icon indicating copy to clipboard operation
code copied to clipboard

Add build and run controls for Flatpak based projects

Open meisenzahl opened this issue 3 years ago • 58 comments

Related to https://github.com/elementary/code/issues/939

It's been quite a long time since I played around with it. After merging master the logic is broken in some places.

Bildschirmfoto von 2021-02-06 09 20 34

meisenzahl avatar Feb 06 '21 08:02 meisenzahl

ngl I think that haveing a small 'section' of the titlebar to have a n almost media-controls-esque play, stop and a button to open a build settings menu in the sidebar or something. Panic's Nova is something with a smilar design in that it's a mac exclusive but also using native design to macOS, Code should be the same for elemetary OS. I'm going to post some more screenshots in the main issue related to this

hanaral avatar Feb 06 '21 08:02 hanaral

@hanaral I separated the run button into it's own section.

Bildschirmfoto von 2021-02-06 09 41 47

I have to implement some more logic to add a stop button.

meisenzahl avatar Feb 06 '21 08:02 meisenzahl

@hanaral I separated the run button into it's own section.

I meant something a bit more like this: 107103342-3c49bf00-6815-11eb-86c3-7a2063f63184

I have to implement some more logic to add a stop button.

Maybe add dummy buttons for now since I don't see it being optimal to PR as-is (from a UX standpoint)

hanaral avatar Feb 06 '21 10:02 hanaral

Okay let's keep it simple and focus on building, installing and running a project in this PR.

@hanaral collect features in your issue. So that we can split this up into smaller PRs.

meisenzahl avatar Feb 06 '21 10:02 meisenzahl

@meisenzahl Thanks for working on this - it will be useful. Not sure if it is still in progress and should be marked "draft"? Some comments after an initial try:

  • "Run" button should change appearance (or show/hide) depending on whether a compilable project is loaded/focused
  • Need to be able to choose to build only or build and install
  • Tooltip should indicate what action will be taken on which project
  • "Run" button is not active unless you click on a file in the sidebar even if a suitable ("meson.build") file is loaded and highlighted in the sidebar.
  • The "Run" button is not active when the project is collapsed
  • Maybe put "Build" and "Install" options in the project folder context menu?
  • Cannot see output unless running Code in a terminal

jeremypw avatar Feb 06 '21 17:02 jeremypw

I'm not entirely sold on the idea of installing projects systemwide from Code at all. In general, I avoid installing projects compiled from source as much as possible.

Given the fact that we're expecting 3rd party applications to be built as flatpaks in the near future, and flatpak has a much more consistent and predictable build process that supports any build system (meson, cmake, GNU make etc), I think that would be a more worthwhile use of time. And installing/running built flatpaks doesn't have so much potential to mess up the system.

davidmhewitt avatar Feb 06 '21 17:02 davidmhewitt

But Code is for development not for installing things per se. The default should definitely be to build, not install. However, for testing some things need to be installed so that should be available. I agree that installing into a VM or something other than a production machine is probably wise.

jeremypw avatar Feb 06 '21 18:02 jeremypw

Given the fact that we're expecting 3rd party applications to be built as flatpaks in the near future, and flatpak has a much more consistent and predictable build process that supports any build system (meson, cmake, GNU make etc), I think that would be a more worthwhile use of time. And installing/running built flatpaks doesn't have so much potential to mess up the system.

Could you expand a bit more how this relates to using Code? Do you mean Code should only build Flatpaks?

jeremypw avatar Feb 06 '21 18:02 jeremypw

I'm saying that this PR currently adds support for building and installing meson based projects. As you say, to test many of these, they have to be installed which you have to be aware of the consequences of.

Given that most elementary repos now have a Flatpak manifest, and all 3rd party apps that a lot of people probably use Code to develop will be Flatpak based too. It might be more worthwhile having this PR add support for building and running Flatpak packages.

It solves the problem of needing to install the apps (since they're sandboxed) and it has the bonus of being an easier build system format to support in Code.

davidmhewitt avatar Feb 06 '21 19:02 davidmhewitt

@jeremypw thanks for your comments! I will address the points.

I will focus in this PR on being able to build and launch a project through the UI. @davidmhewitt has already noted that system-wide install is not ideal, but necessary in many cases. So I'm open to whether we build the project, install system-wide and launch it, or use Flatpak.

meisenzahl avatar Feb 07 '21 06:02 meisenzahl

Current state

  • Emit a signal for every command line output in ProjectManager
    • could be used to show it in a view in a following PR
  • Add build button
    • only build the project
  • Add stop button
    • stop a process if the project is building or has started
  • Set sensitive for build, run and stop buttons

https://user-images.githubusercontent.com/10796736/107141642-79ec3c00-692a-11eb-9db4-ed435696dbf5.mp4

I'm open to suggestions for the build icon.

meisenzahl avatar Feb 07 '21 07:02 meisenzahl

"Run" button should change appearance (or show/hide) depending on whether a compilable project is loaded/focused

@jeremypw I already tackled this but I could use help for the logic of when a project is activated or a file is selected.

meisenzahl avatar Feb 07 '21 08:02 meisenzahl

@davidmhewitt I agree that FlatPak building and installing needs to be implemented in Code as well. I don't think it needs to be exclusively Flatpak oriented though. There will always be code that is not FlatPak (system level stuff).

To keep PR small the FlatPak features could be a separate PR.

jeremypw avatar Feb 07 '21 11:02 jeremypw

And the system level stuff almost always needs to be started in a specific way:

  • wingpanel: got to kill the currently running instance twice so that gnome-session stops restarting it.

  • gala: got to start with gala --replace to replace the currently running instance and have a way of starting another instance afterwards so you don't get stuck in a session without a window manager.

  • daemons: got to kill the currently running instance

And so on...

Code isn't and can't be aware of these nuances for each system application. So personally I think having a run button for anything that isn't a Flatpak application is a recipe for disaster.

So for most system applications, this run button won't work anyway (because there's probably an already running instance), but it'll be silently overwriting the installed version of the packages with (potentially broken) ones in the background. Then the next time you boot the system, you have no window manager. We might understand what's going on there, but to put a big green run button in a user facing application that hides this complexity doesn't sit well with me.

davidmhewitt avatar Feb 07 '21 12:02 davidmhewitt

@davidmhewitt Yeah, I agree that any install capability needs to be fenced off and not just a single click away. Maybe with a custom pre-install command associated with each project.

I would support not implementing system install for now. A button for initiating a build would still be convenient but without a way of seeing the output is of limited use.

To be honest, I mostly use Geany and do not use the buttons for building/installing - I do everything from the terminal anyway. So perhaps we do need to think a bit more about who we are aiming this feature at and for what purpose.

Maybe in the future all projects will be built as Flatpaks from day one but I would have thought that most projects are started as native then Flatpak'd later.

Asking for @elementary/ux input.

jeremypw avatar Feb 07 '21 12:02 jeremypw

I totally agree that virtualy everything to do with application, component or runtime development should be restricted to flatpak since Code should be able to function like Xcode in a way but instead specifically for elementary and AppCenter. I also think it would be nice to have web dev support tools too, since that is also another huge component of development in general.

hanaral avatar Feb 07 '21 13:02 hanaral

It's kinda out of scope for this issue, but any chance we can get a more rounded play button? That way code could use a modified version of it with some kind of 'developer' symbol on it (a hammer as a badge maybe?)

hanaral avatar Apr 23 '21 17:04 hanaral

Would benefit from https://github.com/elementary/code/issues/991

Possible follow-up PRs

  • Show output of build and run in a TerminalView

meisenzahl avatar Apr 24 '21 12:04 meisenzahl

@jeremypw thanks for your review. Good suggestions, which I have implemented straight away.

Yes, the build/run button sensitivities update if the process is stopped externally.

meisenzahl avatar Apr 25 '21 05:04 meisenzahl

I just want to point out a couple issue that may seem small but are actually quite problematic:

  • Using the sandboxed version of this branch doesn't, and afaik (currently), has no way of working due to the restrictions the command line
  • I've already opened a discussion for this, but at this point the ball needs to get rolling on whether this IDE remains the default text editor or not since the public beta is almost out.

✨️ Solutions (?) ✨️

  1. Maybe try to create a small builtin server on the OS that can run the flatpak-builder and flatpak commands, which Code would ask for over DBus (maybe there is already something that can do this)
  2. Potentially using Writer as a base, try replacing the default text editor with a notes app that can sync with DAV notes and also import/export RTF files

I understand that these may warrant their own issues, but this seems to be a big PR anyway

hanaral avatar Apr 25 '21 13:04 hanaral

It is definitely needed to throw some kind of dialog if the build fails, preferably containing the critical message from the output (I see you are capturing that already). At the moment, the PR fails to flatpak-build itself and fails silently.

jeremypw avatar May 04 '21 14:05 jeremypw

OK, I have now got a flatpak building OK (after managing to install the elementary Sdk and Platform). I note that there is no feedback for a successful build either, though.

One small bug I noticed is that the build/run/stop buttons are insensitive at startup even though there is only one project open and there is an active document. Clicking on that documents source view or sidebar entry or switching tabs does not make the buttons sensitive. Only when a different item is clicked in the sidebar do the buttons become sensitive.

jeremypw avatar May 04 '21 16:05 jeremypw

I am getting an error if I try the "run" command even though the "build" command is successful. error: Build directory /media/jeremy/Shared/shared_data/Vala/gnonograms/.flatpak-builder/rofiles/rofiles-cDbytD not initialised, use flatpak build-init

I get the same error running flatpak-builder --run <build-dir> <manifest> <command> in the terminal. If I run the flatpak build-init I get error: Build directory ./flatpak-build already initialised so I do not know what is going on ...

If I use flatpak build-init --update then it succeeds but the run command still gives the same error.

jeremypw avatar May 04 '21 18:05 jeremypw

Added an error dialog

Screenshot from 2021-05-04 20 39 04

meisenzahl avatar May 04 '21 18:05 meisenzahl

I am getting an error if I try the "run" command even though the "build" command is successful. error: Build directory /media/jeremy/Shared/shared_data/Vala/gnonograms/.flatpak-builder/rofiles/rofiles-cDbytD not initialised, use flatpak build-init

I get the same error running flatpak-builder --run <build-dir> <manifest> <command> in the terminal. If I run the flatpak build-init I get error: Build directory ./flatpak-build already initialised so I do not know what is going on ...

If I use flatpak build-init --update then it succeeds but the run command still gives the same error.

This seems to be a shared mount of VirtualBox. This can lead to errors. Would be great if you build the project in the local file system.

meisenzahl avatar May 04 '21 18:05 meisenzahl

@meisenzahl It is not in VirtualBox although the source is on a different partition to the main system. I have build and installed the flatpak OK in the past but I haven't tried using the --run command before. I'll try moving the code onto the system partition and try again.

jeremypw avatar May 04 '21 18:05 jeremypw

The dialog box looks nice although it would be better if the details showed 80 characters as the messages can be lengthy.

jeremypw avatar May 04 '21 18:05 jeremypw

Not sure how hard it would be but maybe the secondary text could be taken from the final critical message in the output?

jeremypw avatar May 04 '21 18:05 jeremypw

@jeremypw et voilà !

Screenshot from 2021-05-05 12 43 40

meisenzahl avatar May 05 '21 10:05 meisenzahl

Looks good to me! Need to see what the UX team says. Hopefully the problem I have with the run command is due to my setup not this PR. I'll test again when I have got manual build/run working.

jeremypw avatar May 05 '21 12:05 jeremypw