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 761549 - replace g_io_scheduler with GTask
replace g_io_scheduler with GTask
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-04 12:19 UTC by Razvan Chitu
Modified: 2016-02-08 16:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
file-operations: replace io_scheduler in helper functions (4.41 KB, patch)
2016-02-04 12:19 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler with GTask API (21.46 KB, patch)
2016-02-04 12:19 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler with GTask API (21.61 KB, patch)
2016-02-04 12:45 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler with GTask API (22.17 KB, patch)
2016-02-08 15:52 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler with GTask API (22.17 KB, patch)
2016-02-08 16:01 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler in helper functions (4.70 KB, patch)
2016-02-08 16:27 UTC, Razvan Chitu
none Details | Review
file-operations: replace io_scheduler with GTask API (21.61 KB, patch)
2016-02-08 16:29 UTC, Razvan Chitu
committed Details | Review
file-operations: replace io_scheduler in helper functions (4.70 KB, patch)
2016-02-08 16:34 UTC, Razvan Chitu
committed Details | Review

Description Razvan Chitu 2016-02-04 12:19:26 UTC
see patches.
Comment 1 Razvan Chitu 2016-02-04 12:19:30 UTC
Created attachment 320425 [details] [review]
file-operations: replace io_scheduler in helper functions

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. In order to make this change possible, a
first step would be to replace the scheduler in the helper functions called
during the jobs.

Replace g_io_scheduler_send_to_mainloop with g_main_context_invoke in helper
functions. Since g_main_context_invoke is not blocking, add a mutex and a
condition so the current thread is blocked until the operation is completed in
the main loop.
Comment 2 Razvan Chitu 2016-02-04 12:19:36 UTC
Created attachment 320426 [details] [review]
file-operations: replace io_scheduler with GTask API

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. The helper functions used in the
operations have been changed in a previous patch so it is now possible to port
the jobs themselves to the new API.

The job structures are now data for tasks, which are handled by the existing
functions in separate threads. For finalizing the operations, the existing
"job_done" functions are now used as callbacks.
Comment 3 Razvan Chitu 2016-02-04 12:45:14 UTC
Created attachment 320427 [details] [review]
file-operations: replace io_scheduler with GTask API

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. The helper functions used in the
operations have been changed in a previous patch so it is now possible to port
the jobs themselves to the new API.

The job structures are now data for tasks, which are handled by the existing
functions in separate threads. For finalizing the operations, the existing
"job_done" functions are now used as callbacks.
Comment 4 Carlos Soriano 2016-02-04 20:17:57 UTC
As afraid as I am with this change, patches looks good to me. I created a gnome-3-20 branch, so we can commit to master and this code won't be there.
Comment 5 Carlos Soriano 2016-02-04 20:18:12 UTC
Review of attachment 320427 [details] [review]:

good!
Comment 6 Carlos Soriano 2016-02-04 20:22:20 UTC
Review of attachment 320425 [details] [review]:

See the comment inline. Would be nice to have this in the commit message as well. The commit message nor the code explains why we need to make it blocking.

::: libnautilus-private/nautilus-file-operations.c
@@ +1065,2 @@
 	int result;
+	gboolean completed;

Can you add a small comment here to explain why we need this? Something similar to the commit message.
Like:
"Needed to block the thread to wait for the response from the dialogs"
Comment 7 Razvan Chitu 2016-02-08 15:52:17 UTC
Created attachment 320639 [details] [review]
file-operations: replace io_scheduler with GTask API

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. The helper functions used in the
operations have been changed in a previous patch so it is now possible to port
the jobs themselves to the new API.

The job structures are now data for tasks, which are handled by the existing
functions in separate threads. For finalizing the operations, the existing
"job_done" functions are now used as callbacks.
Comment 8 Carlos Soriano 2016-02-08 15:56:59 UTC
Review of attachment 320639 [details] [review]:

::: libnautilus-private/nautilus-file-operations.c
@@ +1067,3 @@
 	gboolean show_all;
+	/* dialogs are ran from operation threads, which need to be blocked until
+	   the user gives a valid response */

The style is with first capital letter and a asterisk every line, finalizing in a newline with an asterisk aligned and the slash.
Like:
/* Test,
 * 124
 */
Comment 9 Razvan Chitu 2016-02-08 16:01:52 UTC
Created attachment 320640 [details] [review]
file-operations: replace io_scheduler with GTask API

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. The helper functions used in the
operations have been changed in a previous patch so it is now possible to port
the jobs themselves to the new API.

The job structures are now data for tasks, which are handled by the existing
functions in separate threads. For finalizing the operations, the existing
"job_done" functions are now used as callbacks.
Comment 10 Carlos Soriano 2016-02-08 16:07:41 UTC
Review of attachment 320640 [details] [review]:

Yep, thanks!
Comment 11 Razvan Chitu 2016-02-08 16:27:23 UTC
Created attachment 320642 [details] [review]
file-operations: replace io_scheduler in helper functions

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. In order to make this change possible, a
first step would be to replace the scheduler in the helper functions called
during the jobs.

Replace g_io_scheduler_send_to_mainloop with g_main_context_invoke in helper
functions. Since g_main_context_invoke is not blocking, add a mutex and a
condition so the current thread is blocked until the operation is completed in
the main loop.
Comment 12 Razvan Chitu 2016-02-08 16:29:45 UTC
Created attachment 320643 [details] [review]
file-operations: replace io_scheduler with GTask API

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. The helper functions used in the
operations have been changed in a previous patch so it is now possible to port
the jobs themselves to the new API.

The job structures are now data for tasks, which are handled by the existing
functions in separate threads. For finalizing the operations, the existing
"job_done" functions are now used as callbacks.
Comment 13 Razvan Chitu 2016-02-08 16:34:34 UTC
Created attachment 320644 [details] [review]
file-operations: replace io_scheduler in helper functions

Nautilus file operations are implemented as asynchronous jobs scheduled using
g_io_scheduler. Since g_io_scheduler has been deprecated, these operations
should be using the simpler GTask API. In order to make this change possible, a
first step would be to replace the scheduler in the helper functions called
during the jobs.

Replace g_io_scheduler_send_to_mainloop with g_main_context_invoke in helper
functions. Since g_main_context_invoke is not blocking, add a mutex and a
condition so the current thread is blocked until the operation is completed in
the main loop.
Comment 14 Carlos Soriano 2016-02-08 16:36:45 UTC
Review of attachment 320644 [details] [review]:

Yeah
Comment 15 Carlos Soriano 2016-02-08 16:37:02 UTC
Review of attachment 320643 [details] [review]:

LGTM
Comment 16 Carlos Soriano 2016-02-08 16:37:02 UTC
Review of attachment 320643 [details] [review]:

LGTM
Comment 17 Carlos Soriano 2016-02-08 16:37:04 UTC
Review of attachment 320643 [details] [review]:

LGTM
Comment 18 Razvan Chitu 2016-02-08 16:42:35 UTC
Attachment 320643 [details] pushed as 1913ec0 - file-operations: replace io_scheduler with GTask API
Attachment 320644 [details] pushed as 5a6f869 - file-operations: replace io_scheduler in helper functions