RFC: CI: Provide a summary of memory map changes

[DavidVentura]

This PR adds a step in CI, with a script printing as a comment the changes to the memory map
Example:

The PR itself clashes a bit with #294 but I can rebase from that if preferred
The python script is a modification of this script, using something else would also be fine, my goal with the PR is the summary on changes.

I think this ties nicely with #313 moving forward

[JF002]

Hi and thanks for this RFC/PR! I think this is a really good idea! 👍

As you've already noticed, there are a few PR related to Github actions that are still pending, and that's mostly my own fault. Lake of time and of knowledge about Actions do not help.

I'll however try to find the time to have a better look at them as I think CI can help us a lot, and especially if you use awesome tools like yours!

[DavidVentura]

As you've already noticed, there are a few PR related to Github actions that are still pending, and that's mostly my own fault. Lake of time and of knowledge about Actions do not help.

no worries! most of them address the same issues in the same way, so if you prefer to merge one of those first, I'm happy to rebase afterwards

[JF002]

Ok, I'm trying to focus again on these PR related to the CI!
Your PR seems to work and provides additional feature, and I think I'll use it as the first one to make the Action work again ;)

But uh... where can I see the output of the script (the comment by the action-bot) ? I can see on your repo that the Actions were successful, but I cannot find the comment with the table with the changes.

EDIT: I found it, in the comments of the PRs that were built by the Actions!

I also noticed that the version of InfiniTime is hardcoded into the workflow file (line 137, for example). Would it be possible to make this file adapt automatically to the version of InfiniTime?

Or should we generate the workflow file via CMake to specify the current version of InfiniTime when it's configuring the project?

[T0RAT0RA]

Upgrading the version avoid using the unsecure flag:

Suggested change

	uses: fiam/arm-none-eabi-gcc@v1.0.2
	env:
	ACTIONS_ALLOW_UNSECURE_COMMANDS: true
	uses: fiam/arm-none-eabi-gcc@v1.0.4

(See https://github.com/JF002/InfiniTime/pull/534/files#r678790589)

[T0RAT0RA]

Upgrading the version avoid having to install cbor manually:

Suggested change

	git clone --branch v1.5.0 https://github.com/JuulLabs-OSS/mcuboot
	git clone --branch v1.7.2 https://github.com/JuulLabs-OSS/mcuboot

(See https://github.com/JF002/InfiniTime/pull/534/files#r678790633)

[T0RAT0RA]

No need if you use mcuboot v1.7.2

Suggested change

pip3 install --user 'cbor>=1.0.0'

[mabuch]

I do not think, that the proposed design solution should be implemented like this. The reason being that this PR always runs two builds, the "old" version it uses as reference as well as the new version.

In general, this to me seems like a lot of unnecessary CPU usage. Because we most likely want to differentiate between commits and pull-requests. I do not know whether this usage analysis really is all that interesting for commits to the master branch (and develop branch as proposed here).
For pull requests this seems to be a nice idea, in order to give a quick overview of the size of the change. I therefore recommend splitting the workflow into two workflows, one for commits and one for pull-requests. I think it would be sufficient to have this analysis for pull-requests.

Should this be implemented, I also recommend to choose an approach that works for the docker build chain as well (and others, if there are others). As I have no experience with this, I cannot tell if this already could be used there or whether some changes need to be made for this.

---

An alternative to always recomputing the *.map of the reference version could be to generally (automatically) store the *.map file for every run of the Github Action. This would eliminate the re-computation of comparison-versions and we would create an archive of all *.map files for every version, something that might be useful anyways.

---

These are just my thoughts.

---

@JF002 Regarding

Or should we generate the workflow file via CMake to specify the current version of InfiniTime when it's configuring the project?

This would be a bad idea in my optinion. It would be better to change the script to work without hardcoded version numbers.

[JF002]

CI builds were (finally) fixed in #579. Feel free to contribute and improve Github Actions configuration in a new PR ;)