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

build: fix py versions in buildroot builds #12235

Merged
merged 2 commits into from
Mar 6, 2023

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Mar 6, 2023

The python version numbers are now generated from git tags. That means you have to run git commands with a cwd inside the git directory. While building on buildroot, this was not happening because when buildroot builds out-of-tree projects it does so after first rsync'ing their contents (i.e. not the .git directory) into the build directory.

That means that when we called the version machinery in the buildroot make scripts to do things like create /etc/VERSION.json everything worked, because those makefiles explicitly ran the script in the external tree; but when buildroot called setup.py build, that happened in the build directory which is inside the buildroot directory and therefore the scripts would pick up the latest tag from buildroot, since that tag scheme matches the robot-stack tag scheme.

The way to fix this is to make sure the version runs inside the source directory when building on buildroot. This could be done a couple ways; in my opinion, this is a nice combination of minimally altering the python code that is reused outside of buildroot while still keeping things obvious: adding an optional environment variable respected by the setup.pys (rather than the build script, which would make the environment variable hard to find) that can change the location of the git calls.

Testing

This PR will be built by buildroot. We need to make sure that the output of that build has the correct wheel versions.

  • Locally build and check the wheel versions in a mount of the system partition
  • Download the CI build and check the wheel versions in a mount of the system partition
  • Install the CI build on a robot and check that it works in practice

Risks

Minimal if the above all works. Flex doesn't use the same processes so it should be unaffected.

The python version numbers are now generated from git tags. That means
you have to run git commands with a cwd inside the git directory. While
building on buildroot, this was not happening because when buildroot
builds out-of-tree projects it does so after first rsync'ing their
contents (i.e. _not_ the .git directory) into the build directory.

That means that when we called the version machinery in the buildroot
make scripts to do things like create /etc/VERSION.json everything
worked, because those makefiles explicitly ran the script in the
external tree; but when buildroot called setup.py build, that happened
in the build directory which is inside the buildroot directory and
therefore the scripts would pick up the latest tag _from buildroot_,
since that tag scheme matches the robot-stack tag scheme.

The way to fix this is to make sure the version runs inside the
source directory when building on buildroot. This could be done a couple
ways; in my opinion, this is a nice combination of minimally altering
the python code that is reused outside of buildroot while still keeping
things obvious: adding an optional environment variable respected by the
setup.pys (rather than the build script, which would make the
environment variable hard to find) that can change the location of the
git calls.
@sfoster1 sfoster1 requested review from a team as code owners March 6, 2023 17:40
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #12235 (55a4dbb) into release_6.3.0 (c1cb13f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           release_6.3.0   #12235   +/-   ##
==============================================
  Coverage          74.01%   74.01%           
==============================================
  Files               2193     2193           
  Lines              60360    60360           
  Branches            6223     6223           
==============================================
  Hits               44676    44676           
  Misses             14254    14254           
  Partials            1430     1430           
Flag Coverage Δ
app 72.60% <ø> (ø)
g-code-testing 97.19% <ø> (ø)
hardware 59.93% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 46.28% <ø> (ø)
shared-data 75.90% <ø> (ø)
step-generation 88.46% <ø> (ø)
system-server 94.75% <ø> (ø)
update-server 65.65% <ø> (ø)
usb-bridge 81.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

1 similar comment
Copy link
Collaborator

@y3rsh y3rsh left a comment

Choose a reason for hiding this comment

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

Installed on my robot and
image
image

@sfoster1 sfoster1 merged commit d190888 into release_6.3.0 Mar 6, 2023
@sfoster1 sfoster1 deleted the build-fix-wheel-versions branch March 6, 2023 19:56
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

3 participants