Revisit Nuttx Build

[davids5]

@dagar

Moving this here.

Revisiting the build is fine.

We still need to maintain the make oldconfig and menuconfig in whatever we do. We need to keep in mind that a defconfig changes need a clean build. We need to keep in mind that a patch change needs a clean copy of the affected files in the patch.

Getting the incremental build to work when switching branches would be nice if it can be done. Getting the incremental build to work making edits in the NuttX subtree would be nice.

One gotcha is debugging via eclipse in the build dirs. If one edits the build dirs copy of the file without realizing it you can lose your edits (and your mind).

It would great if we could fix that or offer to merge the changes. The problem will be the patched files. Still a copy back would be prefered.

[dagar]

What about applying the patches in place? Does Nuttx have an out of source build?

[davids5]

What about applying the patches in place?

We did that before cmake. The was an objection: why is "NuttX" dirty? - Finding a git based solution as opposed to patches would be the answer. But I need the same level of control I get with the patches. Multi branches tied to different Nuttx submodels, support of multiple submodules and in multiple repos.

Does Nuttx have an out of source build?

No and since we iterate board configs as we build. in tre would be `` distclean cp configs in build export (create zip) unzip export

Whis is exactly what we had before cmake.

[lucasdemarchi]

@davids5 I think the way patches are applied done in #6878 is a great improvement so this can be in for now.

I'd still like to keep the patches on a branch in our fork of NuttX... this would ease how we apply newer patches, not needing the steps for "1. create patch with right prefix; 2) possibly edit the path; 3) copy do PX4/Firmware".

Since now the patches are sorted, we can easily create a branch on NuttX that has these patches applied. Then we can remove the infra to apply the patches on CMake and rather work with submodule updates.

After that I think the next step is to add support for building NuttX out of tree, so we don't need the export + unzip sequences. This may give us some trouble if upstream doesn't care about it. If it proves too troublesome we can think of a way to have in-tree build with the right set of .gitignore files so it doesn't cause the dirty submodule you mentioned.

[davids5]

@lucasdemarchi

I am really glad you are looking this. I have a workable solution that allows me to continue to develop, with now a minimal impact on non NuttX developers.

I'd still like to keep the patches on a branch in our fork of NuttX... this would ease how we apply newer patches, not needing the steps for "1. create patch with right prefix; 2) possibly edit the path; 3) copy do PX4/Firmware".

For a fork that the maintainer updates they can do as they like. We can even add support to the build to turn off the patches.

But this will not work for my work flow and development needs.

1) My upstream tracking development gets broken often by upstream non autonomous commits that can persist for days or weeks. I am speaking about massive breaking API and functional changes done on master the do not have a clean commit history.

While upstream does look out for thing that my creator PX4 and give me heads up, we can not change the upstream workflow or practices.

2) Not all patches are public. Therefore XYZ/Firmware needs to drive the changes so that a common NuttX can be used. Otherwize each XYZ team will need a PX4 nuttx fork and a maintainer.
That may work for your team but it does not scale for all teams.

Please keep in mind - we have no upstream support in changing the build. It is recursive Make with no degree of freedom. The export build it the only upstream supported facility (and we carry a patch for that)

Also I need to have a functional git environment so if there is a conditional .gitignore - that would be great.

If you would propose a step-by-step workflow - I would be happy to see if I can use it in all the use cases I have.

My current work flow on the upstream branch of NuttX and it's submodules.

1) Update NuttX's submodules only upstream from upstream master foreach subfolder git fetch and pull 2) Review all arch/changes in supported archs I support for all vendors. 3) run oldconfig and on all 20+ board configs. foreach board make target oldconfig_target 4) Build all 20+ board configs. make check 6) Mitigate changes. (this will likely generate patches to keep me working) git diff --no-prefix --dst-prefix=NuttX/nuttx/ --src-prefix=NuttX/nuttx/ >>../../nuttx-patches/wip_inflight_to_upstream.patch Submit applicable fixes to upstream - once upstreamed and settled start at 1 again.

7) Test on 3 major targets px4fmu-v3_default px4fmu-v5_default, and others. 8) Push all submodules. foreach subfolder of NuttX git push 9) commit on NuttX to the new nuttx submodules. git add [nuttx] [apps] [...] git commit Document PX4 contribs and whatever else came in 8) Push NuttX git push origin upstream 10) On PX4/Firmware (branch) Update / commit defconfig changes and NuttX submodule. git add NuttX git commit Document PX4 contribs and whatever else came in. git add patches git commit git add and commit each board change 11) Push PX4/Firmware (branch) git push origin branch 12) Wait for CI -

rinse-lather-force push and repeat. :)

Out of the above I get the applicable changes and backports this to FMU/Firmware master. git fetch origin git checkout master git pull origin master (I know old habits die hard) git checkout -b master_<branchname>

gitsync (git submodule sync --recursive && git submodule update --init --recursive) Depending on what changed work in that NuttX subfolder git checkout -b <basegithash>_branchname foreach change (all related hashes) git cherry-pick changed-hashes git diff \<basegithash\> --no-prefix --dst-prefix=NuttX/nuttx/ --src-prefix=NuttX/nuttx/ >../../nuttx-patches/< new or update> ( I suppose I should be using git format-pathch here)

Removed cruft and breaking changes from patches. commit and push

Once CI is happy rebase all repos I support.

[lucasdemarchi]

Hi @davids5 .. I think what I'm suggesting is basically replacing the step 10 and 11. I prototyped it here to see if it would work and apparently it did. I'm pushing for you to take a look.

Instead of generating the patch, copying it to nuttx-patches dir and making the build system to manually apply them, this would be replaced with:

10. Switch to branch px4 on the submodule that's being patched and backport the patch (or create the patch that's not material for upstream).
11. In NuttX, update the submodule
12. In PX4/Firmware, update the submodule.

With this there's no need anymore to create the patch files and instruct the build system to apply them. It's implicitly done by updating the submodule. Let me know what you think.

https://github.com/lucasdemarchi/Firmware/commits/pr-build-nuttx https://github.com/lucasdemarchi/PX4NuttX-1/commits/px4 https://github.com/lucasdemarchi/nuttx/commits/px4 https://github.com/lucasdemarchi/apps/commits/px4

Since you already have to split the commits per submodule in order to send upstream, I don't see this as a problem, but as I said I don't want to disrupt your workflow since you are the one doing the heavy lift work.

[lucasdemarchi]

And for example, to add the changes from @zehortigoza would be basically to apply the branch https://github.com/lucasdemarchi/nuttx/commits/pr-zeh-changes on the px4 branch that we would maintain and propagate the submodule updates.

That branch had one conflict when I was backporting it, which is easier to solve with the git tools. And the branch is built-tested only. @zehortigoza can you take a look if I got the conflict correctly?

[davids5]

@lucasdemarchi Can we discuss this on a call?

[davids5]

Hi @lucasdemarchi

I was thinking about the discussion we had, and I am looking forward to seeing the workflow (with commands) laid out.

Please set me straight if I am missing the point, or some git skills/tools that invalidate the following:

But this is what I see:

A B PX4NuttX - (the knot)
 +A B NxWidgets 
 +A B apps
 +A B misc/tools
 +A B nuttx

Assuming I have only: (A) master PX4 tied to one currently a FIXED commit on PX4NuttX and (B) master_some_arch_tied_to_upsteam_nuttx to my FLOATING upstream branch commit on PX4NuttX

A needs all {REJECTS,BACKPORTS, PENDING} B needs all {REJECTS, PENDING}

I think the proposal would require me to carry, maintain and keep in sync 10 instances of branches.

A) 5 repos with one workflow - commits history is are applied patches. B) 5 with I think??? a rebased workflow on upstream master or we lose the changes in the history.

With my current solution I have 2 to branches to maintain and (2 A&B) single directories to manage. The operation for B is always foreach dir git pull upstream master

These 2 patch directories answers the question: How is this PX4 branch of nuttx different from upstream NuttX?

Patches: There is no different workflow required to answer that question for A & B.
Git: A seems easy, the last commits are the deltas. B where are they in the 6-12-25 month cycle? On top if we use a rebase workflow. I guess the force pushes would be ok if I was the only one on B.

I think I can never force push on A. - correct?

Patches: On B When a patch fails can see the intent in the patch and simply redo it. Git:I have potentially multiple merge failures (ultimately sqush-able) but instead of looking at the diff. I have to use git blame and reconcile the conflict in an N way diff - or do i have that wrong?

