Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix version.dtsi is reset after local firmware build #385

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

huber-th
Copy link
Contributor

Advantage 360 Pro PR template

I discovered yesterday that the command I introduced in #376 to reset the version.dtsi file after a local build to reduce noise on code changes to repositories is not behaving as intended and went undetected during my testing given that my checked in version.dtsi was from the same day and branch. Apologies for that.

What's changed:

This change updates how the git command to reset the version.dtsi is being called. It is no longer called within a shell function to ensure it does run in the order it is defined after the firmware build is complete.

Why has this change been implemented:

In #376 a new step was introduced for local builds to undo changes to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi file to be reset too early and therefore causes an incorrect version number to be used for the version macro when run locally. This went unfortunately undiscovered as the checked in version.dtsi was the same on the day the change in #376 was tested and was not noticed until I build a new change to my keymap locally a few days ago.

The git command introduced in #376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when the function calls are expanded by make. This causes the version.dtsi file to be reset before the firmware build process is even started which resultes in the version.dtsi currently checked in to the repo to be used for local builds instead of the newly generated file when make starts.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:

What (if any) actions must a user take after this change:

No further actions must be taken by users. Local builds will start using the correct version number generated by bin/get_version.sh again while maintaining the reset of the version.dtsi file after a local firmware build is complete.

In #376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in #376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in #376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
@ReFil
Copy link
Collaborator

ReFil commented Feb 20, 2024

Thanks for spotting and fixing this!

@ReFil ReFil merged commit dd3da03 into KinesisCorporation:V3.0 Feb 20, 2024
1 check passed
@huber-th huber-th deleted the bug/makefile branch February 20, 2024 15:22
cr0rc pushed a commit to cr0rc/Adv360-Pro-ZMK that referenced this pull request Feb 25, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
arogulin pushed a commit to arogulin/Adv360-Pro-ZMK that referenced this pull request Feb 26, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
OAGr added a commit to OAGr/Adv360-Pro-ZMK that referenced this pull request Mar 2, 2024
* V3.0: (57 commits)
  Fix version.dtsi is reset after local firmware build (KinesisCorporation#385)
  Update build workflows (KinesisCorporation#376)
  Makefile enhancements to optimize local workflows (KinesisCorporation#363)
  Update Makefile variables (KinesisCorporation#335)
  Base ZMK update (KinesisCorporation#326)
  Prefer `tr` to ${char^^}, which does not work on older bash versions (KinesisCorporation#303)
  Add version macro (KinesisCorporation#300)
  Add pull request template (KinesisCorporation#293)
  Revert "Updated keymap"
  Revert "Updated keymap"
  Updated keymap
  Updated keymap
  Make get_version use bash from $PATH (KinesisCorporation#287)
  Update bluetooth settings in light of user feedback (KinesisCorporation#289)
  Revert "Add version macro to keymap.json (KinesisCorporation#269)"
  Add version macro to keymap.json (KinesisCorporation#269)
  Changelog new base ZMK update (KinesisCorporation#268)
  Version compiled FW with automatic macro (KinesisCorporation#267)
  Document new NKRO settings (KinesisCorporation#264)
  Improve documentation based on feedback (KinesisCorporation#260)
  ...
yukitaka pushed a commit to yukitaka/Adv360-Pro-ZMK that referenced this pull request Mar 21, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
luna-flowers pushed a commit to luna-flowers/Adv360-Pro-ZMK that referenced this pull request Mar 22, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
alok added a commit to alok/Adv360-Pro-ZMK that referenced this pull request Apr 8, 2024
…ro-ZMK

* 'V3.0' of https://github.com/KinesisCorporation/Adv360-Pro-ZMK:
  Fix Makefile errors that prevent builds on macOS (KinesisCorporation#409)
  Fix version.dtsi is reset after local firmware build (KinesisCorporation#385)
  Update build workflows (KinesisCorporation#376)
ztomer pushed a commit to ztomer/Adv360-Pro-ZMK that referenced this pull request Apr 16, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
ztomer pushed a commit to ztomer/Adv360-Pro-ZMK that referenced this pull request Apr 16, 2024
…ion#385)

In KinesisCorporation#376 a new step was introduced for local builds to undo changes
to the version.dtsi file after a build in order to reduce noise to the repo.

Unfortunately the way used to execute the step causes the version.dtsi
file to be reset too early and therefore causes an incorrect version
number to be used for the version macro when run locally. This went
unfortunately undiscovered as the checked in version.dtsi was the same
on the day the change in KinesisCorporation#376 was tested and was not noticed until
I build a new change to my keymap locally a few days ago.

The git command introduced in KinesisCorporation#376 is wrapped into a shell function.

However what was missed is that commands run by the shell function are run when
the function calls are expanded by make. This causes the version.dtsi file to be reset
before the firmware build process is even started which resultes in the version.dtsi
currently checked in to the repo to be used for local builds instead of the
newly generated file when make starts.

This change updates how the git command to reset the version.dtsi
is being called. It is no longer called within a shell function to ensure it
does run in the order it is defined after the firmware build is complete.

Builds run through GitHub actions are not impacted and always used the correct version.dtsi

For more information and context see:
  - https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
  - https://www.gnu.org/software/make/manual/html_node/Reading-Makefiles.html
gabrielfreiberg added a commit to gabrielfreiberg/Adv360-Pro-ZMK that referenced this pull request May 5, 2024
* upstream_V3.0:
  Changelog update (KinesisCorporation#448)
  Minor docs update (KinesisCorporation#445)
  Document new layer colors and modifier color configuration option (KinesisCorporation#431)
  Zephyr 3.5 Update (KinesisCorporation#426)
  Fix Makefile errors that prevent builds on macOS (KinesisCorporation#409)
  Fix version.dtsi is reset after local firmware build (KinesisCorporation#385)
  Update build workflows (KinesisCorporation#376)
gabrielfreiberg added a commit to gabrielfreiberg/Adv360-Pro-ZMK that referenced this pull request May 5, 2024
* main:
  Changelog update (KinesisCorporation#448)
  Minor docs update (KinesisCorporation#445)
  Document new layer colors and modifier color configuration option (KinesisCorporation#431)
  Zephyr 3.5 Update (KinesisCorporation#426)
  Fix Makefile errors that prevent builds on macOS (KinesisCorporation#409)
  Fix version.dtsi is reset after local firmware build (KinesisCorporation#385)
  Update build workflows (KinesisCorporation#376)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants