Как у меня украли авторство патча / forpes.ru

Главная
Как у меня украли авторство патча

Как у меня украли авторство патча +47

27.09.2023 18:27

PatientZero 95 22000 Источник

Контекст

Примерно полтора года назад я попросил у своей бывшей компании разрешения поработать над проблемой, влиявшей на возможности отладки нашего проекта: gdbserver не мог отлаживать многопоточные приложения, работающие на архитектуре PowerPC32. Подключение к gdbserver было поломано, и он больше не мог контролировать сессию отладки. Разные люди исследовали эту проблему до меня, и у меня уже был неплохой фундамент, но мы всё равно не знали точно, в каком конкретно программном компоненте таится проблема: это мог быть тулчейн, gdbserver, ядро Linux или собственные патчи, которые мы устанавливали поверх дерева ядра. От нахождения первопричины мы находились довольно далеко.

Исследование проблемы

Изучив уже имевшиеся результаты анализа этой проблемы и воспользовавшись своими навыками гугл-фу, я сделал первое открытие: электронную переписку, в которой не только описывались те же симптомы, что и у нашей проблемы, но и указывался конкретный коммит, с которого она появилась. Патч, вносивший баг, перемещал определение thread_struct thread из середины task_struct в конец — казалось бы, невинное изменение.

После отладки этой проблемы Холгер Бранк заметил следующее:

По моим наблюдениям, gdbserver отправляет для каждого потока SIGSTOP ядру и ждёт ответа. Ядро получает все сигналы, но в случае ошибки отвечает только на некоторые из них. Затем это сопоставляется с моим выводом “ps”, так как я вижу, что некоторые потоки находятся не в состоянии pthread_stop, а потом gdbserver переходит в состояние ожидания.

На низком уровне проблема заключалась в том, что после взаимодействия с gdbserver некоторые потоки находились в ошибочном состоянии процесса и gdbserver больше не мог ими управлять.

Я потратил 3-4 дня на чтение описаний коммитов, связанных с архитектурой и с изменениями в task_struct, пытаясь разобраться, была решена ли эта проблема в последующих версиях ядра (спойлер: не была). Я перемещал thread_struct thread, чтобы определить, когда возникла проблема, и использовал pahole для изучения структуры task_struct. Я воспользовался ftrace, чтобы разобраться, когда диспетчеризировались потоки отлаживаемого процесса, и так понял, что это может быть проблема повреждения памяти: зависавшие потоки, в отличие от всех других, диспетчеризировались только один раз. Изначально я отказался от мысли, что это может быть проблема повреждения памяти, потому что в исходной переписке говорилось следующее:

содержимое буфера всегда равно нулю и не меняется. То есть, по крайней мере, никто не записывает ненулевые значения в буфер.

И вот что я получил за то, что не проверил, что структура не переписывается нулевыми байтами (всегда проверяйте свои допущения).

Я помнил, что в архитектуре x86 есть отладочные регистры , которые можно использовать для срабатывания контрольных точек записи данных. На самом деле, именно так я устранил баг в своём прошлом разработчика ПО. Разумеется, PowerPC тоже реализует подобную функциональность при помощи регистра DABR.

Я изучил, как можно использовать аппаратные контрольные точки в Linux, и в конечном итоге реализовал по этому прекрасному ответу на StackOverflow модуль ядра Linux. Это позволило мне установить аппаратную контрольную точку на поле __state, чтобы определить, что же выполняет в него запись.

Поиск бага

И именно так я нашёл проблему: мой модуль ядра показал трассировки стека из мест, где выполнялась запись в поле __state структуры task_struct. Я обратил внимание на странность, выявившую переполнение буфера в ptrace_put_fpr (используемого POKEUSER API). Это привело к переписываю важных полей из task_struct, например __state, в котором хранится состояние процесса и которое также используется ядром для отслеживания того, какие процессы остановлены отладчиком.

Что же было причиной этого переполнения? Получение индекса, который должен был использоваться с массивом 32-битных элементов и индексирование массива 64-битных элементов. Было 64 индексов, адресующих FPR, то есть общая адресуемая память составляла 64 * 8 = 512 байтов. Но в массиве fp_state.fpr было только 32 записи, то есть доступной памяти имелось всего 32 * 8 = 256 байтов. Это позволяло пользователю (то есть gdbserver), записывать до 256 байтов после конце массива.

Отправка патча апстрим

Я отправил патч команде безопасности ядра (security@kernel.org), потому что решил перестраховаться: проблема повреждения памяти, способная переписывать состояния процессов, может иметь последствия для безопасности. К сожалению, список рассылки приватный, так что я не могу дать ссылку на исходный патч, отправленный мной. Мне ответил мейнтейнер PowerPC Майкл Эллерман, сказав, что свяжется со мной лично, чтобы разобраться в проблеме. Я отправил ему два патча с устанением проблемы: исходный, который отправил в список рассылки безопасности, и ещё одну версию (достаточно сильно отличающуюся от первой), в которой реализовывались некоторые предложения, полученные в ответе на мой первый патч. Второй патч был основан на уже существующем коде ядра, эмулирующем операции PowerPC32 на PowerPC64 (да, там индексирование FPR реализовано верно). Ни один из них не был принят Майклом Эллерманом и вместо этого он реализовал собственную версию исправления. Я сказал ему, что был бы очень признателен, если он примет патч от меня, чтобы я получил свидетельство об устранении этой проблемы и стал контрибьютором ядра. Кроме того, я был открыт к работе с ним, отвечал на его отзывы и отправлял последующие версии патчей. Он сказал примерно следующее (передаю общий смысл):

Простите, но моя версия лучше. Если вы хотите быть контибьютором ядра Linux, то вот проблема, которую можно решить.

Мне показалось это очень непонятным и оскорбительным. Вместо того, чтобы признать мой вклад в решение проблемы, он лишь дал мне ещё больше работы. Моя компания и я должны были получить надлежащую репутацию за решение этой проблемы, особенно учитывая объём приложенных усилий.

