Fallback to conflicts when merge is done in non-tty environment (read… #69
Conversation
… GUI app). Merge in GUI apps led to silent failure to merge .s7substate properly. It left .s7substate in a visually non-conflict state. Now, if merge happens in non-tty env (we cannot interactively ask user what to do), we fallback to the conflict.
| @@ -21,13 +21,15 @@ fi | |||
| # | |||
| # INSTALL_PATH: | |||
| # "The directory in which to install the build products. This path is prepended by the DSTROOT." | |||
| # | |||
| # Starting with Sonoma, xcodebuild install (without clean) stopped replacing existing binary in the DSTROOT. | |||
There was a problem hiding this comment.
А это был дивный сюрприз. Собираю – вижу старый вывод. Очень странная дичь. Может быть это на самом деле Xcode 15, уже нет сил разбираться.
| @@ -48,7 +48,7 @@ echo | |||
| echo cherry-pick | |||
|
|
|||
| # cherry-pick only last commit from 'release/documents-7.2.4' | |||
| echo M | git cherry-pick release/documents-7.2.4 | |||
There was a problem hiding this comment.
Т.к. теперь if (!istty(stdin)) -> Conflict, то в интеграционных тестах полностью полагаюсь на S7_MERGE_DRIVER_RESPONSE
| @@ -474,19 +474,7 @@ - (void)testBothSideUpdateSubrepoToDifferentRevisionConflictResolution_Merge { | |||
|
|
|||
| ++callNumber; | |||
|
|
|||
| if (1 == callNumber) { | |||
There was a problem hiding this comment.
Это были тесты невалидного ввода от блока резолва. Теперь такого быть не может.
| if (!isatty(fileno(stdin))) { | ||
| fprintf(stderr, | ||
| "\033[31m" | ||
| "to run interactive merge of s7 subrepos, please run the following command in Terminal:\n" |
There was a problem hiding this comment.
Не уверен, что все пользователи GUI смогут это прочитать, но я старался.
Например, Sourcetree не показывает stderr :know-how-can:
Мне конечно все это не нравится, потому чут люди, как пить дать, начнут лазить в .s7substate руками, и это для них плохо кончится, т.к. они не обновят соотв-но .s7control.
Может еще завтра что-то по этому поводу придумаю. Можно написать в .s7substate коментарий, но не факт что его кто-то прочитает. А кодов дописать придется.
| @@ -27,14 +27,14 @@ - (instancetype)init { | |||
| { | |||
| void *self __attribute((unused)) __attribute((unavailable)); | |||
There was a problem hiding this comment.
Когда вы поревьюите, я вынесу все тело этого блока в метод. Не хочу это сразу делать, чтобы диф не раздувать.
| if (response.length > 0 && | ||
| [S7ConfigMergeDriver mergeResolution:&resolution | ||
| forUserInput:[response characterAtIndex:0] | ||
| allowedResolutions:~0]) |
There was a problem hiding this comment.
non-blocking: Может добавить S7ConflictResolutionOptionAll?
There was a problem hiding this comment.
Не хотел ради тестовых кодов заморачиваться.
На самом деле вообще вопрос зачем я тут Options именно нагородил. Можно обойтись простым enum-ом, и сетом возможных значений. Option наталкивает на лишние мысли, что можно замаскировать несколько значений. Снова таки, решил уже не раздувать ПР, и пожить так.
…to a separate method
|
Как и угрожал, вынес кода из блока в отдельный метод(ы). Логику не затрагивал. Пройдут чеки, и замержу. |
… GUI app).
При мерже через GUI
.s7substateпомечался как конфликтный, но в теле файла конфликта не было. Там оставалось текущее содержание.На самом деле,
S7ConflictResolutionOption resolution;не инициализированная, так что вполне возможно, что результат был рандомный, и мне везло получатьS7ConflictResolutionOptionKeepLocal.Теперь, если не tty(stdin), мы оставляем в файле конфликты для всех разошедшихся сабреп.
Попутно порефакторил кода. Такое ощущение, что когда я их писал, я чрезмерно упоролся, и боролся с проблемами, которых быть не может – типа что
resolveConflictBlockвернет не то значение. Перенес валидацию ввода пользователя в сам блок, и на вызывающей стороне стало все проще.Заодно убрал странный вечный цикл. На практике он приводил к тому, что по Ctrl+C невозможно было выйти из режима "what do you want to do?". У меня это давно было записано, но все руки не доходили.