<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 22, 2017 at 9:22 AM, Robert P. J. Day <span dir="ltr"><<a href="mailto:rpjday@crashcourse.ca" target="_blank">rpjday@crashcourse.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
got into an animated discussion the other day regarding proper<br>
design of patch files associated with a recipe. as a demo, i can<br>
actually use part of a recipe file ripped from the meta-digi-arm<br>
layer, for "kernel-module-qualcomm", whose recipe file contains (in<br>
part):<br>
<br>
SRC_URI = " \<br>
${CAF_MIRROR};destsuffix=${PV}<wbr>;branch=${SRCBRANCH} \<br>
file://qualcomm-pre-up \<br>
file://modprobe-qualcomm.conf \<br>
file://0001-qcacld-Fix-<wbr>compiling-errors-when-BUILD_<wbr>DEBUG_VERSION.patch \<br>
file://0002-Update-cfg80211_<wbr>vendor_event_alloc-call-for-<wbr>newer-ke.patch \<br>
file://0003-wlan_hdd_main-<wbr>Update-cfg80211_ap_stopped-to-<wbr>nl80211_.patch \<br>
file://0004-qcacld-2.0-remove-<wbr>unused-code.patch \<br>
file://0005-Including-header-<wbr>file-for-regulatory_hint_user.<wbr>patch \<br>
file://0006-Updating-calls-to-<wbr>alloc_netdev_mq.patch \<br>
file://0007-wlan_hdd_cfg80211-<wbr>update-cfg80211_inform_bss-<wbr>params-.patch \<br>
file://0008-wlan_hdd_p2p-<wbr>Update-call-to-cfg80211_rx_<wbr>mgmt-for-dif.patch \<br>
file://0009-linux_ac-Fix-for-<wbr>f_dentry.patch \<br>
file://0010-native_sdio-src-<wbr>hif-Do-not-call-to-HIGH-SPEED-<wbr>functi.patch \<br>
file://0011-osdep_adf.h-fix-<wbr>for-undefined-ath_sysctl_<wbr>pktlog_size.patch \<br>
file://0012-Kbuild-Add-<wbr>compilation-flag-based-on-<wbr>kernel-support.patch \<br>
file://0013-Kbuild-do-not-<wbr>compile-the-DEBUG-version-<wbr>inconditiona.patch \<br>
file://0014-Kbuild-Group-most-<wbr>of-the-relevant-DEBUG-options.<wbr>patch \<br>
file://0015-wlan_hdd_cfg80211-<wbr>fix-missing-ifdef-clause.patch \<br>
file://0016-Add-.gitignore-<wbr>rules.patch \<br>
file://0017-wlan_hdd_main-<wbr>initialize-all-adapter-<wbr>completion-vari.patch \<br>
file://0018-qcacld-Indicate-<wbr>disconnect-event-to-upper-<wbr>layers.patch \<br>
file://0019-wdd_hdd_main-<wbr>Print-con_mode-to-clearly-see-<wbr>if-in-FTM.patch \<br>
file://0020-Makefile-Pass-<wbr>BUILD_DEBUG_VERSION-to-kbuild-<wbr>system.patch \<br>
file://0021-cosmetic-change-<wbr>log-level.patch \<br>
file://0022-fix-issue-with-_<wbr>scan_callback.patch \<br>
"<br>
<br>
when i was presented with something resembling the above the other<br>
day, my immediate reaction was , "yuck." at the time, i opined that,<br>
if the purpose of the above was to simply add a qualcomm module, it<br>
was silly to do that with a lengthy, numbered series of patches that<br>
were almost certainly inter-dependent and even numbered to enforce a<br>
particular processing order.<br>
<br>
i insisted that the above was unnecessarily messy and, if a<br>
developer tried to examine the patches, it would be next to impossible<br>
to do that easily, as it would require walking through each patch,<br>
then mentally adding the next one, and so on and so on.<br>
<br>
i suggested that, in cases like this, adding a well-defined<br>
functionality should be done with a single all-encompassing patch or,<br>
at the very worst, a small number of patches that might be applied<br>
independently. as in, "Add-qualcomm-driver.patch".<br>
<br>
the argument i got coming back was that the above made sense as it<br>
reflected the *history* of how that driver came to be added. i<br>
shuddered, and pointed out that if one wanted to see the history,<br>
that's what version control is for -- there is no reason that the<br>
information from version control should be used *verbatim* to generate<br>
the patches for a recipe.<br></blockquote><div><br></div><div>If the patches have been posted (or are otherwise public), then tracking the history</div><div>on the same granularity is a good thing. It helps maintenance and debugging. </div><div><br></div><div>By having the functionality in an "upstream acceptable" history, i.e. one you'd submit</div><div>to the mainline kernel, you have a level of bisectability and other debug techniques</div><div>that will work.</div><div><br></div><div>If it is tracking development history, that would be squashed for an upstream</div><div>submission .. then yes, you don't need it in the patches either.</div><div><br></div><div>But really, if there are a lot of patches, then they should already be in a repo and</div><div>built from that repo directly (versus exporting patches and maintaining them</div><div>separately). But perhaps the export is done to save time and the size of the</div><div>build, etc, etc.</div><div><br></div><div>Bruce</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
in any event, i can appreciate the value of sometimes adding<br>
numbering to patches to enforce a processing order, but when one has a<br>
numbered series of patches getting up to 20 or 25, that strikes me as<br>
time to collapse all that into larger and more modular and more<br>
meaningful patches.<br>
<br>
thoughts?<br>
<br>
rday<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
<br>
==============================<wbr>==============================<wbr>============<br>
Robert P. J. Day Ottawa, Ontario, CANADA<br>
<a href="http://crashcourse.ca" rel="noreferrer" target="_blank">http://crashcourse.ca</a><br>
<br>
Twitter: <a href="http://twitter.com/rpjday" rel="noreferrer" target="_blank">http://twitter.com/rpjday</a><br>
LinkedIn: <a href="http://ca.linkedin.com/in/rpjday" rel="noreferrer" target="_blank">http://ca.linkedin.com/in/<wbr>rpjday</a><br>
==============================<wbr>==============================<wbr>============<br>
<br>
<br>
--<br>
______________________________<wbr>_________________<br>
Openembedded-core mailing list<br>
<a href="mailto:Openembedded-core@lists.openembedded.org">Openembedded-core@lists.<wbr>openembedded.org</a><br>
<a href="http://lists.openembedded.org/mailman/listinfo/openembedded-core" rel="noreferrer" target="_blank">http://lists.openembedded.org/<wbr>mailman/listinfo/openembedded-<wbr>core</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"</div>
</div></div>