Also to benefit from CI on A I would need to feature branch to not clobber master after updating the PXNuttX submodules and the NuttX submodule commit.

Do I have this right?

[davids5]

@dagar - I think the parallel build is not respecting the patch ordering. Per our chat I am noting that here.

[dagar]

@davids5 @lucasdemarchi I was playing around with git and it makes it really easy to go back and forth between commits and patch files.

For example, if you were working in a branch on top of upstream (I tagged it upstream) you could turn all commits into patches with a single git format-patch. I omitted some details like path setup.

git format-patch upstream

/home/dagar/git/Firmware/nuttx-patches/nuttx/0001-REJECTED-add-math.h.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0002-REJECTED-fix-shadow-wanings.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0003-REJECTED-avoid-export-copy-with-export-insitu.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0004-REJECTED-support-c-11.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0005-REJECTED-cstdint-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0006-REJECTED-static-assert-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0007-REJECTED-ctype-fix-shadow-wanings.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0008-BACKPORT-stm32-flash-F4-dcache-corruption-fix-no-HSI.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0009-BACKPORT-priority-restoration-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0010-BACKPORT-stack-coloration-overreach-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0011-BACKPORT-stm32-serial-dma-hotfix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0012-BACKPORT-i2c-hotfix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0013-BACKPORT-stm32f7-DTCM.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0014-BACKPORT-fix-CRTSCTS-defines.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0015-BACKPORT-cdcacm.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0016-BACKPORT-stm32-serial-break.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0017-BACKPORT-stm32-rcc-keep-HSI-on.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0018-BACKPORT-ramtron-CONFIG-prefix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0019-BACKPORT-stm32f3x-add-BKP.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0020-BACKPORT-stm32-bkp-reference-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0021-BACKPORT-stm32f7-bkp-reference-fix.patch
/home/dagar/git/Firmware/nuttx-patches/nuttx/0022-BACKPORT-stm32f7-serial-dma-hotfix.patch
/home/dagar/git/Firmware/nuttx-patches/apps/0001-REJECTED-add-var-expansion-in-nsh-parse.patch
/home/dagar/git/Firmware/nuttx-patches/apps/0002-REJECTED-silence-jobserver-warnings.patch
/home/dagar/git/Firmware/nuttx-patches/apps/0003-BACKPORT-add-set-ex-to-nsh.patch

You can also apply all patches as a commit each with one command.

git am *.patch

Applying: REJECTED-add-math.h
Applying: REJECTED-fix-shadow-wanings
Applying: REJECTED-avoid-export-copy-with-export-insitu
Applying: REJECTED-support-c++11
Applying: REJECTED-cstdint-fix
Applying: REJECTED-static-assert-fix
Applying: REJECTED-ctype-fix-shadow-wanings
Applying: BACKPORT-stm32-flash-F4-dcache-corruption-fix-no-HSI-on
Applying: BACKPORT-priority-restoration-fix
Applying: BACKPORT-stack-coloration-overreach-fix
Applying: BACKPORT-stm32-serial-dma-hotfix
Applying: BACKPORT-i2c-hotfix
Applying: BACKPORT-stm32f7-DTCM
Applying: BACKPORT-fix-CRTSCTS-defines
Applying: BACKPORT-cdcacm
Applying: BACKPORT-stm32-serial-break
Applying: BACKPORT-stm32-rcc-keep-HSI-on
Applying: BACKPORT-ramtron-CONFIG-prefix
Applying: BACKPORT-stm32f3x-add-BKP
Applying: BACKPORT-stm32-bkp-reference-fix
Applying: BACKPORT-stm32f7-bkp-reference-fix
Applying: BACKPORT-stm32f7-serial-dma-hotfix
Applying: REJECTED-add-var-expansion-in-nsh-parse
Applying: REJECTED-silence-jobserver-warnings
Applying: BACKPORT-add-set-ex-to-nsh

With corresponding git commits.