Я считаю очень несправедливым, что нам выдали только тэг Reported-by. Вот в чём заключается предназначение этого тэга:

Тэг Reported-by позволяет отдать должное людям, которые находят баги и сообщают о них; его цель — вдохновлять таких людей помогать нам в будущем.

Ну, лично я точно не вдохновился на помощь сообществу разработки ядра. Напротив, я ощущаю себя униженным и разгневан, что моя работа не получила должного признания.

Заключение

Я потратил много времени и труда на анализ первопричины, устранение бага, тестирование и валидацию исправления, получение обратной связи от других инженеров моей компании, адаптацию исправления под последнюю версию ядра и отправку двух патчей мейнтейнеру PowerPC Майклу Эллерману. Вместо того, чтобы принять мой патч или подсказать направление к верному решению, он реализовал собственное исправление, поблагодарив только за отчёт о проблеме (о которой уже сообщали шестью годами ранее).

Мой первый вклад в ядро был очень разочаровывающим и досадным опытом, я столкнулся с людьми, которые не считают важным отдать тебе должное за твою работу.

Комментарии (95)

PuerteMuerte
27.09.2023 19:31
#26006322
+22
Ну, такое. Как по мне, патчи делают всё-таки для исправления ошибок, а не для звёзд на погонах.
1. shasoftX
  27.09.2023 19:31
  #26006362
  +83
  Однако если задачу выполнил ты, но очередное звание присвоили командиру, а тебя просто похлопали по плечу, то остаётся осадочек.
  1. develmax
    27.09.2023 19:31
    #26006482
    +8
    Это очень распространено в корпоративном сегменте - как правило руководитель получает признание за хорошую работу команды, а участники команды не получают ничего такого.
  1. PuerteMuerte
    27.09.2023 19:31
    #26006526
    +35
    Ну только он не выполнял задачу, а тот чувак не командир. Он энтузиаст, который никакую задачу не получал, по своей личной инициативе раскопал проблему, написал какой-то патч, разработчик ядра его в таком виде не принял, и написал свой, который, по его словам, более корректный. Ок, почему нет?
    
    А потом энтузиаст сказал "хочу стать разработчиком ядра", разработчик говорит: "Ок, без проблем, вот задача, которая нужна, сделаешь, будешь". А тот обиделся, дескать не-не-не, вы меня за патч в разработчики ядра должны были принять, а вы ещё какие-то таски на меня сбрасывать собрались. Ну, извини, братан, разработка ядра - это всё-таки таски как раз и делать.
    
    vanxant
    27.09.2023 19:31
    #26006620
    +36
    Ну хз. Как правило, баги типа переполнения массива лечатся относительно просто. А вот найти место, где именно это переполнение происходит, вот это уже задачка посерьёзнее. Может занять и 80%, и 99% времени.
    
    Как минимум, чувака можно было взять в соавторы за его исследовательскую часть.
    
    И да, кроме морального удовлетворения наличие своего емейла в списке разработчиков ядра линупса - и ссылка на этот факт в резюме - может принести весьма даже вкусные материальные прибавки.
    
    PuerteMuerte
    27.09.2023 19:31
    #26006658
    +2
    И да, кроме морального удовлетворения наличие своего емейла в списке разработчиков ядра линупса - и ссылка на этот факт в резюме - может принести весьма даже вкусные материальные прибавки.
    
    Я понимаю, и вот это, мне кажется, и было мотивацией человека. Он-то баг искал не просто так, в порядке чистого энтузиазма, а заранее договорившись с работодателем, в оплаченное им время. И сходу потребовал для себя максимальные плюшки. Как по мне, совсем ничего страшного, что его резюме будет не настолько идеальным, как он хотел бы :)
    
    Tiriet
    27.09.2023 19:31
    #26006800
    +8
    ну и что с того, что он действовал не из альтруистических соображений, а рассчитывая на материальную (или нематериальную) личную выгоду? с работодателем он договаривался или нет- если верить статье, то чувак из своих соображений проделал высококвалифицированную работу на благо сообщества. А чел, который должен был результаты этой работы включить в ядро- сделал косметические правки и включил ее как свою- то есть, украл и присвоил себе результаты. Совсем ничего страшного, если резюме не будет идеальным, но очень нехорошо то, что что такая ситуевина вообще случилась и что я (и Вы) о ней узнал(и)- это бьет по репутации всего механизма опенсурса.
    
    tenzink
    27.09.2023 19:31
    #26006922
    +13
    В тексте не сказано косметические ли правки сделал maintainer. Могло быть и так, что фикс автора никуда не годился
    
    Tiriet
    27.09.2023 19:31
    #26007058
    +2
    если верить статье (если ей верить! точку зрения маинтейнера мы не знаем)- то правки реально косметические- потому что проблема не в том, как именно реализовать защиту от выхода за границы буфера, а в том, где именно этот выход вообще происходит- работа по написанию сабжевого патча заключалась не в кодинге, а в определении причины бага. Но имеющаяся система такова, что на основании субъективной красоты кодинга патча все бонусы за патч уходят кодеру, реализовавшему самый красивый код, реализующий простую правку в простом месте, которое сложно найти, а самая сложная работа по поиску простого места остается безнаказанной.
    
    WQS100
    27.09.2023 19:31
    #26007082
    +2
    а самая сложная работа по поиску простого места остается безнаказанной
    
    Действительно, большое упущение
    
    arokettu
    27.09.2023 19:31
    #26007852
    +2
    ИМХО это и есть reported-by, где он и указан
    
    nidalee
    27.09.2023 19:31
    #26007874
    +4
    Reported-by - это жалоба на баг, а не "почти решение".
    
    arokettu
    27.09.2023 19:31
    #26008112
    это всё от "у меня не работает" до непринятого пулл реквеста
    
    b237
    27.09.2023 19:31
    #26008232
    +4
    Можно было бы и что-то более подходящее найти, как будто тегов мало https://news.ycombinator.com/item?id=37685190
    
    Reported-By для того, кто завёл баг 6 лет назад, Debugged-By для автора статьи, автором кода оставить мэйнтейнера
    
    robertd
    27.09.2023 19:31
    #26009118
    Там есть совсем подходящие, кажется (число — количесто упоминаний в ядре)
    
    - 147 co-authored-by: - 4779 co-developed-by: - 15568 suggested-by:
    
    tenzink
    27.09.2023 19:31
    #26007700
    +2
    Ну так себе косметика - в 6 строчках 3 дефекта/недочёта (комментарий ниже). Ментейнер исправил косяки автора, выкатил корректный фикс
    
    nidalee
    27.09.2023 19:31
    #26007356
    +33
    Вот предложенный код:
    #ifdef CONFIG_PPC32 *data = ((unsigned int *)child->thread.fp_state.fpr)[FPRINDEX(index)]; #else flush_fp_to_thread(child); if (fpidx < (PT_FPSCR - PT_FPR0)) memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long)); else *data = child->thread.fp_state.fpscr; #endif
    
    Вот финальный:
    flush_fp_to_thread(child); if (fpidx < (PT_FPSCR - PT_FPR0)) { if (IS_ENABLED(CONFIG_PPC32)) *data = ((u32 *)child->thread.fp_state.fpr)[fpidx]; else memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long)); } else *data = child->thread.fp_state.fpscr;
    
    Комментарий к коду с другого ресурса:
    The maintainer's patch is better.
    
    1. It doesn't lose the fpidx < (PT_FPSCR — PT_FPR0) test from the 32 bit case.
    
    2. In the put function, it doesn't lose the *data = child->thread.fp_state.fpscr; fallback assignment, which is taken when fpidx is out of bounds.
    
    3. It avoids the pointless FPRINDEX macro which reduces to identity if TS_FPRWIDTH is 1. The original patch's commit message says «On PPC32 it's ok to assume that TS_FPRWIDTH is 1», but then includes this anyway:
    
    #define FPRNUMBER(i) (((i) — PT_FPR0) >> 1)
    #define FPRHALF(i) (((i) — PT_FPR0) & 1)
    #define FPRINDEX(i) TS_FPRWIDTH * FPRNUMBER(i) * 2 + FPRHALF(i)
    
    vanxant
    27.09.2023 19:31
    #26007284
    +18
    это бьет по репутации всего механизма опенсурса.
    
    Эммм, ну хз. У вас, видимо, какие-то детские фантазии про мир благородных фей, которые дарят человечеству идеальный код. Нет, люди там ровно те же самые, что и везде. Работают они почти всегда за деньги, просто сначала нужно доказать, что ты типа энтузиаст и достоит. Интриг, склок и подковёрной грызни на "базаре" ОСС чуть ли не больше, чем в соборе корпораций.
    
    Ну а репутацию миру ОСС портит бредятина типа CoC и переименования master в main на фоне отказа принимать патчи по причине страны происхождения автора.
    
    shasoftX
    27.09.2023 19:31
    #26006742
    никакую задачу не получал, по своей личной инициативе раскопал проблему,
    
    Но ведь в той среде именно так и принято. Т.е. именно за такое и "награждают". Я так понял он когда искал, то держал в уме в том числе и награду. Он выполнил всю трудоемкую часть работы, а "командир" сверху полирнул его труды и награду забрал себе. А ему в качестве благодарности предложил другую работу "на энтузиазме". И, учитывая как поступил командир, может оказаться что и эту другую работу награда будет аналогичная.
  1. BerdBerd
    27.09.2023 19:31
    #26008616
    Однако если задачу выполнил ты, но очередное звание присвоили командиру, а тебя просто похлопали по плечу, то остаётся осадочек.
    
    Так что в итоге - залили их версию и их не указали - или же человек исправил проблему по-своему - ну и соответственно их тоже не указал как авторов (итоговый код же писали не они)?
