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 dts-check command and use Pip for some Python packages instead of APT #6799

Merged
merged 6 commits into from
Jun 25, 2024

Conversation

ColorfulRhino
Copy link
Collaborator

@ColorfulRhino ColorfulRhino commented Jun 25, 2024

Description

Please see the commit messages for details on each change.

Fixes: https://paste.armbian.com/ezuliwofij.bash

This will also make sure that build hosts don't use inconsistent versions for python3-setuptools and python3-pyelftools. These are vastly different if you compare these packages on e.g. Ubuntu Jammy and Ubuntu Noble.
Pip packages will automatically be updated via Dependabot.

In addition, this change will improve compatibility with other build host OSs since Pip is independant.

In total, these Python packages are not needed anymore on the build host/do not need to be installed:

python3-automat python3-bcrypt python3-click python3-colorama python3-constantly python3-hamcrest python3-hyperlink python3-incremental python3-pyasn1 python3-pyasn1-modules python3-pycurl python3-service-identity python3-twisted python3-wheel python3-zope.interface python3-setuptools python3-pyelftools

--> Much smaller APT package footprint ✅

How Has This Been Tested?

  • ./compile.sh dts-check BOARD=nanopi-r6c BRANCH=edge BUILD_DESKTOP=no BUILD_MINIMAL=yes EXPERT=yes KERNEL_CONFIGURE=no RELEASE=trixie while using no-cache-dir with pip, build host Ubuntu Noble

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

This way the dts-check command works even when no changes to the kernel were made.
…ments.txt

Different build hosts have vastly different versions of setuptools and pyelftools depending on the host OS, e.g. Ubuntu 22.04 has setuptools v59 while the latest version at the time of this commit is setuptools v71.

Using Pip instead of APT to download these packages assures that all build hosts use the same version, removing some points of failures and inconsistencies.
@ColorfulRhino ColorfulRhino requested review from a team and igorpecovnik as code owners June 25, 2024 14:12
@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines Framework Framework components labels Jun 25, 2024
Instead of having to add every new release to the list that does not support python2, inverse this if statement and only check if the build host is Debian bullseye or ubuntu jammy. Every release newer than those do not have python2. Older build hosts are unsupported.
ColorfulRhino referenced this pull request Jun 25, 2024
This makes dependencies easier to track and opens up the possibility for Dependabot to update them.
@igorpecovnik igorpecovnik added Ready to merge Reviewed, tested and ready for merge 08 Milestone: Third quarter release labels Jun 25, 2024
Copy link
Member

@igorpecovnik igorpecovnik left a comment

Choose a reason for hiding this comment

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

Builds also on Jammy, looks fine.

@ColorfulRhino
Copy link
Collaborator Author

Builds also on Jammy, looks fine.

Thanks for testing 👍

@igorpecovnik igorpecovnik merged commit d75b181 into armbian:main Jun 25, 2024
11 checks passed
@ColorfulRhino ColorfulRhino deleted the dts-check-fix5 branch June 25, 2024 17:55
@rpardini
Copy link
Member

@ColorfulRhino breakage here, I think. u-boot's requiring elftools are not finding it. https://paste.next.armbian.com/busaleqife

@rpardini
Copy link
Member

Yeah. Partially reverting this fixes it; I think some ENVs will need to be set for the u-boot-called Python to find the requirements.txt-installed dependencies.

Subject: [PATCH] (partially) Revert "python: Move `python3-setuptools` and `python3-pyelftools` to requirements.txt"
diff --git a/lib/functions/host/prepare-host.sh b/lib/functions/host/prepare-host.sh
--- a/lib/functions/host/prepare-host.sh	(revision 8214d9d9d227591d0a15f8d24932e3db8400bfab)
+++ b/lib/functions/host/prepare-host.sh	(revision d55f2ed9fb955ad4fac4fc721ca75e7b42e2075d)
@@ -290,8 +290,7 @@
 	host_deps_add_extra_python # See python-tools.sh::host_deps_add_extra_python()
 
 	### Python3 -- required for Armbian's Python tooling, and also for more recent u-boot builds. Needs 3.9+; ffi-dev is needed for some Python packages when the wheel is not prebuilt
-	### 'python3-setuptools' and 'python3-pyelftools' moved to requirements.txt to make sure build hosts use the same/latest versions of these tools.
-	host_dependencies+=("python3-dev" "python3-pip" "libffi-dev")
+	host_dependencies+=("python3-dev" "python3-setuptools" "python3-pip" "python3-pyelftools" "libffi-dev")
 
 	# Needed for some u-boot's, lest "tools/mkeficapsule.c:21:10: fatal error: gnutls/gnutls.h"
 	host_dependencies+=("libgnutls28-dev")

@ColorfulRhino

rpardini added a commit to armsurvivors/armbian-build that referenced this pull request Jun 28, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
rpardini added a commit to rpardini/armbian-build that referenced this pull request Jun 28, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
@rpardini
Copy link
Member

Send this partial revert -- it brings back the hostdeps, while keeping the requirements.txt -- so that @ColorfulRhino can calmly fix it. #6818

rpardini added a commit to armsurvivors/armbian-build that referenced this pull request Jun 28, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
@ColorfulRhino
Copy link
Collaborator Author

Send this partial revert -- it brings back the hostdeps, while keeping the requirements.txt -- so that @ColorfulRhino can calmly fix it. #6818

Yes, calmly is a good idea 😌

@ColorfulRhino
Copy link
Collaborator Author

@rpardini I think wherever U-Boot is built just needs the same treatment as make, including the Python packages in the PATH,. since Armbian build script does not use the systems usual Python path to install pip packages.

See here what changed for make: https://github.com/armbian/build/pull/6739/files#diff-4347825abd7145eafaff34b8c24e1f33bf3938cb89ecda7347294ca1b0a99ac7

@rpardini
Copy link
Member

Yeah, might be PYTHONPATH solves it.

@ColorfulRhino
Copy link
Collaborator Author

ColorfulRhino commented Jun 28, 2024

Yeah, might be PYTHONPATH solves it.

Yes, it's very likely. Do you know exactly where this could be inserted? I need to find the place

Maybe somewhere here?

gcc_version_main="$(eval env PATH="${toolchain}:${toolchain2}:${PATH}" "${UBOOT_COMPILER}gcc" -dumpfullversion -dumpversion)"

@ColorfulRhino
Copy link
Collaborator Author

Maybe https://github.com/armbian/build/blob/main/lib/functions/compilation/uboot.sh needs a little makeover like kernel-make.sh to set the ENVs once, because in the u-boot file the envs are all over the place so it's difficult to know where to insert the PATH and/or PYTHONPATH 😅

rpardini added a commit to armsurvivors/armbian-build that referenced this pull request Jun 29, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
rpardini added a commit to armsurvivors/armbian-build that referenced this pull request Jun 29, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
rpardini added a commit to armsurvivors/armbian-build that referenced this pull request Jun 29, 2024
…elftools` to requirements.txt"

- "Fixes" armbian#6799

This (partially) reverts commit 04f619d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08 Milestone: Third quarter release Framework Framework components Ready to merge Reviewed, tested and ready for merge size/medium PR with more then 50 and less then 250 lines
Development

Successfully merging this pull request may close these issues.

None yet

3 participants