After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 771145 - Run button does not work
Run button does not work
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
3.21.x
Other Linux
: Normal major
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-10 01:44 UTC by Mohammed Sadiq
Modified: 2017-02-25 03:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
autotools: don't miss lines when parsing make output (2.15 KB, patch)
2017-02-17 13:00 UTC, Mohammed Sadiq
none Details | Review
autotools: don't parse empty variables (840 bytes, patch)
2017-02-19 02:48 UTC, Mohammed Sadiq
committed Details | Review
autotools: don't skip parsing the last line (1.90 KB, patch)
2017-02-19 02:48 UTC, Mohammed Sadiq
committed Details | Review
autotools: guard against NULL values (1.22 KB, patch)
2017-02-19 03:28 UTC, Mohammed Sadiq
committed Details | Review
autotools: fix finding installation targets (3.40 KB, patch)
2017-02-19 11:37 UTC, Mohammed Sadiq
none Details | Review
autotools: fix finding installation targets (3.39 KB, patch)
2017-02-19 12:27 UTC, Mohammed Sadiq
none Details | Review
autotools: fix finding installation targets (3.39 KB, patch)
2017-02-19 12:49 UTC, Mohammed Sadiq
none Details | Review
makecache: fix replacement of -C <directory> (3.44 KB, patch)
2017-02-25 03:46 UTC, Christian Hergert
committed Details | Review
makecache: handle out of order make results (4.53 KB, patch)
2017-02-25 03:46 UTC, Christian Hergert
committed Details | Review

Description Mohammed Sadiq 2016-09-10 01:44:36 UTC
The run button (C-F5), which is supposed to run the application does not work.

This was tested with the inbuilt gnome application (in C) created using New Project.
Comment 1 Mohammed Sadiq 2016-10-24 14:05:27 UTC
I can't reproduce this anymore. Hopefully, it's fixed.

Thanks.
Comment 2 Mohammed Sadiq 2016-11-04 13:48:43 UTC
It seems like the issue is not resolved completely. It seems to happen randomly.

The following issues are logged to terminal when run button is clicked. Different issues logged when on jhbuild and on system installed versions.

In both cases I have used 'Host operating system' as runtime.

From jhbuild (jhbuild run gnome-builder):
19:11:58.0807                             Ide[9683]:  WARNING: Failed to locate build target

From system installed gnome-builder (Debian testing, version 3.22.1):
19:06:14.0355                             Ide[30637]:  WARNING: A build is already in progress

I wasn't able to bring up a way that can always reproduce this issue. Though the following method almost always brings up the issue:

1. Build Some application (tested with inbuild GNOME template example in C)
2. Run the application using the run button.
3. Close the application, and try to run application again.

Result:
The application did not run the second time.

Please let me know if more information is needed.

Thanks.
Comment 3 Christian Hergert 2016-11-04 19:50:56 UTC
Please test with either git-master or 3.22.2. I believe it was fixed since 3.22.1.
Comment 4 Mohammed Sadiq 2016-11-04 23:19:12 UTC
(In reply to Christian Hergert from comment #3)
> Please test with either git-master or 3.22.2. I believe it was fixed since
> 3.22.1.

The jhbuild version was git master. Anyway I shall test 3.22.2 and inform once its packaged in Debian testing.

Thanks.
Comment 5 Mohammed Sadiq 2016-11-12 13:02:30 UTC
Debian tested has updated gnome-builder to version 3.22.2. And I can't reproduce this bug anymore.

Marking as fixed.
Comment 6 Mohammed Sadiq 2016-12-05 01:56:30 UTC
I still seems to have this problem. And I saw that this happened after uninstalling the packagekit package. Installing the packagekit package fixes the problem again (Though it sometimes doesn't run for some reason).

I don't know why packagekit is required to run the program. Sorry, reopening again.
Comment 7 Christian Hergert 2016-12-05 02:07:27 UTC
Have you tested with 3.22.3 or git-master?

We don't do anything with packagekit.
Comment 8 Christian Hergert 2016-12-22 01:31:02 UTC
This seems to work with me on 3.22.4
Comment 9 Mohammed Sadiq 2016-12-22 07:19:36 UTC
(In reply to Christian Hergert from comment #8)
> This seems to work with me on 3.22.4

Yup. For me too, but not always. Sometimes, run button never works. In some cases it works flawlessly for every click. I'm trying to find a way to reproduce this in a reliable way. But everything is just random now. Don't close this bug now. 

Thanks.
Comment 10 Martin Ejdestig 2017-01-31 16:19:14 UTC
I have this problem as well with 3.22.4 in Arch. The run button never does anything. I have tested with two different Meson projects.

(They both have multiple executables though. Am I supposed to get a list of executables in that case? Should I file a separate bug?)
Comment 11 Patrick Griffis (tingping) 2017-02-01 03:49:54 UTC
(In reply to Martin Ejdestig from comment #10)
> I have this problem as well with 3.22.4 in Arch. The run button never does
> anything. I have tested with two different Meson projects.
> 
> (They both have multiple executables though. Am I supposed to get a list of
> executables in that case? Should I file a separate bug?)

It just uses the first executable it finds that is installed in the /bin dir
Comment 12 Martin Ejdestig 2017-02-01 18:03:48 UTC
> It just uses the first executable it finds that is installed in the /bin dir

But looking at the plugin code it does targets.append() anyway, right?

Also, would be nice to be able to choose what to run somehow (dropdown arrow next to run button?) if there are multiple executable targets. E.g. a project that is a library may have multiple examples and test executables (not meant to be installed).
Comment 13 Christian Hergert 2017-02-02 01:05:55 UTC
We already have a dropdown arrow next to the run button for what to run the project with (Run, Run with Profiler, eventually with Debugger, etc). But we might be able to use the submenu system with GtkPopoverMenu to make the equivalent of a submenu with the list of available targets.
Comment 14 Patrick Griffis (tingping) 2017-02-02 06:56:01 UTC
(In reply to Martin Ejdestig from comment #12)
> > It just uses the first executable it finds that is installed in the /bin dir
> 
> But looking at the plugin code it does targets.append() anyway, right?

Yea Meson has the nice property of being trivial to extract every target so we just return the full list for when that is useful in the future. The Autotools plugin has to do more work and just extracts what it is specifically told to.
Comment 15 Martin Ejdestig 2017-02-02 10:10:00 UTC
> We already have a dropdown arrow next to the run button for what to run the
> project with (Run, Run with Profiler, eventually with Debugger, etc). But we
> might be able to use the submenu system with GtkPopoverMenu to make the
> equivalent of a submenu with the list of available targets.

Ah, right. But yes, some way to choose what to run would be nice. :)

> Yea Meson has the nice property of being trivial to extract every target so we
> just return the full list for when that is useful in the future.

OK.

I have never seen anything in "Run Output" when pressing the run button.

Come to think of it, I actually have a Meson project that only builds a static library and a unit test executable that uses googletest. Test output is not printed. Just added a busy loop that never exits in one of the tests to see if the process was run at all. It is not. Running the executable in ~/.cache/gnome-builder from the terminal I can verify that it hangs forever.

Any more information I can provide to help with this?
Comment 16 Patrick Griffis (tingping) 2017-02-02 11:06:42 UTC
(In reply to Martin Ejdestig from comment #15)
> I have never seen anything in "Run Output" when pressing the run button.
> 
> Come to think of it, I actually have a Meson project that only builds a
> static library and a unit test executable that uses googletest. Test output
> is not printed. Just added a busy loop that never exits in one of the tests
> to see if the process was run at all. It is not. Running the executable in
> ~/.cache/gnome-builder from the terminal I can verify that it hangs forever.

Well you can't run libraries and tests are not included in the general target list nor are in they installed to /bin so they will not run.

Meson has a tool to expose running tests and gathering output Builder just needs to add an interface to that and do the same for Autotools.
Comment 17 Martin Ejdestig 2017-02-02 12:29:06 UTC
I removed test() and just kept executable(). Still does not work.
Comment 18 Martin Ejdestig 2017-02-02 18:40:05 UTC
> and tests are not included in the general target list

Hum, mesonintrospect --targets shows the executable regardless of wheter i removed test() or not. So I do not understand what you mean here. Anyway, did not work to remove it so does not really matter. :)
Comment 19 Patrick Griffis (tingping) 2017-02-02 20:43:59 UTC
(In reply to Martin Ejdestig from comment #18)
> > and tests are not included in the general target list
> 
> Hum, mesonintrospect --targets shows the executable regardless of wheter i
> removed test() or not. So I do not understand what you mean here. Anyway,
> did not work to remove it so does not really matter. :)

Ah you are right, it shows the executables but the functionality for *running* tests is in `mesontest`. Anyway since the test executables are not installed they never make it to the point where builder would try to run them. This is all very basic stuff that needs to be expanded.
Comment 20 Martin Ejdestig 2017-02-03 09:02:28 UTC
Oh, so an executable _has_ to be installed to be runnable? I thought the logic in the plugin just prioritized executables put in /bin. Two things then:

1) Should it be possible to run executables without requiring the build system to do install? Would be nice, I think.

2) If there is nothing to run this should be reflected in the UI somehow, right?
Comment 21 Christian Hergert 2017-02-05 22:06:14 UTC
(In reply to Martin Ejdestig from comment #20)
> Oh, so an executable _has_ to be installed to be runnable?

Yes. But by "runnable" we mean using the run button at the top. There is no reason we cannot provide alternate interfaces for things like unit tests. In fact, I'd like to see someone do a SoC project this year to add a unit testing panel and test runner abstraction that the build systems can implement.

> 1) Should it be possible to run executables without requiring the build
> system to do install? Would be nice, I think.

No. We do not want people to continue doing hacks to run out of tree. If we support this, it will be through a very specific abstraction designed for unit testing. And in the meson case, we would expect mesontest to handle that for us.

> 2) If there is nothing to run this should be reflected in the UI somehow,
> right?

That would be a nice improvement.
Comment 22 Mohammed Sadiq 2017-02-17 13:00:32 UTC
Created attachment 346067 [details] [review]
autotools: don't miss lines when parsing make output

New targets where added only if the key doesn't end with "dir".
So when parsing make output, if the end lines end with "dir" as key
they are not added to targets. This caused missing bin dirs.

This commit allows to parse the whole output once, and update the
targets based on the output separately, which would reduce the issue
(though some races are still present).
Comment 23 Christian Hergert 2017-02-18 02:23:34 UTC
I don't think this should be necessary.

Is it possible that commit 91b9b1da8c460546f76795a4bd885e678df05661 could have been affecting it?

I just tested a new project quick and it seems to be working.
Comment 24 Mohammed Sadiq 2017-02-18 04:14:53 UTC
(In reply to Christian Hergert from comment #23)
> I don't think this should be necessary.
> 
> Is it possible that commit 91b9b1da8c460546f76795a4bd885e678df05661 could
> have been affecting it?
> 
> I just tested a new project quick and it seems to be working.

The issue is still present. I still don't exactly know why it is happening. I shall try to investigate further and report, if I find anything fishy.

Until then, let the patch attached above be not committed (I can't mark it obsolete).

Thanks.
Comment 25 Mohammed Sadiq 2017-02-19 02:48:06 UTC
Created attachment 346170 [details] [review]
autotools: don't parse empty variables
Comment 26 Mohammed Sadiq 2017-02-19 02:48:34 UTC
Created attachment 346171 [details] [review]
autotools: don't skip parsing the last line

New targets where added only if the key doesn't end with "dir".
So when parsing make output, if the end lines end with "dir" as key
they are not added to targets. This caused missing bin dirs.

This commit makes makecache not skip the processing of last line.
Comment 27 Christian Hergert 2017-02-19 02:50:39 UTC
Review of attachment 346170 [details] [review]:

LGTM
Comment 28 Christian Hergert 2017-02-19 02:51:31 UTC
Review of attachment 346171 [details] [review]:

Nice, I like it. LGTM
Comment 29 Christian Hergert 2017-02-19 02:52:32 UTC
Thanks for tracking this pesky one down!

Attachment 346170 [details] pushed as e44e6da - autotools: don't parse empty variables
Attachment 346171 [details] pushed as ef4e4b7 - autotools: don't skip parsing the last line
Comment 30 Mohammed Sadiq 2017-02-19 03:28:46 UTC
Created attachment 346172 [details] [review]
autotools: guard against NULL values

This is against master
Comment 31 Mohammed Sadiq 2017-02-19 11:36:19 UTC
Sorry. Due to some races, issues are still present. And on an average, this happens once every 10 runs, or so.

A patch is incoming.

Thanks
Comment 32 Mohammed Sadiq 2017-02-19 11:37:47 UTC
Created attachment 346179 [details] [review]
autotools: fix finding installation targets

Due to some races, the interesting targets like bin_PROGRAM may be
parsed first before bindir is parsed. This may cause failure in finding
installation targets.

This commit parses the make output twice, but only when there is chance for a miss,
and tries to find the installation directory.
Comment 33 Mohammed Sadiq 2017-02-19 12:27:26 UTC
Created attachment 346180 [details] [review]
autotools: fix finding installation targets

Due to some races, the interesting targets like bin_PROGRAM may be
parsed first before bindir is parsed. This may cause failure in finding
installation targets.

This commit parses the make output twice, but only when there is chance for a miss,
and tries to find the installation directory.
Comment 34 Mohammed Sadiq 2017-02-19 12:49:42 UTC
Created attachment 346185 [details] [review]
autotools: fix finding installation targets

Due to some races, the interesting targets like bin_PROGRAM may be
parsed first before bindir is parsed. This may cause failure in finding
installation targets.

This commit parses the make output twice, but only when there is chance for a miss,
and tries to find the installation directory.

A bit of µOptimization won't hurt. :)
Comment 35 Christian Hergert 2017-02-25 03:46:53 UTC
Created attachment 346692 [details] [review]
makecache: fix replacement of -C <directory>

This was pretty complex, for no real good reason anymore. It was probably
due to needing to be relative for when we end up in a different build
directory (such as flatpak), but we map in all those directories now,
so we should be okay with using the proper build directory. Meaning we
can just simply replace the argument after -C.
Comment 36 Christian Hergert 2017-02-25 03:46:59 UTC
Created attachment 346693 [details] [review]
makecache: handle out of order make results

If we get out-of-order results from our request for variables from make,
we need to resolve them after all lines have been returned. This just does
everything as a two-pass operation, avoiding mutation on the first pass.
Comment 37 Christian Hergert 2017-02-25 03:48:52 UTC
Alright, this should be fixed now, for reals. :)

Attachment 346692 [details] pushed as 22fa2e4 - makecache: fix replacement of -C <directory>
Attachment 346693 [details] pushed as 631f872 - makecache: handle out of order make results