1. akuli
  27.09.2023 19:31
  #26006456
  +2
  Одно прекрасно дополняет другое. Почему нет?

holomen
27.09.2023 19:31
#26006348
+33
хмм.. автор решал свою проблему. он нашел ее и решил. но в ядре решили ее несколько иначе. неприятно, да. но не ужас-ужас-кАшмар.
Какое то нытье десятилетней девочки когда родители не оценили как она САМА(!) наштукатурилась. она ведь так старалась стать красивой.
1. 19Zb84
  27.09.2023 19:31
  #26006462
  +12
  Ну в любом случае это не правильно. Переписать в сотни раз легче чем найти и сделать первый рабочий вариант.
  
  Наверное делать просто не надо было, то что не просили.
  1. develmax
    27.09.2023 19:31
    #26006490
    +10
    Имхо, здесь не хватает "based on" решение автора. Если бы можно было принять патч, но переписать уже по другому, но в истории и везде было бы написано, что оно базируется на конкретном решении или идеи другого автора, то всё было бы прозрачно, и это принесло бы пользу всем. Например, если в ядре решили переписать, но пришёл бы ещё один автор,посмотрел бы на всё это, то возможно он бы предложил бы ещё и третье решение, которое могло сочетать в себе лучшее из предыдущих двух решений.
    
    olartamonov
    27.09.2023 19:31
    #26006510
    +2
    Нет, так делать категорически нельзя. По юридическим причинам: по сути, вы предлагаете две работы, связанные только тем, что они решают одну и ту же проблему, объявить производными друг от друга. А «производная работа» — это уже юридически значимое понятие, определяющее, в частности, и распространение действия лицензии первой работы на вторую.
    
    Это будет иметь такие последствия, что небо с овчинку покажется.
    
    aamonster
    27.09.2023 19:31
    #26006562
    +6
    Погодите. Разве всё, что идёт в ядро, не должно быть под gpl? А она, вроде, решает эти проблемы довольно жёстко...
    
    olartamonov
    27.09.2023 19:31
    #26006570
    +2
    Здесь нет нарушения GPL, код двух патчей совершенно разный.
    
    aamonster
    27.09.2023 19:31
    #26007450
    +2
    Я немножко про другое: "распространение лицензии первой на вторую" не изменит примерно ничего, ибо обе должны быть под gpl. Да, если автора производного патча спросят – он должен дать исходник первоначального, но вроде на этом всё, нет?
    
    olartamonov
    27.09.2023 19:31
    #26007736
    В этом случае не изменит.
    
    Но вы создадите прецедент, в рамках которого завтра вам выкатит досудебную претензию юротдел какой-нибудь милой корпорации на том основании, что ваш код, хоть и не имеет с их кодом ни одной общей строчки, решает ту же самую проблему.
    
    qrKot
    27.09.2023 19:31
    #26007978
    +4
    Изменит, и очень сильно. Вы фактически предлагаете объявить код мейнтейнера производным от кода человека, чей патч не приняли.
    
    Свободность же кода держится на 2 столпах: "лицензионная чистота" + "необходимость явного согласия всех авторов на изменение правил лицензирования". Критерий авторства - наличие хотя бы одной строчки кода за авторством указанного человека.
    
    Если закрепить "производность", то а) мы получим еще одного "автора", не имеющего ни одной строчки кода в ядре (что поделит на ноль действующий механизм лишения права голоса в виде переписывания кода автора), б) получим прецедент, который позволит коммерческим компаниям ломануться в суд с претензиями а-ля "они вдохновлялись нашим АПИ" или "15 лет назад разработчик читал книжку, в которой в качестве примера был приведен наш код, поэтому ядро - наше".
    
    Ну вот не надо такого.
    
    Areso
    27.09.2023 19:31
    #26007638
    Не согласен с оценкой, что код патчей совершенно разный. Второй сниппет выглядит улучшением первого.
    
    olartamonov
    27.09.2023 19:31
    #26007738
    +3
    В них нет ни одной общей строчки, и даже подход к решению проблемы — разный.
    
    arokettu
    27.09.2023 19:31
    #26008046
    Подобный вклад не является copyrightable, поэтому это не вопрос лицензии, а вопрос морали и традиций опенсорса
    
    aamonster
    27.09.2023 19:31
    #26008098
    Ну это да.
    По опыту работы – у нас (не опенсорс) в commit messages в таких случаях принято указывать всех участников, но тут цель другая – чтобы знать, кому задавать вопросы, если что.
    А описание ситуации выглядит так, будто не хватает промежуточного статуса между "автор патча" и "репортер" (потому как чувак действительно сделал сильно больше, чем зарепортил баг – он нашёл причину, и что его патч не приняли – не столь важно). Наверное, до сих пор не настолько обидчивые люди контрибьютили, и им для счастья хватало уважения от конкретного майнтэйнера за почти готовое решение, которое только реализовать, как положено.
    
    andrey_gavrilov
    27.09.2023 19:31
    #26014082
    -1
    Это называется не "обидчивый" — такое определение кстати оскорбительно по отношению к этому человеку в данной ситуации. Акт микроагрессии.
  1. PrinceKorwin
    27.09.2023 19:31
    #26006768
    +10
    У меня был подобный опыт. Посылал патч в PostgreSQL. Его тоже переписали и меня в никакие списки не включили.
    
    Но это нормально. Решить локальную проблему может быть не просто, да. Но сделать патч так чтобы (ниже) тоже может быть не просто:
    
    Он удовлетворял всем принятым в проекте формальным требованиям (комменты, отступы, нейминги и т.д.)
    
    Чтобы этот код попутно ничего не сломал
    
    Покрыть новый код тестами
    
    Согласовать это изменение с остальными членами команды
    
    ritorichesky_echpochmak
    27.09.2023 19:31
    #26006920
    +7
    Это да. У нас есть фичи которые по 2-3 месяца делают и их не берут в прод, а где-то год спустя когда про них снова вспоминают оказывается что проще написать заново, чем смержиться с мастером. И есть фиксики которые вроде все потестили, прокликали на стейдже, но т.к. не очевидно что ничего не сломано - тоже не уходят в прод пока дым не пойдёт. А это всего-лишь свой закрытый продукт на десяток разработчиков.
    
    Патчи нужно уметь писать. Если у автора после двух итераций не получилось написать хороший патч, то я не вижу проблем что патч сделал кто-то другой при условии что он сделал это лучше.
    
    776166
    27.09.2023 19:31
    #26008514
    Есть проблема. После такого желания коммитить пропадает. При том, что оставить в коде ссылку на того, кто начал закрывать проблему, описанную в баге, которому много лет, по-сути, бесплатно. Для этого патронов упоминают любыми способами, потому что обычно человеку ничего не нужно, кроме понимания того, что он полезен, если ему нечего дать в обмен на его помощь. А здесь даже на это пожидились. Я бы после такого отношения забил бы даже думать про какие-то коммиты, если сообщество таким образом не поощряет своих членов.
  1. wolfer
    27.09.2023 19:31
    #26008120
    +1
    зависит от того насколько корректен этот первый рабочий вариант. Он может не учитывать кучу ситуаций. Тут бы нужен комментарий-сравнение, что именно переписано. Вряд ли там простой рефакторинг. Вероятно "первый рабочий" рабочий только на узком диапазоне ситуаций. И за такое половинчатое решение выдавать достаточно высокий статус в разработческой среде? Это его обесценивание. Я честно говоря не знал, что за один патч можно получить статус контрибьютора, для мне это всегда были люди, которые активно работают, а не просто когда то что то сделали 1 раз
    
    kipar
    27.09.2023 19:31
    #26008436
    https://habr.com/ru/articles/763960/comments/#comment_26007356

