Ma première contribution au noyau m’a été volée

• Ariel Miculas a enquêté sur un problème qui cassait le débogage multithread avec gdbserver sur PowerPC32 et a découvert un bug de corruption mémoire dans le noyau Linux, mais le correctif final a été intégré sous forme d’un patch séparé du mainteneur, ne lui laissant qu’un crédit Reported-by
• Les indices venaient d’un ancien fil de discussion par e-mail portant sur les mêmes symptômes et d’un commit qui déplaçait l’emplacement de thread_struct thread dans task_struct ; certains threads restaient dans un état de processus incorrect, ce qui faisait perdre le contrôle à gdbserver
• L’analyse de la cause a utilisé pahole, ftrace et des points d’arrêt matériels basés sur le DABR de PowerPC ; elle a confirmé qu’un débordement de tampon dans ptrace_put_fpr écrasait des champs comme task_struct.__state
• Le débordement se produisait parce qu’un index prévu pour des éléments 32 bits était appliqué à un tableau 64 bits ; après le tableau fp_state.fpr, qui ne contient que 32 entrées, le gdbserver côté utilisateur pouvait écrire jusqu’à 256 octets au-delà de la limite
• Le mainteneur PowerPC Michael Ellerman a appliqué son propre correctif ; dans une réponse ultérieure, il a présenté des excuses, reconnaissant que le seul crédit Reported-by ne reflétait pas correctement la contribution et qu’il aurait fallu faire évoluer le patch avec lui

---

Débogage gdbserver cassé sur PowerPC32

• Il y a environ un an et demi, Ariel Miculas a demandé du temps dans son ancienne entreprise pour enquêter sur un problème qui bloquait les fonctionnalités de débogage d’un projet
• Le problème était que, sur l’architecture PowerPC32, gdbserver ne parvenait pas à déboguer correctement les applications multithread
  • La connexion à gdbserver se coupait et il n’était plus possible de contrôler la session de débogage
  • Il n’était pas certain que la cause se trouve dans la toolchain, gdbserver, le noyau Linux ou des patches noyau personnalisés
• Après des recherches et une enquête sur l’existant, il a trouvé un fil de discussion par e-mail décrivant les mêmes symptômes
  • Ce fil pointait aussi vers le commit ayant provoqué le problème
  • Le patch en question déplaçait la définition de thread_struct thread, auparavant au milieu de task_struct, vers la fin

Des indices menant de l’état des threads à une corruption mémoire

• Selon l’analyse existante de Holger Brunck, gdbserver envoyait un SIGSTOP au noyau pour chaque thread et attendait une réponse, mais en situation d’erreur le noyau ne répondait qu’à certains signaux
• À bas niveau, après interaction avec gdbserver, certains threads se retrouvaient dans un état de processus incorrect, puis gdbserver perdait le contrôle
• Ariel a passé trois à quatre jours à lire les descriptions de commits liés à PowerPC et les changements autour de task_struct, afin de vérifier si le problème avait été résolu dans des versions ultérieures du noyau, mais n’a pas trouvé de solution
• Il a modifié l’emplacement de thread_struct thread pour confirmer les conditions de reproduction, et a examiné la disposition de task_struct avec pahole
• Il a utilisé ftrace pour suivre quand les threads du processus débogué étaient ordonnancés
  • Les threads bloqués n’étaient ordonnancés qu’une seule fois, contrairement aux autres threads
  • Cette observation l’a amené à reconsidérer la possibilité d’une corruption mémoire

La cause réelle trouvée grâce aux points d’arrêt matériels

• Il a confirmé que, comme les debug registers sur x86, PowerPC disposait d’une fonctionnalité similaire via le registre DABR
• Après avoir étudié comment utiliser les points d’arrêt matériels sous Linux, il a implémenté un module noyau en s’appuyant sur une réponse Stack Overflow
• Avec ce module, il a posé un point d’arrêt matériel sur le champ task_struct.__state afin de tracer qui écrivait cette valeur
• Le module noyau personnalisé affichait la stack trace de l’emplacement qui écrivait dans task_struct.__state, et l’un des chemins suspects menait à un débordement de tampon dans ptrace_put_fpr
• ptrace_put_fpr est utilisé par l’API POKEUSER, et le débordement écrasait des champs importants de task_struct, comme __state
  • __state stocke l’état du processus
  • Le noyau utilise aussi ce champ pour suivre les processus arrêtés par un débogueur

