Skip to content

Add version to the binary during build #12

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 13, 2022
Merged

Add version to the binary during build #12

merged 2 commits into from
May 13, 2022

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented May 12, 2022

Replace the string in the patch overriding the version, This way when running imgtool version the correct version is reported.

@umbynos umbynos added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure topic: code Related to content of the project itself labels May 12, 2022
@umbynos umbynos requested a review from per1234 May 12, 2022 15:08
@umbynos umbynos self-assigned this May 12, 2022
@umbynos
Copy link
Contributor Author

umbynos commented May 12, 2022

Example of the problem can be seen here #11 (comment)

per1234
per1234 reviewed May 13, 2022
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using an obvious placeholder in the patch to make the system more clear?

Like changing the current patch:

imgtool-packing/patches/0008-Imgtool-Append-arduino-to-version-string.patch

Lines 18 to 19 in 5619cae

-imgtool_version = "1.8.0"
+imgtool_version = "1.8.0-arduino"

To something like this:

-imgtool_version = "1.8.0"
+imgtool_version = "ARDUINO_VERSION_PLACEHOLDER"

@umbynos umbynos force-pushed the umbynos/version branch from 5619cae to 2fd1807 Compare May 13, 2022 07:11
per1234
per1234 requested changes May 13, 2022
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot I was going to make one follow-up suggestion if the placeholder idea was accepted:

Update the patch message to reflect the new nature of the patch:

imgtool-packing/patches/0008-Imgtool-Append-arduino-to-version-string.patch

Line 4 in 2fd1807

Subject: [PATCH] Imgtool: Append -arduino to version string

@umbynos umbynos force-pushed the umbynos/version branch 2 times, most recently from 54dfa22 to 0c82b4f Compare May 13, 2022 08:54
@umbynos
Copy link
Contributor Author

umbynos commented May 13, 2022

Test release here https://github.com/arduino/imgtool-packing/releases/tag/1.8.0-rc7

@umbynos
Copy link
Contributor Author

umbynos commented May 13, 2022 

❰umberto❙~/Downloads/imgtool_1.8.0-rc7_Linux_64bit❱✔≻ ./imgtool version                                                              11:11:49
1.8.0-rc7

@umbynos umbynos requested a review from per1234 May 13, 2022 09:12
per1234
per1234 requested changes May 13, 2022
View reviewed changes
@umbynos umbynos force-pushed the umbynos/version branch from 0c82b4f to baafbef Compare May 13, 2022 14:16
@umbynos umbynos merged commit 7976506 into main May 13, 2022
@umbynos umbynos deleted the umbynos/version branch May 17, 2022 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 per1234 approved these changes

Assignees

@umbynos umbynos

Labels
topic: code Related to content of the project itself topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@umbynos @per1234