Skip to content

Allow to specify the TinyPilot version identifier#1871

Merged
jotaen4tinypilot merged 5 commits intomasterfrom
debian-pkg-params
Mar 17, 2025
Merged

Allow to specify the TinyPilot version identifier#1871
jotaen4tinypilot merged 5 commits intomasterfrom
debian-pkg-params

Conversation

@jotaen4tinypilot
Copy link
Copy Markdown
Contributor

@jotaen4tinypilot jotaen4tinypilot commented Mar 14, 2025

Follow-up of https://github.com/tiny-pilot/tinypilot-pro/pull/1474, specifically https://github.com/tiny-pilot/tinypilot-pro/pull/1474#issuecomment-2725155594.

This PR modifies the interface of the build-debian-pkg dev script so that it takes on multiple (optional) named flags, instead of just one (optional) position argument:

  • The --build-targets flag replaces the previous positional input argument.
  • The newly added --tinypilot-version now allows to set the version of the Debian package.
    • That way we are independent from the ambiguous behaviour of the git describe --tags command. We can still use it as implicit default, though. In Pro, we can then explicitly pass in the version identifier from the CircleCI build context – without, however, by directly making the build-debian-pkg script depend on the CircleCI context, e.g. by referencing the CIRCLE_TAG environment variable inside of build-debian-pkg.

When inspecting the parameters of the actual build, the changes appear to work fine:

Review on CodeApprove

Copy link
Copy Markdown
Contributor

@db39 db39 left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved on CodeApprove
✔️ Approved

LGTM!


👀 @jotaen4tinypilot it's your turn please take a look

@jotaen4tinypilot jotaen4tinypilot merged commit 41e4dea into master Mar 17, 2025
@jotaen4tinypilot jotaen4tinypilot deleted the debian-pkg-params branch March 17, 2025 20:26
@jotaen4tinypilot
Copy link
Copy Markdown
Contributor Author

(Merging regardless of failing build, since it’s “just” our license URL test being flaky again.)

jotaen4tinypilot added a commit that referenced this pull request Mar 26, 2025
Follow up of #1871.

Git short hashes can be longer than 7 characters, see the [“Short SHA-1”
section of the git
docs](https://git-scm.com/book/en/v2/Git-Tools-Revision-Selection).

That’s relevant in our Pro repo, as you can [see in our latest
build](https://app.circleci.com/pipelines/github/tiny-pilot/tinypilot-pro/4799/workflows/4ed6c793-a8f1-458e-b29c-434005e5ae0f/jobs/50173),
where the output of `git describe --tags --long` yields
`2.7.0-rc.4-5-g0bfe1e1f`, i.e. with 8 characters following the `-g`.
Therefore, the regex replacement didn’t match.

Contrary to [my assumption about config
protection](tiny-pilot/tinypilot-pro#1475 (comment))
([which wasn’t
correct](tiny-pilot/tinypilot-pro#1475 (comment))),
this seems to be the actual reason why the change didn’t have any effect
in the PR build.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1872"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

Co-authored-by: Jan Heuermann <jan@jotaen.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants