Develop by tiennou · Pull Request #117 · gitx/gitx

tiennou · 2018-02-13T23:51:14Z

Sorry for the dull name.

This is the result of a few years worth of ~~love~~ low-level work on GitX innards. I'm now running that branch for the time being (I'll likely find bugs and squash them).

The plan is :

make sure it's not more buggy than it was (heh)
merge this in a few weeks
finish the release process tweaks (almost there)
enjoy our first release since a long time 🍻

Highlights are :

reworked task-management system (see PBTask)
finalized the separation of PBGitRepository from anything document-related.
reworked the sheets API

So if developers/users want to give a hand in testing, please give this a spin !

@gitx/core @ksuther @mangerlahn @muhqu

Task with lots of output would block when their pipe would get full. Drain our side of the pipe from time to time so the task can continue tasking. Ref gitx#97

We might get the termination/completion notification before we actually had a chance to read anything, so we would reallocate `standardOutputData` under the readability handler.

ksuther · 2018-02-14T05:14:52Z

Thank you for all the updates!

I'm now running this with @mangerlahn's branch merged in (it merged cleanly other than stray xcodeproj and xib conflicts).

I'd suggest moving to the latest ObjectiveGit in this branch as well. The current version of ObjectiveGit/libgit2 that gitx is depending on is pretty old and has some problems that results in poor performance on repositories with lots of submodules (libgit2/libgit2#4016).

aharrison24 · 2018-02-14T15:29:11Z

I'm trying out the develop branch now. Just had it crash when I tried to perform a push from the UI. The push seems to have happened, but GitX crashed immediately after. Here's the error report:
https://gist.github.com/aharrison24/713ab5aac9f9eb5e6bfd1de2368d78ed

Key piece of info appears to be:

*** Terminating app due to uncaught exception 'WebKitThreadingException', reason: '-[WebView windowScriptObject] was called from a secondary thread'
terminating with uncaught exception of type NSException
abort() called

ksuther · 2018-02-14T21:06:31Z

PBTask throws an exception when trying to discard changes to a file. It looks like NSPipe doesn't allow calling offsetInFile on its file handle.

*** -[NSConcreteFileHandle offsetInFile]: Illegal seek
(null)

* thread #32, queue = 'com.apple.NSFileHandle.fd_monitoring', stop reason = breakpoint 1.1
    frame #0: 0x00007fff7b35cc46 libobjc.A.dylib`objc_exception_throw
    frame #1: 0x00007fff5474c9ed CoreFoundation`+[NSException raise:format:] + 205
    frame #2: 0x00007fff5679ecbf Foundation`-[NSConcreteFileHandle offsetInFile] + 78
  * frame #3: 0x00000001000f4291 GitX`__48-[PBTask performTaskOnQueue:terminationHandler:]_block_invoke.157(.block_descriptor=<unavailable>, handle=0x000060c0005f6d40) at PBTask.m:149
    frame #4: 0x00007fff5682af68 Foundation`__33-[NSConcreteFileHandle _monitor:]_block_invoke + 50
    frame #5: 0x000000010037a692 libclang_rt.asan_osx_dynamic.dylib`__wrap_dispatch_source_set_event_handler_block_invoke + 290
    frame #6: 0x00000001017f8d1f libdispatch.dylib`_dispatch_client_callout + 8
    frame #7: 0x000000010180d3c3 libdispatch.dylib`_dispatch_continuation_pop + 940
    frame #8: 0x00000001017fb41a libdispatch.dylib`_dispatch_source_invoke + 658
    frame #9: 0x000000010180e8c4 libdispatch.dylib`_dispatch_queue_serial_drain + 425
    frame #10: 0x0000000101800790 libdispatch.dylib`_dispatch_queue_invoke + 322
    frame #11: 0x000000010180ffbb libdispatch.dylib`_dispatch_root_queue_drain_deferred_wlh + 785
    frame #12: 0x0000000101814e94 libdispatch.dylib`_dispatch_workloop_worker_thread + 892
    frame #13: 0x000000010187502b libsystem_pthread.dylib`_pthread_wqthread + 980
    frame #14: 0x0000000101874c45 libsystem_pthread.dylib`start_wqthread + 13

This reverts commit 638e567.

This is a stop-gap measure: calling -reloadRefs from a background queue wrecks the KVO system because we're doing weird things with our notifications when reloading references.

tiennou · 2018-02-21T01:23:00Z

Heads-up ! Those two issues should be gone now (first 2 commits above). I've also reworked the error reporting so we get (hopefully) nicer error messages when tasks fail.

ksuther · 2018-02-21T01:29:23Z

Discard changes is working again. Another regression that I found: Commit hooks aren't working. I have a pre-commit and commit-msg hook and no warning appears anymore when they fail. I haven't had a chance to debug it myself yet.

With HEAD detached, the parseSymbolicReference code would find an OID as an unresolvedTarget, which would result in a nil branchRef, thus a nil _headOID, and a subsequent crash when trying to add this nil OID to a set "somewhere". Simplify all that by just using whatever our underlying HEAD reference says.

tiennou · 2018-02-25T21:21:13Z

I'm merging, just because I find it cumbersome to follow too much branches.

The issues you (various) reported should all be fixed, so if you find that something is not working, please open an issue (for real this time).

tiennou and others added 30 commits February 13, 2018 23:11

Move the task wrangling out to PBTask

5f63934

Add environment managing to PBTask

d02a281

Add an inDirectory variant to our nicer methods

d5645d9

Give access to the output data as an UTF-8 string

db8f097

Stash the task's output in the error when execution fails

32fe24a

Add a way to schedule blocks on specific queues

097c456

Add a -description

d35616d

Actually report whether the task was successful or not

5ee7be1

Stringify the task's arguments when reporting errors

42cc839

Blacklist some more env vars

5abdea0

Fix hang caused by task data output size

affa0a7

Task with lots of output would block when their pipe would get full. Drain our side of the pipe from time to time so the task can continue tasking. Ref gitx#97

Break retain cycle caused by retaining our task inside blocks

96485d8

Task debug logging

a4102a3

Don't close our read handle if there's still data available

c793307

Fix a double-free by synchronizing task reading

3d35946

We might get the termination/completion notification before we actually had a chance to read anything, so we would reallocate `standardOutputData` under the readability handler.

Comment that we might need a check when writing to tasks

638e567

Split the git-using methods in a category

ff5b674

Change a few methods to PBTask

60a9500

Import PBTask from the category so it doesn't get sprinkled everywhere

424b462

Switch the history search to PBTask

8a5bbd3

More PBTask-ing

9dcedbd

Switch PBWebController to PBTask

69931be

Switch PBGitXProtocol to PBTask

1a8bf1c

De-deprecate

200c14e

Removed unnecessary #imports of PBEasyPipe.h

0943934

Remove extra calls to readDataToEndOfFile that were causing exceptions

85909e5

Add NSError parameters to PBGitRepository methods

f65baf9

Remove backward-compatibility layer

aa9c41f

PBGitRepository isn't part of the controller hierarchy anymore

59a85f1

Unused method

25281b0

tiennou added 4 commits February 15, 2018 00:22

Revert "Comment that we might need a check when writing to tasks"

82d5c27

This reverts commit 638e567.

Ensure KVO notifications are sent from the main thread

19cf7be

This is a stop-gap measure: calling -reloadRefs from a background queue wrecks the KVO system because we're doing weird things with our notifications when reloading references.

Enhance error messages when additional data is available

fede5a7

Beautify error messages

d9b3333

tiennou force-pushed the develop branch from a098df3 to d9b3333 Compare February 15, 2018 03:22

tiennou added 4 commits February 15, 2018 04:50

Unused method

1d4a7e8

Use Objective-Git to get the list of remotes

8866c52

Correct reversed postIndexRefresh argument

1e93339

Remove unused deprecated PBGitBinarySupport methods

14f473a

tiennou added 11 commits February 23, 2018 23:14

Incorrect return on error function

5e4d922

Reenable deprecation warnings

0913e06

Update last few "trivial" uses to PBTask

78a7388

Make our context menu items use the responder chain

57365cf

Move IBActions out of PBRefController into PBGitHistoryController

c16d5ad

Rewire delete-reference sheet

004d7b7

Copy-pasting things from the history list is handled at the window level

08c77bd

Rewire diff context menu item

cbcba80

Rewire stash actions

2e63424

Rewire tag info action

40c88c8

tiennou merged commit 1ec4f02 into gitx:master Feb 25, 2018

tiennou deleted the develop branch February 25, 2018 21:21

robinwhittleton mentioned this pull request Feb 25, 2018

Build not working now that develop branch has been merged #118

Closed

tiennou mentioned this pull request Sep 5, 2018

Locking up as of 0378e8e #156

Closed

Navigation Menu

Repository navigation

Develop - #117

Develop#117
tiennou merged 69 commits into
gitx:masterfrom
tiennou:develop

tiennou commented Feb 13, 2018

Uh oh!

ksuther commented Feb 14, 2018

Uh oh!

aharrison24 commented Feb 14, 2018

Uh oh!

ksuther commented Feb 14, 2018

Uh oh!

tiennou commented Feb 21, 2018

Uh oh!

ksuther commented Feb 21, 2018

Uh oh!

tiennou commented Feb 25, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tiennou commented Feb 13, 2018

Uh oh!

ksuther commented Feb 14, 2018

Uh oh!

aharrison24 commented Feb 14, 2018

Uh oh!

ksuther commented Feb 14, 2018

Uh oh!

tiennou commented Feb 21, 2018

Uh oh!

ksuther commented Feb 21, 2018

Uh oh!

tiennou commented Feb 25, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants