weixin_39517520
weixin_39517520
2021-01-10 03:57

Restructure patch ordering and dissolve 90000-wip-inflight-to-upstream.patch

Per discussion and suggestion from order patches to avoid patch order dependent patch failures.

Why are there nuttx-patches?

Upstream submission

  1. Upstream may reject a patch for "not applicable" An example of this would be

    00002-fix-shadow-wanings.patch
    PX4 build with stricter warnings than does upstream. The patch applies compiler specific pragmas that have been rejected upstream.
  2. PRs that go upstream a can come back very different. a) they can get reformatted and the essence of the PR can be lost in b) multiple further upstream formatting commits or c) committed with other wip in upstream. This is not an issue on the PX4 branches tied to the PX4 NuttX upstream branch. The PX4 NuttX upstream branch tracks upstream NuttX master at a regular uptake rate and I have to keep it building and running. On PX4/Firmware master The PX4 NuttX is lagging behind upstream and we want only our changes.

  3. Taken as is. This is not an issue on the PX4 NuttX upstream branch that tracks upstream NuttX master but on PX4/Firmware master that is lagging behind upstream these become backports.

What it 90000-wip-inflight-to-upstream.patch?

On the PX4 branches tied to the PX4 NuttX upstream branch: This file has the changes to keep the development and build moving along while the work in process is in flight to upstream. Depending on the outcome of the Upstream submission - the content can be 1) moved to a non-upstream able permanent patch. In Case 2) or 3) the contest can be removed once the PX4 NuttX upstream branch has been merged with the upstream NuttX master.

On the PX4 Firmware master branch: This file has the changes to keep the development and build moving along while the work in process is iflight to upstream. Depending on the outcome of the Upstream submission - the content can be 1) moved to a non-upstream able permanent patch. Or in case 2) and 3) Backported against masters NuttX submodules, This usually requires manual intervention to remove disjoint files that got committed upstream with the PX4 contribution or in the subsequent beautification edits. Therefore the patch file is cleaned up and saved as a 6000 bp series patch file.

Why are you not using Git for this? (If I only could)

Workflow incompatibilities with upstream NuttX. 1. Upstream NuttX master may be used as a development branch from time to time. Lacking a re integration workflow means there will not be a clean commit set.

  1. As has happened in the past a the upstream repos may be reorganized as submodule and then brought back into the tree.

Therefore rebasing will not work and even cherry picking can be difficult. With this KISS approach we can see all the changes PX4 requires to NuttX and edit and re-apply them when the upstream files set changes.

I am open to suggestions on a better way - to date I have not found one.

Got Some great suggestions and feedback - Thank you!

Naming has been changed per recommendation to enforce ordering and classification


00001-PENDING-fix-xyz.patch
00002-BACKPORT-fix-abc.patch
00003-REJECTED-remove-shadow-warnings.patch

cmake has been taught to order the patches based on name.

该提问来源于开源项目:PX4/Firmware

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

8条回答

  • weixin_39917211 weixin_39917211 3月前

    LGTM

    点赞 评论 复制链接分享
  • weixin_39854369 weixin_39854369 3月前

    maybe we should keep the numbering only so we have a correct order in which patches are applied. Then the status we keep in the next "field". Example:

    
    00001-PENDING-fix-xyz.patch
    00002-BACKPORT-fix-abc.patch
    00003-REJECTED-remove-shadow-warnings.patch
    ...
    

    If the commit messages contain these words, you can even use git to add the correct patch when you are adding a new one:

    
    git format-patch -1 --start-number <next-sequence-number> --src-prefix=a/NuttX/nuttx/  --dst-prefix=b/NuttX/nuttx/
    </next-sequence-number>

    Tha would also preserve the commit message, which would be nice, and allow to be easily applied.

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 3月前

    The naming sounds good.

    But I think you assuming I can get a clean commits from upstream. I am concerned that might bringing other unrelated changes as just experienced. What would you suggest there?

    点赞 评论 复制链接分享
  • weixin_39966602 weixin_39966602 3月前

    At some point we'll need to revisit how PX4 wraps the Nuttx build. It generally works pretty well, but still has a few issues. https://github.com/PX4/Firmware/issues/6501 https://github.com/PX4/Firmware/issues/6820

    CMake ExternalProject_Add might be a better solution. https://cmake.org/cmake/help/v3.0/module/ExternalProject.html

    点赞 评论 复制链接分享
  • weixin_39854369 weixin_39854369 3月前

    If the unrelated changes are (potentially) harmful, I'd add an additional step of editdiff file.patch. Or the equivalent using git tools - you could have px4-stash branch in which you apply the patches before generating it to be applied on PX4. If it's not harmful, just bring the change since we will get it nonetheless when we upgrade.

    点赞 评论 复制链接分享
  • weixin_39812577 weixin_39812577 3月前

    Are you ok to merge as-is or do you want further cleanup before we do?

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 3月前

    once CI is happy (I got the ordering correct for all builds) this is GTG.

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 3月前

    At some point we'll need to revisit how PX4 wraps the Nuttx build. It generally works pretty well, but still >has a few issues.

    6501

    6820

    CMake ExternalProject_Add might be a better solution. >https://cmake.org/cmake/help/v3.0/module/ExternalProject.html

    Continuing "Revisit how PX4 wraps the Nuttx build" here

    点赞 评论 复制链接分享