izard
27.09.2023 19:31
#26006410
+27
Если новый патч действительно лучше, то в чем проблема? Так обычно и бывает. Я как-то лет 15 назад решил проблему клиента, пропатчив ему ядро, и отправил патч мейнтейнеру подсистемы. Мейнтейнер переписал патч с нуля, гораздо лучше и правильней, чем я; потом продал его моему клиенту и конкурентам клиента, а потом засабмитил свой патч в ядро. И все молодцы.
1. saga111a
  27.09.2023 19:31
  #26006580
  +3
  Проблема в том, что основная ценность это не итоговый код, а понимание точного места где этот код надо написать. Какой точно код написан имеет куда меньшее значение. Действие мейнтейнера имели одну простую цель - унизить автора, для него патчи писать является рутинной задачей, для человека который смог найти проблему - это большая заслуга, мог бы и указать на реальное авторство. Или как писали на hacker news - сразу же покрыть сверху своим патчем улучшающим начальный (https://news.ycombinator.com/item?id=37671991)
  1. izard
    27.09.2023 19:31
    #26006586
    +19
    Цель мейнтейнера - иметь качественный код в своей подсистеме, а не унижать авторов. Унижение авторов опционально, Линус это хорошо умел, кстати. Я тогда тоже обиделся, и рассказал на линуксконе через пару лет мейнтейнеру системы на уровень выше, и он мне сказал, что это все так и должно работать.
    
    saga111a
    27.09.2023 19:31
    #26006596
    +6
    А вы сами уверены, что это так должно работать? Лично я уверен, что нет. Использование своего положения не является ни причиной ни основанием унижать кого-то.
    
    У меня был большой период общения с людьми которые вели себя схожим образом и всегда говорилось что самое главное это общее дело, а то что общение с говницом постоянн это опционально. Когда я закончил с ними взаимодействие, постепенно до меня дошло что на самом деле самое важное в работе этих людей это была возможность общаться с говницом, унижать других. Еще через 10 лет, после большого взаимодействия с людьми которые добились практически максимального в своей области, до меня дошло что вот те, исходные, ребята с говницом на самом деле просто самоутверждаются на других за счет очень узкой области потому как больше совершенно ничего не могут в других областях жизни, и таким образом выражают обиды на общество.
    
    izard
    27.09.2023 19:31
    #26006602
    +16
    Мейнтейнер написал автору - "> Thanks for your patch, but I wanted to fix it differently. Can you try > the patch below and make sure it fixes the bug for you?"
    
    Не вижу особенного хамства.
    
    bak
    27.09.2023 19:31
    #26006652
    -4
    Нормальное поведение - это описать что не так и попросить доработать. И уж если автор исходного патча откажется - написать самому. А сходу брать и писать по своему - это не уважение. К сожаление в open source синдром вахтера очень распространен.
    
    saga111a
    27.09.2023 19:31
    #26006714
    "Мне мой код нравится больше. Давай попробуй на вот этих задачах и может быть мы примем патч" -- это звучит примерно так. Так себе подход с учетом труда вложенного автором. Варианты сделать быстро и в "сотрудничестве" были, но майнтейнер выбрал довольно странный путь
    
    PuerteMuerte
    27.09.2023 19:31
    #26006738
    +21
    Варианты сделать быстро и в "сотрудничестве" были, но майнтейнер выбрал довольно странный путь
    
    Не странный, а самый очевидный - поблагодарить за найденную ошибку, но исправить баг тем способом, который ему лично кажется более корректным. Потому что мейнтейнер - он, ему и решать. Опция "подобрать наиболее психологически удобный для репортера способ сотрудничества" в функционале мейнтейнера отсутствует напрочь.
    
    Вы же поймите, задачи мейнтейнера - поддерживать своё приложение/часть приложения, только и всего. Репортеров, их легион, и у каждого второго свои требования/хотелки/психологические особенности. Мейнтейнер не может, и не должен тратить своё время на то, чтобы подобрать к каждому оптимальный и справедливый ключик. Всё, что он должен - посмотреть патч, принять, отклонить или переделать. Репортим мы и посылаем патчи для того, чтобы фиксились баги в нужном нам софте, а не чтобы свои карьерные вопросы решать.
    
    PrinceKorwin
    27.09.2023 19:31
    #26006770
    +5
    Нормальный подход. Я даже у себя в команде на код-ревью бывает так делаю.
    
    Есть моменты когда проще/быстрее/качественнее сделать свой патч на базе чужого чем пытаться человеку за кучу итераций объяснить что требуется.
    
    Причиной этому может быть, например, его слабое владение какой-то специфичной частью (рефлексия, многопоточность и т.д.).
    
    AllexIn
    27.09.2023 19:31
    #26006848
    +3
    И ставите таску как выполненную вами?
    
    PrinceKorwin
    27.09.2023 19:31
    #26006894
    +1
    Нет. Тем не менее некоторые все равно обижаются.
    
    saga111a
    27.09.2023 19:31
    #26006980
    +3
    Так тут история именно о том что поставили как выполнено другим, а предложение указать иное послали делать другие баги. Это и вызывает вопросы.
    
    PrinceKorwin
    27.09.2023 19:31
    #26007500
    +4
    Если посмотреть на патч от автора и меинтейнера, то объективно второй патч лучше.
    
    В этой ситуации я на стороне меинтейнера.
    
    Знаю. Обидно. Да. Сам бывал ни раз в подобных ситуациях (мои патчи в KDE, PostgreSQL были "приняты" точно также). И это нормально.
    
    Другие мои патчи были приняты без сильных переделок и с сохранением авторства.
    
    Вы поймите и меинтейнера тоже. Его задача не сделать счастливым автора патча. Его задача улучшить кодовую базу проекта.
    
    antiquar
    27.09.2023 19:31
    #26007578
    +2
    Так то оно так. Но вряд ли данная история будет мотивировать, случись такое, кого-то в следующий раз оказывать помощь. "Знаю, как исправить косяк, а тебе не скажу".
    
    Оно, конечно, может, у мейнтейнера и без того все хорошо, никакая помощь не нужна.
    
    saga111a
    27.09.2023 19:31
    #26007624
    +4
    Патч мейнтейнера логично, что лучше, знать кодовую базу стороннему человеку объективно сложно. Про это пишут абсолютно все и везде(на реддите или hacker news).
    Мои рассуждения строятся только на том что человек сделал хорошее и он реально нашел неочевидное - место где именно надо исправить, такие действия должны быть оценены нормально, несколько больше чем "указал на багу". В противном случае это уменьшает желания других помогать проекту. Про подобный подход писали другие кто посылал патчи, много упоминаний -- либо оставили без ответа, а потом от своего имени исправили либо еще и обругали, а потом исправили. Такая политика не будет способствовать улучшению кода.
    
    PrinceKorwin
    27.09.2023 19:31
    #26007768
    +3
    Ну смотрите. Давайте посмотрим чуть глубже. Вот я имею проблему с определенным кодом. Вижу там авторство. И иду к этому человеку чтобы получить помощь.
    
    Возвращаемся к текущей ситуации. Если там (как сейчас) указан меинтейнер, то я на верном пути и он мне может дать ответ т.к. этот патч реально его и он понимает что он делает.
    
    Если же там оставить оригинального автора, то при обращении к нему я получу 146% ответ - иди к меинтейнеру, это его патч. Спрашивается - какого фига тогда?
    
    Да. Для кого-то это убийство мотивации. Например мой патч в iBatis (сейчас это MyBatis) был принят полностью без изменений, но забрано авторство. Это отбило желание иметь дело с этой либой и перевод проектов на другой ORM.
    
    В других случаях мой патч действительно был улучшен меинтейнеров и включен в релиз без указания моего авторства. И это нормально. Это не отбивает мотивацию.
    
    saga111a
    27.09.2023 19:31
    #26007812
    Мое предположение:
    У меня есть серьезное подозрение что стриггернуло именно предложение по закрывать другие таски. Так что возможно если бы он просто описал как вы, даже имея этот текст заготовкой ситуация пошла бы иным путем.
    Предложение от кого-то на hacker news:
    принять патч и сверху сразу наложить свой.
    
    Чем хотя бы последний вариант плох?
    
    PrinceKorwin
    27.09.2023 19:31
    #26007862
    +5
    Чем хотя бы последний вариант плох?
    
    Всем :) честно
    
    Знаете. Иногда происходит откат патчей. И здесь появляется не нулевой шанс откатить не оба, а только последний. Что уже может аукнуть.
    
    В своей практике я такое пресекаю сразу.
    
    saga111a
    27.09.2023 19:31
    #26007902
    Всем :) честно
    
    Спасибо, с описанной вами плоскости не думал. Теперь понял почему иной чем сделал мейнтейнер это плохо.
    
    Остаётся надеяться что текущая ситуация поможет установить нормально правила. Чтобы и в код не тянуть и чтобы патчеписатели получали то, что ожидают.
    
    ritorichesky_echpochmak
    27.09.2023 19:31
    #26006966
    +3
    Берём какую-нибудь, возможно кривовато настроенную жиру, заводим там багу, бага падает на первую линию саппорта. Будут ли ребята из саппорта или тот кто зарепортил багу указаны в коммите? Багу пофиксили каким-то коммитом ребята посидевшие вместе (парное программирование, ага) - кто автор, а на ком сейчас висит задача? Далее багу переводят на тестировщика (потому что у жиры только один assignee) - как дописать тестировщика в авторство уже сделанного коммита?
    Я пока хамства от ментейнера не увидел, а желание обидеться на всех кругом и результат этого желания - вижу регулярно
    
    AllexIn
    27.09.2023 19:31
    #26007048
    +3
    Значок "контрибьютор" - это ценность. Когда выпадает возможность эту ценность получить - это мотиватор. Когда эту ценность не получаешь, ожидаемо будешь недоволен.
    Ваше желание показать что мой пример не всеобемлющий - это замечательно. Но я и не ставил своей целью написать всеобъемлющий пример. Моя цель была показать, что поведение@PrinceKorwinотличается от того, что описано в статье.
    
    tenzink
    27.09.2023 19:31
    #26007760
    +6
    Автор ещё не дорос до значка "контрибьютор". Не нужно девальвировать это, давая кому попало. Да и ведёт он себя предельно неуважительно и токсично (в отличии от ментейнера). Название поста "как у меня украли авторство кода" намекает
    
    arokettu
    27.09.2023 19:31
    #26008036
    Название поста либо автор изменил, либо переводчик перевел желтушно, в оригинале - "How I got robbed of my first kernel contribution", т.е. это не про авторство, а про саму возможность стать контрибьютором
    
    tenzink
    27.09.2023 19:31
    #26008078
    Да, перевод желтушный. В оригинале не так жёстко, хотя robbed всё равно цепляет
    
    SergeyMax
    27.09.2023 19:31
    #26008516
    +2
    Оригинал звучит ещë более желтушно, как по мне.
    
    arokettu
    27.09.2023 19:31
    #26008008
    +2
    Как мейнтейнер я прежде всего думаю что человек, который пришел ко мне с проблемой, хочет чтобы мой код для него работал, а если он пришел с патчами то прежде всего хочет сэкономить мне время. Ну потому что когда я куда-то контрибьючу, это чтобы люди побыстрее выпустили апдейт и я наконец убрал костыль из своего кода. Если патч можно доработать, я попрошу доработать, если сам подход плох, проще переписать самому, я сам так как минимум 2 раза заворачивал патчи и мои патчи заворчивали бесчисленное число раз и это нормально. Если же человек хочет именно стать контрибутором, то он должен об этом сказать
  1. dartraiden
    27.09.2023 19:31
    #26006610
    +8
    Какой точно код написан имеет куда меньшее значение.
    
    Тем не менее, абы какой попало код в ядро не принимают.
    
    Например, когда компания Paragon прислала патч, добавляющий в ядро поддержку NTFS, ей пришлось 26 раз его переделывать, устраняя замечания. Несмотря на то, что добавление такой возможности имеет большое значение.
    
    saga111a
    27.09.2023 19:31
    #26006712
    -1
    Безусловно абы что не надо, но действия с данным типом и другими в итоге приводят к тому что код в ядре будет хуже чем могло бы быть. Люди просто посылать туда не будут.
    
    fshp
    27.09.2023 19:31
    #26010118
    Мой патч приняли, но я не уверен, что его кто-то тестировал кроме меня.
  1. ritorichesky_echpochmak
    27.09.2023 19:31
    #26006926
    +7
    Офигеть. А где вы вычитали, что ментейнер хотел прям унизить автора? Пруфы, пожалуйста. Объективные. Я пока вижу, что автор пишет что фундамент у него был, но принимать соавторство вот тех ребят, фундамент которых он поюзал, он не горит. И тащить 100500 человек "фундамента" в "авторы линукса" при условии что в ядро от них не попало ни одной строки по итогу - ну такое себе. Так можно и Таненбаума автором Линукса приплести, ведь какие-то идеи пересекаются.
    
    ritorichesky_echpochmak
    27.09.2023 19:31
    #26007070
    Пруфов не привели, но карму слили. Хотели обидеться - обиделись. Продолжайте, это очень "конструктивно") Я бы после общения в настолько "конструктивном" ключе не то что коммит не принял, но и в ч.с. добавил. Потому что лэйбочки лэйбочками, а вот хамство и перевирание действительно демотивирует делать что-то для людей. И демотивируют что-то делать не вот того кто первый раз (и скорее всего единственный) что-то попытался, а тех, кто что-то делает на регулярной основе. А потом мы читаем статьи о выгоревших ментейнерах