* 4ce6edd - (HEAD) BACKPORT-stm32f7-serial-dma-hotfix (11 minutes ago) <Daniel Agar>
* 45952c1 - BACKPORT-stm32f7-bkp-reference-fix (11 minutes ago) <Daniel Agar>
* 4ad4d0b - BACKPORT-stm32-bkp-reference-fix (11 minutes ago) <Daniel Agar>
* 8d7fddd - BACKPORT-stm32f3x-add-BKP (11 minutes ago) <Daniel Agar>
* c75697d - BACKPORT-ramtron-CONFIG-prefix (11 minutes ago) <Daniel Agar>
* c74a31c - BACKPORT-stm32-rcc-keep-HSI-on (11 minutes ago) <Daniel Agar>
* 00ae5e5 - BACKPORT-stm32-serial-break (11 minutes ago) <Daniel Agar>
* 7d58183 - BACKPORT-cdcacm (11 minutes ago) <Daniel Agar>
* 007a8c2 - BACKPORT-fix-CRTSCTS-defines (11 minutes ago) <Daniel Agar>
* 63fe8fb - BACKPORT-stm32f7-DTCM (11 minutes ago) <Daniel Agar>
* 13ed09d - BACKPORT-i2c-hotfix (11 minutes ago) <Daniel Agar>
* 60952a0 - BACKPORT-stm32-serial-dma-hotfix (11 minutes ago) <Daniel Agar>
* 3161058 - BACKPORT-stack-coloration-overreach-fix (11 minutes ago) <Daniel Agar>
* 28a7865 - BACKPORT-priority-restoration-fix (11 minutes ago) <Daniel Agar>
* 731a73f - BACKPORT-stm32-flash-F4-dcache-corruption-fix-no-HSI-on (11 minutes ago) <Daniel Agar>
* 2e2a354 - REJECTED-ctype-fix-shadow-wanings (11 minutes ago) <Daniel Agar>
* 9932d93 - REJECTED-static-assert-fix (11 minutes ago) <Daniel Agar>
* 44e388d - REJECTED-cstdint-fix (11 minutes ago) <Daniel Agar>
* 0b43885 - REJECTED-support-c++11 (11 minutes ago) <Daniel Agar>
* a5c73c7 - REJECTED-avoid-export-copy-with-export-insitu (11 minutes ago) <Daniel Agar>
* be8b038 - REJECTED-fix-shadow-wanings (11 minutes ago) <Daniel Agar>
* 4f04b44 - REJECTED-add-math.h (11 minutes ago) <Daniel Agar>
*   8b81cf5 - (tag: upstream) Merged in david_s5/nuttx-3/david_s5/typo-in-stm32f76xx77xx_pinmaph-edited-on-1481298811328 (pull request #182) (4 months ago) <Gregory Nutt>

Each patch file is the same diff, but would also include the committer, date, and commit message. https://github.com/dagar/Firmware/blob/nuttx_test/nuttx-patches/nuttx/0001-REJECTED-add-math.h.patch

So we could really get the best of both worlds with this approach. @davids5 could shuffle patches to keep track of these things, but also trivially apply them to a branch off of PX4's NuttX repo. Additionally, if we don't have to handle patches for every single NuttX build I have a solution that will allow you to edit NuttX files in place.

https://github.com/dagar/Firmware/tree/nuttx_test

[lucasdemarchi]

@dagar yep. It much easier to keep the commits on a git branch. This is basically what I did on the branches pointed above. As I said the difference is basically if patches are kept as separate files or in a branch. Keeping them on a branch means we don't have to apply them at every build and avoid all headaches that this entails (see the amount of issues that this causes).

The only downside is that Nuttx itself has submodules and thus you can't have patches crossing submodules. However since the end goal is to upstream the patches, they should already be split on submodule boundary anyway

[davids5]

@dagar , @lucasdemarchi

I was thinking about the discussion we had, and I am looking forward to seeing the workflow (with commands) laid out.

Please see the https://github.com/PX4/Firmware/issues/6881#issuecomment-289071535 If you both a have a` complete solution, that supports the work I have described above. Please answer the questions and provide a complete workflow with all the commands. I need to see the end-to-end solution. The pieces are nice. But without a complete picture. I can not tell if you have complete solution that is more or less work with the same number of branches and level of support.

In your scenarios: Assume the current nuttx (44ad7e224c1ef17911ab8b4101fd624ad9ee4177) has only back port development only. It is Nuttx + rejects + backports.

The upstream branch of NuttX - is always the latest upstream. That has say PX4 master_nxphlite tied to it.

When dev is done on NuttX upstream it is branched as upstream_kinetis upstream_kinetis is the WIP NuttX branch that is rebased on upstream NuttX and has the work in progress for that processor. That will go upstream and get picked up in the next upstream uptake.

While that PR (opr patch set) is inflight to upstream, master_nxphlite is tied back to NuttX upstream and the patches are placed in a pending (wip_inflight) patch file. Once that PR is merge into master upstream. wip_inflight is deleted and master_nxphlite is then tied and built against upstream. This can happen 5 time a day.

Only the rejects and pending patches are in PX4/Firmware master_nxphlite . master_nxphlite bounces between upstream_kinetis and upstream and the pending patches come and go.

master_nxphlite is constantly rebased on PX4 master.

Using 3 px4 repos say PX4, NXP, INTEL with only PX4 having a NuttX submodules. NXP and INTEL just reference PX4/NuttX. INTEL and NXP repos may have different patches, depending on the development phase.

Please describe in detail the work flow in maintaining PX4, NXP, INTEL. for current NuttX and upstream

[lucasdemarchi]

Basically what I'm suggesting is "Don't change your workflow, just change how patches are applied in the end".

This means that you only replace the last step on your workflow: instead of copying a .patch file to PX4/Firmware/nuttx-patches you just switch to the px4 branch on that repository and commit. Example:

a) you added a bug fix b) you generate a patch c) you send it to upstream d) you copy it to PX4/Firmware e) you commit there and raise a PR