Une écriture de tableau pouvant dépasser jusqu’à 256 octets

• La cause du débordement était l’application à un tableau d’éléments 64 bits d’un index conçu pour écrire dans un tableau d’éléments 32 bits
• L’index pointant vers les FPR comportait 64 valeurs, et la plage d’accès avec des éléments de 64 bits était de 64 * 8 = 512 octets
• En réalité, le tableau fp_state.fpr ne contenait que 32 entrées, avec seulement 32 * 8 = 256 octets de mémoire utilisable
• Résultat : l’utilisateur, c’est-à-dire gdbserver, pouvait écrire jusqu’à 256 octets au-delà de la fin du tableau

Soumission upstream et conflit autour du crédit

• Ariel a estimé que ce problème de corruption mémoire, capable d’écraser la mémoire d’état des processus, pouvait avoir un impact de sécurité et a envoyé le patch à [e-mail]
  • Cette liste de diffusion étant privée, il n’était pas possible de lier le patch soumis à l’origine
• Le mainteneur PowerPC Michael Ellerman l’a contacté en privé pour répondre qu’il allait traiter le problème
• Ariel a envoyé deux patches de correction
  • Le patch original envoyé à la liste de diffusion sécurité
  • Une autre version du patch intégrant des suggestions faites sur la soumission originale
• Le second patch s’appuyait sur du code noyau existant qui émulait le comportement PowerPC32 sur PowerPC64, et ce code gérait correctement l’indexation des FPR
• Michael Ellerman n’a accepté aucun des deux patches et a implémenté son propre correctif
• Ariel voulait que son patch soit accepté, à la fois pour le mérite d’avoir corrigé ce problème et pour être crédité comme contributeur au noyau pour la première fois ; il était aussi prêt à envoyer une version de suivi intégrant les retours
• En pratique, le seul crédit reçu a été le tag Reported-by, ce qu’Ariel a jugé injuste au regard des efforts consacrés à l’analyse de la cause, au correctif, aux tests, à la validation, à la prise en compte des retours internes et à l’adaptation au noyau le plus récent
  • D’après la documentation Linux, Reported-by est un tag qui crédite la personne ayant trouvé et signalé un bug
  • Ce problème avait déjà été signalé six ans auparavant

Mise à jour et réponse d’excuses

• Un utilisateur de Hacker News a signalé un e-mail dans lequel Michael Ellerman écrivait : « merci pour le patch, mais je préférerais corriger autrement. Peux-tu tester le patch ci-dessous pour vérifier qu’il corrige le bug ? »
• Cet e-mail montre que Michael Ellerman a choisi sa propre implémentation plutôt que de relire le patch d’Ariel, et a demandé à Ariel de tester ce patch
• Ariel a retrouvé le patch original envoyé à la liste de diffusion sécurité et a transféré le fil de discussion vers linuxppc-dev
• La mise à jour ultérieure inclut une réponse d’excuses concernant la manière dont le patch a été traité
  • Elle contient notamment : « j’aurais dû consacrer plus de temps à faire évoluer ton patch avec toi »
  • Elle indique également que le tag Reported-by ne reflétait pas correctement la contribution d’Ariel

[GN⁺]

Avis sur Hacker News

• Même s’il ne fallait pas accepter l’intégralité du patch, il aurait été plus approprié de créditer Ariel Miculas.
  Il y a de fortes chances que ce problème de sécurité n’aurait été corrigé par personne s’il ne l’avait pas trouvé et envoyé une proposition de correction.
  Si Michael a lu son patch, n’a modifié que quelques éléments de style puis l’a soumis sous son propre nom, ne pas lui attribuer de crédit est contraire à l’éthique ; même si le code était mauvais, cela restait une collaboration, et après une telle expérience on n’aurait pas envie de retravailler ensemble.

  • Avant, je pensais qu’il était acceptable de reprendre la contribution de quelqu’un, de la réécrire entièrement dans ma branche puis de la fusionner, ou de la réécrire sous forme de nouveau commit dans une PR GitHub.
    Dans ce dernier cas, il reste au moins un Co-Authored-By, mais je n’ai compris à quel point l’expérience pouvait être amère pour le contributeur que lorsqu’une personne a déversé le code de mon projet dans son propre dépôt, sans historique de commits, en appelant ça un « fork ».
    Même si le code était mauvais, la correction refactorée qui a été fusionnée n’aurait pas existé sans lui, et dire « ma version est meilleure » peut être perçu comme assez méprisant.
    Cela dit, après avoir vu une réponse qui renvoie à la vraie réponse du mainteneur, la dernière phrase ne tient pas.
  • D’accord. Le mainteneur peut décider, pour des raisons de qualité, d’écrire lui-même le code qui ira dans le noyau, et c’est compréhensible en soi.
    Mais dans ce cas, l’auteur du billet est plutôt un coauteur de ce patch.
    Dans l’édition académique, ce serait un peu comme envoyer une prépublication à quelqu’un, puis voir cette personne publier un article sur le même sujet en vous mentionnant seulement dans les remerciements, sans vous inclure comme coauteur.
    Ici, il ne s’agit pas de publication académique mais de développement du noyau, mais la mention d’auteur reste importante dans un CV professionnel.
  • Je vois le rôle d’un mainteneur expérimenté comme celui d’un facilitateur, quelqu’un qui aide autant que possible à intégrer proprement les contributions des autres.
    Les retours sur le style et la cohérence de structure sont nécessaires, mais un bon mainteneur est quelqu’un qui reconnaît le mérite et fait en sorte que les choses avancent sans heurts en coulisses.
  • Les premières fois où j’ai envoyé des patchs au noyau Linux, il n’était même pas encore possible de faire des soumissions semi-automatiques via LKML, donc j’ai avancé en discutant avec le mainteneur.
    À l’époque, quand Ted Tso a été satisfait de l’exactitude de mon travail, il a intégré le patch, et je n’ai ni demandé ni reçu de crédit.
    Ce qui m’intéressait, c’était seulement d’améliorer le noyau, pas d’y laisser mon nom.
    Quelques années plus tard, mon nom s’est retrouvé dans la liste des contributeurs pour autre chose, mais dans un cas comme dans l’autre, ça m’était égal.
  • Il a bien été crédité. Le commit contient Reported-by: Ariel Miculas, et cela semble exact.

• Le patch du mainteneur est meilleur.

  1. Il ne perd pas le test fpidx < (PT_FPSCR - PT_FPR0) dans le cas 32 bits.
  2. Il ne perd pas l’affectation de remplacement *data = child->thread.fp_state.fpscr; utilisée quand fpidx est hors limites dans la fonction put.
  3. Il évite la macro inutile FPRINDEX, qui se réduit à une identité quand TS_FPRWIDTH vaut 1.
     Le message de commit du patch original disait « sur PPC32, on peut supposer que TS_FPRWIDTH vaut 1 », tout en incluant quand même FPRNUMBER, FPRHALF et FPRINDEX.
     Ce n’est pas parce qu’on a trouvé une incohérence de tableau facile à corriger qu’il faut accepter tel quel un patch bogué.
     Cela dit, le mainteneur aurait dû reconnaître que Miculas n’avait pas simplement signalé le problème : il l’avait étudié, avait trouvé la cause racine et avait même produit sa propre version d’une correction simple presque identique.
     Reported-by est imprécis pour exprimer la nuance selon laquelle la personne concernée a mené une investigation approfondie et identifié la cause.
  • Je suis d’accord pour dire que le travail du mainteneur est meilleur. Mais il aurait simplement pu faire une revue de code du patch et demander une mise à jour.
    C’est souvent ainsi qu’on procède avec les nouveaux contributeurs dans un projet, et c’est la bonne manière de faire.
    C’est un processus d’apprentissage et d’intégration ; si l’on refait simplement le travail à leur place, on sape leur motivation à contribuer et on leur retire aussi l’occasion d’apprendre.
  • Le débogage est difficile, parfois très difficile. Écrire une correction dans une base de code familière est relativement trivial, surtout quand on dispose déjà d’une version fonctionnelle de la correction précédente comme guide.
    Écrire une deuxième version est toujours plus facile.
    Qualifier cela de simple correction, c’est ne pas comprendre la quantité de travail qu’implique la correction de bugs en développement logiciel.
    Ici, Michael a lui-même reconnu qu’il n’avait pas réussi à reproduire le problème au début et ne l’avait confirmé que plus tard.
    Cela montre bien la quantité de travail que l’OP a consacrée au débogage et à la correction, et il n’est pas correct de s’approprier ce travail.
  • Dire « ce n’est pas parce qu’on a trouvé une incohérence de tableau facile à corriger qu’il faut accepter tel quel un patch bogué » revient à déformer les propos des autres.
    Dans le texte original, il disait vouloir que son patch soit accepté pour obtenir le crédit et devenir contributeur du noyau, et qu’il était prêt à envoyer des versions suivantes en tenant compte des retours.
  • Personne ne dit qu’il fallait accepter le patch original tel quel.
    Le mainteneur aurait pu checkout le code de l’OP, y apporter les changements souhaités, puis prendre les deux minutes nécessaires pour committer en tant que coauteur.
    Ainsi, la meilleure correction aurait été fusionnée et le travail de quelqu’un d’autre aurait aussi été reconnu.
    C’était d’autant plus la meilleure solution que l’OP avait clairement exprimé vouloir être crédité.
    Cela dit, ces mainteneurs de dépôts semblent souvent manquer de savoir-vivre, donc ce n’est pas très surprenant.
  • J’ai envoyé quantité de patchs et de PR à de nombreux projets open source, la plupart du temps pour résoudre mes propres irritants.
    Les PR ne sont souvent pas acceptées pour diverses raisons, mais je ne m’en soucie pas.
    J’envoie des PR pour rendre le code public, pas pour passer du temps à les rendre suffisamment « bonnes » pour être fusionnées.
    Je les fournis telles quelles, et si quelqu’un les apprécie assez, il peut s’occuper de la procédure.
    Certaines ont été fusionnées telles quelles, d’autres ont été un peu modifiées par le propriétaire du dépôt avant d’être fusionnées.
    Je n’écris pas du code open source pour la gloire ou la popularité, mais parce que j’aime ça.
    Les années 80 et 90 me manquent.

• Le noyau Linux n’est pas une récompense, et même si je compatis avec l’auteur du post, il est normal que la communauté préfère la solution la plus maintenable parmi les versions proposées
  Le mainteneur était donc dans son périmètre d’autorité
  Cela dit, il est très facile de créditer la personne qui a contribué au correctif
  Dans Node.js, on attribue le mérite dans les notes de version, et quand on reprend une partie du travail d’une PR en la modifiant, on essaie d’ajouter Co-Authored-By:
  Quand on maintient un projet depuis longtemps, le crédit finit par compter moins pour soi-même, et il est facile d’oublier qu’il compte énormément pour les nouveaux contributeurs
  Dans un cas comme celui-ci, où partager le crédit était important pour l’auteur du post et mérité, le manque d’attention a été une perte à la fois pour lui et pour cette partie de Linux

  • Si vous avez consacré beaucoup d’efforts à un projet pour corriger votre contribution et qu’à la fin on vous dit « ma version est meilleure, allez-vous-en », il y a de quoi être en colère
    Ce n’est pas une façon durable de faire vivre un projet qui dépend des efforts de la communauté
    Pour un projet aussi vaste et respecté que le noyau Linux, je pensais qu’il y aurait des gens capables de veiller à l’intérêt du projet tout en respectant les nouveaux contributeurs
    Personne ne dit qu’il faudrait accepter une solution qui n’est pas la meilleure simplement parce qu’elle vient d’un nouveau contributeur
    La bonne approche consiste à relire le patch, expliquer ce qui pose problème et pourquoi, proposer comment le réécrire, et, si nécessaire, écrire cette partie ensemble afin de devenir coauteurs
    C’est ainsi qu’on crée un flux de contributeurs qui continueront à s’y intéresser
  • « Le noyau Linux n’est pas une récompense » semble, à la lecture de cet article, vrai dans le pire sens du terme
  • Il semble avoir été crédité via le tag Reported-By

• La documentation du noyau définit Suggested-by comme l’une des conventions de tags
  Le tag Suggested-by: indique que l’idée du patch a été proposée par cette personne et garantit qu’elle soit créditée pour cette idée
  Il est notamment indiqué qu’il ne faut pas l’ajouter sans l’autorisation de la personne à l’origine de la suggestion, surtout si l’idée n’a pas été publiée sur un forum public
  Le texte précise aussi que créditer correctement les personnes qui signalent des idées augmente les chances qu’elles aident à nouveau à l’avenir
  Dans cette situation, Suggested-by aurait peut-être été plus approprié
  Cela aurait mieux exprimé le fait qu’il avait trouvé la solution au problème, mais que, n’étant pas familier avec le projet, la syntaxe exacte du patch n’avait pas été conservée
  Référence : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

  • Je n’ai pas comparé les patchs, mais d’après le commentaire ci-dessous [1], la seule différence entre le patch accepté et le patch proposé serait l’emplacement de la modification
    L’un place le ifdef à l’extérieur du if, l’autre dans le if interne ; il ne s’agirait donc pas d’une différence technique, mais d’une différence de style
    Si c’est le cas, l’auteur a fait bien plus que simplement « suggérer » une idée de patch
    Il a débogué le problème et rédigé le patch complet, qui a été accepté avec une petite modification
    1 : https://news.ycombinator.com/item?id=37672558
  • Cette approche est possible elle aussi, mais elle me semble mieux convenir aux cas qui ne concernent pas la sécurité
    Pour les problèmes de sécurité, la rapidité et l’exactitude comptent davantage que la forme, et les mainteneurs du noyau ont correctement traité le sujet en le créditant avec Reported-by
Suggested-by aurait été un peu mieux, mais ce n’est pas ce que voulait l’auteur du post ; il semble qu’il voulait le badge de « contributeur au noyau » sur la base d’un très petit patch

• À voir certaines réactions dans ce fil, j’espère que certains ne géreront surtout pas de développeurs juniors ou intermédiaires
  Si vous avez dans l’équipe un junior motivé, qui soumet une PR utile sur un problème mais de qualité insuffisante, la pire réaction est de le réprimander parce que ce n’est « pas assez bien », de jeter son code puis de le réécrire soi-même
  Cela casse sa motivation, fait perdre du temps à cette personne comme à vous, l’empêche d’apprendre quoi que ce soit, l’empêche de s’améliorer ou de contribuer utilement la prochaine fois, et le dissuade aussi de demander de l’aide ou de contribuer spontanément
  Même s’il s’est attaqué à une zone de code complexe, il existe de meilleures façons d’interagir

  • Gérer et coacher un junior dans une entreprise ou une équipe, ce n’est pas la même chose que recevoir un petit patch d’une personne quelconque sur Internet
    Pour un junior de l’équipe, on s’attend à ce qu’il reste un certain temps, et l’entretien a déjà permis de vérifier dans une certaine mesure ses compétences de base, donc on peut faire confiance à son potentiel de progression
    Une personne quelconque sur Internet n’a aucune obligation de rester, et la plupart de ces correctifs « drive-by » ne débouchent sur rien de durable
    On ne sait pas quelles sont réellement ses compétences, ni comment elle réagira aux demandes de modification
    Du point de vue du management, l’une des choses les plus frustrantes est que quelqu’un place une personne quelconque dans ma ligne hiérarchique et me rende responsable de ses performances
    Maintenant, imaginez qu’une personne sur Internet, pétrie d’un fort sentiment de dû, ait passé quelques jours à produire un correctif médiocre, et qu’on attende de moi que je la coache comme si j’étais son manager : cette analogie ne tient pas
    Avant, je me demandais pourquoi il y avait tant de remous autour des PR non sollicitées quand on héberge un projet sur GitHub, mais maintenant je comprends
    Se faire accuser d’être un « mauvais manager qui casse la motivation et fait perdre du temps » parce qu’on ne veut pas gérer des PR aléatoires, c’est un niveau excessif de sentiment de dû
    Exemple : https://news.ycombinator.com/item?id=25940799
  • Si un junior trop enthousiaste a passé une semaine à résoudre une tâche qui n’était ni prioritaire ni dans son périmètre, on peut le prendre à part et lui dire : « On met ça de côté pour l’instant et on y reviendra plus tard. Pour le moment, X est prioritaire et on ne peut pas consacrer du temps à ça »
    Devenir professionnel, c’est aussi apprendre à séparer sa personne de son travail, et à comprendre que ses collègues ne cherchent pas à nous barrer la route par un plan malveillant
    Tout le monde a déjà vu un projet de plusieurs mois annulé, ou reçu des commentaires durs en code review
    Accepter ce genre de choses et en tirer des leçons fait partie du métier de professionnel
    Bien sûr, les personnes directes et impolies doivent aussi apprendre que cette attitude détruit beaucoup de relations
    Nous ne sommes pas des artistes, nous sommes des professionnels
    Si un ingénieur civil junior conçoit un pont présentant des défauts structurels et que le senior dit « désolé, mais à ce stade c’est difficilement récupérable, je vais devoir tout refaire », le senior n’a rien à se reprocher
    Je n’ai pas envie de rouler sur un pont bancal pour préserver l’ego d’un junior
    On dirait que beaucoup de commentaires, écrits par des gens qui n’ont pas beaucoup d’expérience de maintenance de grands projets open source, voient les mainteneurs comme particulièrement durs
    Quand on maintient même un petit projet open source, traiter les PR de personnes quelconques représente beaucoup de travail, et prend souvent plus de temps que de le faire soi-même
    Il faut relire le code minutieusement, échanger avec le contributeur pour corriger les problèmes, et aligner le tout sur le niveau de qualité attendu et les conventions
    Un junior en entreprise a déjà passé un entretien, alors qu’ici il s’agit d’un inconnu quelconque qui demande un changement
    C’est décevant quand une PR est refusée, mais je comprends les deux points de vue
    En lisant les échanges du mainteneur, il ne me semble pas particulièrement méchant ou impoli
  • Il vaudrait mieux relire. On dirait que cela ne s’est pas vraiment passé ainsi, et qu’il s’agit au minimum d’une reconstruction émotionnelle d’un des deux côtés
  • Ce fil contient beaucoup de commentaires durs, ce qui est vraiment décevant
    On se croirait presque dans un mauvais fil de la LKML
    Cette personne a littéralement fait 95 % du travail, et on lui reproche de vouloir être créditée
    Je ne comprends vraiment pas pourquoi
  • Ce fil ressemble à un paratonnerre qui révèle de mauvaises relations interpersonnelles, et vu l’âge du lectorat de HN, c’est assez embarrassant

• En comparant les deux patchs, on voit une différence claire
  La contribution dans l’article lié modifie le comportement des deux branches if...else, tandis que la contribution finale ne modifie que le chemin dans la branche if
  Le mainteneur avait sans doute une raison de changer le code
  Cela dit, le correctif effectif a été copié presque à 100 %, donc il aurait été approprié de donner le crédit à l’auteur du patch initial

  • C’est pareil pour le patch original dans l’échange de mails ci-dessus
    Le résultat initial n’a pas non plus de flush_fp_to_thread ni de condition else sur l’architecture PPC32
    Le patch final appelle flush_fp_to_thread(child);, traite le cas selon CONFIG_PPC32 à l’intérieur de la vérification de plage de fpidx, puis affecte fpscr si l’on est hors plage
    Il y a donc une vraie différence entre les deux solutions
    Je ne peux pas dire laquelle est correcte ou meilleure, mais il est clair que le résultat diffère
  • En tant que personne qui utilise C depuis longtemps, je serais prudent face à du code truffé de macros personnalisées
    Cela peut indiquer une certaine immaturité en C
    La maintenance d’une grande base de code, c’est pour moitié éteindre des incendies, et pour moitié éviter les collisions de noms
  • Je suis l’auteur de l’article. Malheureusement, je ne peux pas mettre de lien vers mon patch original, car je ne l’ai envoyé qu’à la mailing list de sécurité du noyau Linux
    Ce patch était assez différent, et j’aurais dû l’enregistrer quelque part pour pouvoir le montrer

• Cela fait partie du fonctionnement normal des contributions au kernel
  Souvent, la façon la plus concise de communiquer un problème est un patch, donc on envoie un patch, et l’attente devrait être qu’ils ne l’utiliseront pas tel quel
  Rien qu’en y repensant, cela m’est arrivé au moins trois fois [1][2][3]
  Honnêtement, il ne m’est jamais venu à l’idée de me mettre en colère, parce que mon objectif principal était de corriger le bug, et j’étais content que ce soit fait
  Je recommanderais sincèrement à l’OP de supprimer ce billet de blog. Il le regrettera plus tard
  [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
  [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
  [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

  • Nommer l’autre personne et la jeter ainsi sous le bus donne vraiment une très mauvaise impression
    J’ai été assez surpris de voir le nom de l’autre personne

• Dans les projets open source, ce genre de chose arrive tout le temps, et au final cela ne fait que décourager les contributions futures
  L’an dernier, j’ai pris en charge une issue ouverte depuis plusieurs années, créé une PR, corrigé aussi le code de test, et même fait un travail séparé pour corriger un problème de build
  Le mainteneur n’a repris que la PR qui corrigeait le problème de build et a simplement ignoré l’issue d’origine
  Il n’y a absolument aucune chance que je consacre de nouveau du temps à contribuer à ce projet

  • Chez certaines personnes, tout leur CV GitHub est fait de contributions copiées-collées
    On ouvre une PR, puis on reçoit des commentaires demandant des changements
    Quelques jours plus tard, quand on a le temps de les corriger, elle est déjà fermée par une autre PR soumise par un contributeur régulier
    En regardant cette PR, on voit qu’elle est quasiment identique à mon code, avec seulement un nom de variable changé, et qu’elle est approuvée instantanément selon un critère de review « entre amis »
    Quand on regarde le profil de cette personne, presque toutes ses contributions sont du code copié d’autres personnes
  • En tant que mainteneur d’un petit projet, j’apprécie quand des gens contribuent du code ou des tests
    J’essaie de les aider à nettoyer leur contribution pour qu’elle s’accorde avec le reste du codebase, et c’est ce qu’il y a de plus difficile
    Parce que la plupart des développeurs veulent simplement pousser leur correction plutôt que d’apprendre d’abord les conventions du projet
    Malgré tout, si le code est à peu près correct et qu’après le merge il me suffit de passer une quinzaine de minutes à le retoucher et à ajouter quelques tests, je l’accepte
    Ce cas-ci ressemble aussi à ce genre de situation, et le mainteneur aurait dû agir ainsi
    Le contributeur aurait obtenu le crédit, et le mainteneur n’aurait eu qu’à faire quelques minutes de nettoyage nécessaires pour que ce soit accepté en amont ; tout le monde aurait été content
    Dire « ma version est meilleure » est très arrogant et peu professionnel, qu’elle soit effectivement meilleure ou non
    Si vous n’avez pas réussi à corriger le problème pendant les six années qui ont suivi son premier signalement, vous devez reconnaître que, sans la contribution de cette personne, votre propre correctif n’aurait pas existé non plus
    Voir la « solution » puis reprendre le correctif revient, en pratique, presque à du vol
  • D’après mon expérience, ce que les projets de code veulent comme contributions, ce sont des rapports de bugs et des idées d’amélioration, pas forcément votre code
    Les gens prennent personnellement le fait que leur code ne soit pas accepté ou soit réécrit, mais ils oublient que l’écriture de code n’est qu’une petite partie de la gestion d’un projet
  • C’est un problème courant
    En général, je regarde les rapports de bugs et les demandes de merge pour évaluer comment les patchs seront traités
    Sur les projets qui ne semblent pas propices aux contributions, j’ai tout simplement abandonné de nombreuses corrections de bugs, et le kernel Linux de 2003 en était un exemple
    Certains projets fonctionnent simplement comme ça : mieux vaut ne pas s’en mêler, c’est généralement une perte de temps
    Désormais, depuis la révolution de l’IA et les violations massives des licences open source, je n’ai plus soumis de code open source et je ne le ferai plus

• J’ai vraiment du mal à comprendre la toxicité ici
  Les discours du genre « il ne faut pas faire ça pour la reconnaissance, les étoiles, les récompenses ou le CV » me semblent délirants
  Les gens qui disent ça ont-ils un emploi ? Si oui, pour être cohérents, ils devraient travailler entièrement gratuitement
  Un salaire est aussi une forme de reconnaissance d’une contribution
  Personnellement, même si la solution finale diffère de celle proposée, si elle repose sur la découverte de la solution initiale, ajouter un coauteur est une toute petite chose
  L’auteur a des raisons valables de s’en plaindre, et ce ne sera ni la première ni la dernière fois que quelqu’un a le sentiment que sa contribution n’est pas reconnue

  • Les gens se comportent comme si nous étions des robots à code ne voulant aucune reconnaissance pour le bon travail que nous faisons
    Même si le patch lui-même était inférieur, l’effort de débogage ne l’était certainement pas
  • La situation est différente d’une relation de travail salarié
    Dans un emploi, l’employeur et l’employé concluent un accord selon lequel un travail est effectué en échange d’une rémunération
    Si vous entrez sans prévenir dans une entreprise et commencez à faire un travail que personne n’a demandé, peu de gens soutiendraient que vous avez droit à une rémunération
    L’analogie la plus juste avec ce qui est arrivé à l’OP est plutôt celle-là
  • Il aurait été plus juste de créditer l’auteur original et de se présenter soi-même comme éditeur

• Une réponse du genre « désolé, mais ma version est meilleure. Si tu veux devenir contributeur du noyau Linux, voici un problème que tu peux corriger » est un excellent moyen de faire en sorte que quelqu’un ne veuille plus jamais aider, et de démotiver les autres au passage

  • La formulation réelle était différente
    Le mainteneur disait qu’il n’avait pas pu reproduire le crash avec gdbserver, mais qu’il disposait d’un cas de test montrant le bug, qu’il l’avait vérifié et avait testé le correctif ; il a écrit : « merci pour le patch, mais je voulais corriger ça autrement. Peux-tu essayer le patch ci-dessous et vérifier si le bug est corrigé ? »
    https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...
    Il y avait aussi d’autres commentaires de revue qui n’ont pas été mentionnés
    Ils disaient qu’on ne voyait pas bien l’intérêt d’une macro réservée à PPC32, qu’il fallait éviter autant que possible les #ifdef, et que le patch de Michael était plus facile à comprendre
    https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...
  • Le mainteneur n’a pas vraiment dit ça
    L’OP met des mots dans la bouche de l’autre personne puis s’en indigne, ce qui n’est pas une bonne stratégie, surtout dans une communication écrite
  • Surtout si l’OP a écrit que ce problème avait été signalé il y a 6 ans puis ignoré, le comportement du mainteneur est plutôt mauvais
  • En réalité, les choses ne se sont pas du tout passées ainsi
    L’OP donne l’impression que l’échange a été discourtois, mais ce n’était pas le cas
  • De plus, qu’est-ce qui garantit que, la prochaine fois aussi, le mainteneur ne dira pas encore « ma version est meilleure » en pointant vers un autre bug ?