1. iskateli
  27.09.2023 19:31
  #26007810
  +4
  в чём проблема признать вклад человека выполнившего более 90% работы?
  1. SergeyMax
    27.09.2023 19:31
    #26008318
    +2
    Вроде его вклад никто и не отрицает. Но и тащить в ядро любую хрень под этим предлогом тоже не стоит.

isden
27.09.2023 19:31
#26006512
+3
Ну у меня тоже была немного похожая ситуация. У меня, конечно, не ядро, и вообще либа на десяток строк, но PR пришлось отклонить (не очень оптимально было сделано на мой взгляд) и переписать все чуть иначе. Заодно отрефакторил :) Спасибку автору в мессаге честно указал. И все вроде довольны.
1. Wolf4D
  27.09.2023 19:31
  #26008758
  Когда впервые завёл опенсорс-проект и получил свой первый PR, начал интересоваться - а как ПРАВИЛЬНО принять реквест, если чтобы и авторство сохранить, и содержимое коммита поправить?
  
  Удивительно было, что общепринятого решения нет, хоть очевидно, что не я один сталкивался с такой проблемой. В итоге остановился на промежуточном варианте - утянул коммит, переписал его содержимое через amend, и уже тогда принял PR.
  1. isden
    27.09.2023 19:31
    #26008810
    А с какого момента, в таком случае, авторство сохранять? 50% PR переписано? А если один символ, но исправляет критическую ошибку в PR?
    Кмк, тут правильно (в зависимости от масштабов) — либо написать автору что PR не годный и указать на проблемы и ждать исправления, либо переписать самому и указать оригинального автора.
    
    Wolf4D
    27.09.2023 19:31
    #26009452
    У меня был вопрос, что PR в целом годный (добавлен новый функционал), но не вписывается по оформлению и отчасти архитектуре в основной код. Тут быстрее и проще было поправить ручками, чем участвовать в долгой переписке с доброхотом.