You would change (d) and (e) by:

d) switch to px4 branch e) commit there and raise a PR updating the submodule

Notice that on (d) you will already be on top of your pile of changes before, so you can check for conflicts with previous changes at time of applying the patch rather than having to build it and hope things apply cleanly.

With this little change it's possible to remove all the infrastructure from the build system to apply patches (for everyone) while building. This is exactly what I've done on the branches I pointed out: https://github.com/lucasdemarchi/Firmware/commits/pr-build-nuttx https://github.com/lucasdemarchi/PX4NuttX-1/commits/px4 https://github.com/lucasdemarchi/nuttx/commits/px4 https://github.com/lucasdemarchi/apps/commits/px4

(these branches are outdated, but I can easily update them if we want to at least try this approach).

All the issues with patch order, handling dependencies, etc, go away.

Look how this has nothing to do with having repos PX4, NXP, INTEL... this is a non-issue: there are only 2 interesting repositories: upstream NuttX and px4 branch.

[lucasdemarchi]

@dagar why did you close this? This issue is about changing the workflow, not about was committed on #7007. I'm waiting on feedback from @davids5

[dagar]

This was referenced (and closed automatically) by #7007 because the original reason this was opened was my suggestion to replace our custom NuttX build wrapper with cmake ExternalProject_Add. I decided to just fix the dependencies in our custom wrapper instead.

Reopening.

[davids5]

@lucasdemarchi

Notice that on (d) you will already be on top of your pile of changes before, so you can check for conflicts with previous changes at time of applying the patch rather than having to build it and hope things apply cleanly.

We have the ordering issue resolved now.

I think what you are proposing is only applicable to the non development version (current) of NuttX. Right?

It does not make the workflow I have for "upstream" work the same. This means I still have to carry a set of commits or patches to support rejected and wip. In the case of commits - I would have to constantly rebase the affected NuttX submodules on upstream in multiple repos. With patches I can avoid that.

I am not seeing the advantage to having 2 ways to do the same thing.

[lucasdemarchi]

I think what you are proposing is only applicable to the non development version (current) of NuttX. Right?

Not really. It applicable to easily manage multiple versions/branches of NuttX.

It does not make the workflow I have for "upstream" work the same. This means I still have to carry a set of commits or patches to support rejected and wip. In the case of commits - I would have to constantly rebase the affected NuttX submodules on upstream in multiple repos.

You don't really have to rebase. Merge and/or revert still work. Note that we are not changing the NuttX revision for very long times, so I don't see where this "constantly rebase" is coming from. Looking at the log, the last time we synced with upstream was in 07923a8 (Upgrade to Nuttx 7.18+ ==upstream)

[davids5]

@lucasdemarchi see

https://github.com/PX4/Firmware/tree/master_nxphlite (I have not commited the latest there yet as the HW is in WIP)

It flops between: https://github.com/PX4/PX4NuttX/commits/upstream and https://github.com/PX4/PX4NuttX/tree/upstream_kinetis

[davids5]

Done in 1.7 NuttX upgrade