olartamonov
27.09.2023 19:31
#26006522
+18
Юридически и технически мейнтейнер поступил корректно.

Возможно, он одновременно с этим поступил по-хамски, но это без оригинала переписки установить невозможно. Бывают откровенно хамящие мейнтейнеры с синдромом вахтёра, бывают сверхназойливые контрибьюторы с говнокодом.

Vanovsky714
27.09.2023 19:31
#26007408
Интересно, что мнения разделились. Не буду оценивать уровень хамства/наивности сторон, но... Если так сильно вас это задело, то почему вы не напишете в сообщество? Наверняка там есть какие-то публичные формы обратной связи.

glazko
27.09.2023 19:31
#26007802
Я потратил много времени и труда на анализ первопричины, устранение бага, тестирование и валидацию исправления, получение обратной связи от других инженеров моей компании, адаптацию исправления

Явление с которым вы столкнулись я называю «Глубокое синее море», статья , где я постарался изложить его суть. Возможно вам будет интересно

vassabi
27.09.2023 19:31
#26007804
+3
до середины читается нормально, а потом в финале автор оригинала не смог удержаться - оказалось, что звёздочка :) ему была таки важнее чем качество исправления ...

marperia
27.09.2023 19:31
#26007818
+3
Столкнулся ровно с такой же ситуацией, но смешнее. В Firefox containers extension был баг, который не фиксился ничем. Скачал, форкнул репо и оказалось, что там нужно просто обновить библиотеки. Всё работало, код при этом никакой не ломало, а даже наоборот.

PR ментейнер отклонил, я так понял из-за каких-то внутренних проволочек, а зачем сделал ровно такой же (с другими версиями пакетов) через пару месяцев.

Как итог, никакой строчки в профиле, что я являюсь контрибутуром в мозилу, ни чувства сопричастности, ни благодарности, ни желания впредь влезать в опен сурц, ни даже каких-либо сообщений на свой коммит я так и не получил. Зато два месяца сидел с локальной версией плагина потому, что разработчик отклонил патч с обновлением (!) сторонних (!) библиотек с критическими уязвимостями (!), а некоторые даже и без поддержки.

Явно что-то не то в отрасли...
1. PrinceKorwin
  27.09.2023 19:31
  #26007892
  +10
  Вы исходите из того, что в опенсорсе работают только золотые, идеальные люди. Но это не так. Человеческий фактор в опенсорсе стоит очень остро.
  
  Относитесь к этому проще и не распространяйте единичный опыт на всю отрасль.
  
  Ведь в вашем случае самое главное - что вы два месяца сидели на рабочей исправленной версии софта. С закрытым софтом у вас такой возможности не было бы.
  1. eton65
    27.09.2023 19:31
    #26009768
    Вы исходите из того, что в опенсорсе работают только золотые, идеальные люди. Но это не так. Человеческий фактор в опенсорсе стоит очень остро
    
    А разве опенсорс движется не за счет взаимоуважения и благодарности? А значит это крайне важные понятия, пренебрежение которыми опасно.
    
    PuerteMuerte
    27.09.2023 19:31
    #26010284
    +3
    А разве опенсорс движется не за счет взаимоуважения и благодарности?
    
    Нет, вообще нет. У опенсорса широчайший спектр мотиваций, от "пишу, потому что мне это нравится" через "я так удовлетворяю свои амбиции" и "да просто у нас так принято" до "а мне за это хорошо платят". Конечно, где-то там, в этой толпе разработчиков есть и какое-то заметное количество альтруистов, но это далеко не главная движущая сила опенсурсного движения.
    
    eton65
    27.09.2023 19:31
    #26012790
    У опенсорса широчайший спектр мотиваций
    
    Ок, согласен.
    
    от "пишу, потому что мне это нравится"
    "я так удовлетворяю свои амбиции"
    
    Не получится ни у кого писать для удовольствия, если этим будут пренебрегать. Готов поспорить (на интерес)!
    
    "да просто у нас так принято" до "а мне за это хорошо платят"
    
    Вас будет вдохновлять, если пренебрежение вашим трудом будут оправдывать этими словами? Думаю нет. Даже большие деньги недолго удерживают людей в таких условиях.
    
    PuerteMuerte
    27.09.2023 19:31
    #26013140
    Не получится ни у кого писать для удовольствия, если этим будут пренебрегать. Готов поспорить (на интерес)!
    
    Контраргумент - на гитхабе легион живых публичных репозиториев с пет-проектами, которые пишутся просто для себя, без оглядки на внимание других людей, и лишь потом, некоторые из них, может быть, находят других пользователей.
    
    Вас будет вдохновлять, если пренебрежение вашим трудом будут оправдывать этими словами?
    
    А почему пренебрежение? За деньги писать - вообще нормально и обычно. Если мне заплатили за какой-то код, дальше заказчик волен делать с ним всё, что хочет - публиковать, хоть под моим, хоть под своим именем, выпускать с ним брелки или проводить ритуалы чернокнижия.
    
    А под "у нас так принято" я имел в виду ту огромнейшую часть опенсурса, которая является опенсурсной просто потому, что команды изначально начинали открытые проекты, и присоединившиеся к ним новые участники всего лишь продолжают уже существующий проект, без особого идеологического бэкграунда. "Тут так писали, и я так пишу", или потому что заюзали что-то из липучих лицензий а-ля GPL, и опенсурс получился вынужденным.
    
    eton65
    27.09.2023 19:31
    #26014020
    Контраргумент — на гитхабе легион живых публичных репозиториев с пет-проектами, которые пишутся просто для себя, без оглядки на внимание других людей, и лишь потом, некоторые из них, может быть, находят других пользователей
    
    Так в том то и дело, что на Гитхабе не принято критиковать посторонние репозитории.
    
    Если мне заплатили за какой-то код, дальше заказчик волен делать с ним всё, что хочет
    
    Он то конечно волен, но вам все равно будет обидно, если он скажет "код г***но, выкидываем!". А если еще и не заплатил, или вы по доброте душевной это сделали...

vadimr
27.09.2023 19:31
#26008778
+2
Автору надо быть проще. Для оценки его репутации всем абсолютно по барабану, есть он в каком-то списке в недрах сырцов линуха или нет. Шарит в вопросе – молодец.

comandante_teresa
27.09.2023 19:31
#26008968
+1
Ситуация напоминает известную басню про ремонт ударом молотка, где исполнитель выставляет счёт в стиле "удар молотка - 1 коп., определить куда именно ударить - 999,99 руб. Такое ощущение, что проблема в том, что в цепочке "багрепортер-патчер" отсутствует сущность "диагност", а плоды его трудов традиционно уходят "патчеру".

IDDQDesnik
27.09.2023 19:31
#26009070
+1
ИМХО, Мейнтейнер повел себя не правильно, сделав как ему было проще. Есть же тэги Co-developed-by: и Suggested-by:, которые и надо было использовать в данном случае.

WASD1
27.09.2023 19:31
#26009164
+1
Ну по-существу автор считает, что он наработал на ачивку "контрибьютор", а мэйнтейнер считает, что не наработал. По причине, что 0 строк автора осталось в исходном коде.

Если смотреть формально - то не наработал.
Если смотреть фактически - то большинсво работы выполнил автор.
Вывод - наверное надо иметь какую-то промежуточную ачивку между reported - contributor.

event1
27.09.2023 19:31
#26009520
+6
Во-первых, люди что-то стали забывать, что в интернете могут и на хрен послать. Во-вторых, мейнтейнеры исходят из того, что патчи присылают для улучшения ядра, а не для получения очередного звания. Если упоминать всех, кто подвигнул к правильному решению, то в коммитах будут одни трейлеры.

rcl
27.09.2023 19:31
#26010874
+1
Он сказал, - не надо орден, я согласен на медаль.

onegreyonewhite
27.09.2023 19:31
#26012420
Хабр всё же жалобная книга, что бы там в правилах не было написано.