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

Move DTS scripts to another repo #115

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Move DTS scripts to another repo #115

merged 3 commits into from
Apr 10, 2024

Conversation

DaniilKl
Copy link
Contributor

@DaniilKl DaniilKl commented Apr 8, 2024

Move all DTS scripts to another repo and add as git package. Resolves issue Dasharo/dasharo-issues#773.

@DaniilKl DaniilKl requested review from macpijan and TomaszAIR April 8, 2024 09:56
@DaniilKl DaniilKl self-assigned this Apr 8, 2024
@DaniilKl DaniilKl marked this pull request as draft April 8, 2024 09:56
@DaniilKl DaniilKl force-pushed the scripts-to-sep-repo branch from 5c84fc7 to bd08ddd Compare April 8, 2024 10:08
@DaniilKl DaniilKl linked an issue Apr 8, 2024 that may be closed by this pull request
@DaniilKl
Copy link
Contributor Author

DaniilKl commented Apr 8, 2024

Verification results after building and flashing:

λ sudo find ./ -type f | grep "dts-functions.sh\|dts-environment.sh\|dasharo-hcl-report\|touchpad-info\|cloud_list\|dasharo-deploy\|dts\|dts-boot\|dts-profile.sh\|ec_transition"
./usr/share/vim/vim82/syntax/dts.vim
./usr/share/mime/audio/vnd.dts.hd.xml
./usr/share/mime/audio/vnd.dts.xml
./usr/sbin/dasharo-deploy
./usr/sbin/dts-functions.sh
./usr/sbin/dts-environment.sh
./usr/sbin/touchpad-info
./usr/sbin/ec_transition
./usr/sbin/dts-boot
./usr/sbin/dasharo-hcl-report
./usr/sbin/cloud_list
./usr/sbin/dts
./etc/profile.d/dts-profile.sh

Note, for verification branch in:

meta-dts-distro/recipes-dts/dts-scripts/dts-scripts_git.bb:SRC_URI = "git://github.com/Dasharo/dts-v2;protocol=https;branch=main"

should be changed to yocto-cleanup or PR Dasharo/dts-scripts#1 should be merged.

@DaniilKl DaniilKl force-pushed the scripts-to-sep-repo branch from bd08ddd to 16fb9a8 Compare April 8, 2024 10:50
@DaniilKl DaniilKl marked this pull request as ready for review April 8, 2024 10:53
Copy link
Contributor

@macpijan macpijan left a comment

Choose a reason for hiding this comment

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

Overall looks good. Have you managed to test it somehow? At least on QEMU?

"

do_install () {
install -d ${D}/${sbindir}
Copy link
Contributor

Choose a reason for hiding this comment

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

You may consider adding Makefile to the dts-v2 repo, and simply call oe_runmake install 'DESTDIR=${D}' here. That would make it easier to install these script for testing as well, in some test environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose this Makefile will only have two rules: to install these scripts in sbin and to remove it from sbin?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only install would do as well. It should do the same as in the recipe, simply inside a Makefile, which makes it more self-contained (closer to the script), so the recipe does not need to change the installation methods when we change the file paths for instance (and I am sure we will do that).

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not have an example on hand, but I am sure you will find plenty. The one I recently changed was: https://github.com/Dasharo/osfv-scripts/blob/main/osfv_cli/Makefile#L10

The DESTDIR is a standard variable: https://www.gnu.org/prep/standards/html_node/DESTDIR.html

It can be defined in Makefile as some default, but with the ?=, so it can be overriden in Yocto recipe

Copy link
Contributor Author

@DaniilKl DaniilKl Apr 9, 2024

Choose a reason for hiding this comment

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

I have been experimenting for a while with Makefile which I have added in f8afaca3873821d13a9db79c944302d7ff9df259 and it seems that it is not working as I expected with Yocto.

With following do_install:

do_install () {
    install -d ${D}${sbindir}
    install -d ${D}${sysconfdir}/profile.d
    oe_runmake install 'DESTDIR=${D}${sbindir} SYSCONFDIR=${D}${sysconfdir}/profile.d/'
}

I am getting following error during build:

ERROR: dts-scripts-0.1+gitAUTOINC+f8afaca387-r0 do_compile: oe_runmake failed
ERROR: dts-scripts-0.1+gitAUTOINC+f8afaca387-r0 do_compile: ExecutionError('/build/tmp/work/core2-64-dts-linux/dts-scripts/0.1+gitAUTOINC+f8afaca387-r0/temp/run.do_compile.229', 1, None, None)
ERROR: Logfile of failure stored in: /build/tmp/work/core2-64-dts-linux/dts-scripts/0.1+gitAUTOINC+f8afaca387-r0/temp/log.do_compile.229
Log data follows:
| DEBUG: Executing shell function do_compile
| NOTE: make -j 8
| install -D -m 0755 $(find ./ -type f -name dts-profile.sh) //dts-profile.sh
| install -D -m 0655 $(find ./ -type f -name dts-environment.sh) /dts-environment.sh
| install -D -m 0655 $(find ./ -type f -name dts-functions.sh) /dts-functions.sh
| install -D -m 0755 $(find ./ -type f -name cloud_list) /cloud_list
| install -D -m 0755 $(find ./ -type f -name dasharo-deploy) /dasharo-deploy
| install -D -m 0755 $(find ./ -type f -name dts) /dts
| install -D -m 0755 $(find ./ -type f -name dts-boot) /dts-boot
| install -D -m 0755 $(find ./ -type f -name ec_transition) /ec_transition
| install: cannot create regular file '//dts-profile.sh': Permission denied
| install: cannot create regular file '/dts-functions.sh'install: cannot create regular file '/cloud_list': Permission denied
| install: cannot create regular file '/dts-environment.sh': Permission denied
| : Permission denied
| make: *** [Makefile:14: //dts-profile.sh] Error 1
| make: *** Waiting for unfinished jobs....
| make: *** [Makefile:17: /dts-environment.sh] Error 1
| make: *** [Makefile:17: /dts-functions.sh] Error 1
| install: cannot create regular file '/dasharo-deploy'make: *** [Makefile:20: /cloud_list] Error 1
| : Permission denied
| install: cannot create regular file '/dts': Permission denied
| make: *** [Makefile:20: /dasharo-deploy] Error 1
| make: *** [Makefile:20: /dts] Error 1
| install: cannot create regular file '/dts-boot': Permission denied
| make: *** [Makefile:20: /dts-boot] Error 1
| install: cannot create regular file '/ec_transition': Permission denied
| make: *** [Makefile:20: /ec_transition] Error 1
| ERROR: oe_runmake failed
| WARNING: exit code 1 from a shell command.
ERROR: Task (/repo/meta-dts-distro/recipes-dts/dts-scripts/dts-scripts_git.bb:do_compile) failed with exit code '1'
NOTE: Tasks Summary: Attempted 4171 tasks of which 4170 didn't need to be rerun and 1 failed.

Summary: 1 task failed:
  /repo/meta-dts-distro/recipes-dts/dts-scripts/dts-scripts_git.bb:do_compile
Summary: There were 2 ERROR messages, returning a non-zero exit code.
2024-04-09 09:38:20 - ERROR    - Command returned non-zero exit status 1

I did not add any modifications to do_compile, so the warning seems strange.

After some digging I have found the answer: If you start using oe_runmake somewhere in a recipe - Yocto automatically adds oe_runmake to do_config and do_compile in poky/meta/classes/base.bbclass:

addtask configure after do_patch
do_configure[dirs] = "${B}"
base_do_configure() {
	if [ -n "${CONFIGURESTAMPFILE}" -a -e "${CONFIGURESTAMPFILE}" ]; then
		if [ "`cat ${CONFIGURESTAMPFILE}`" != "${BB_TASKHASH}" ]; then
			cd ${B}
			if [ "${CLEANBROKEN}" != "1" -a \( -e Makefile -o -e makefile -o -e GNUmakefile \) ]; then
				oe_runmake clean
			fi
			# -ignore_readdir_race does not work correctly with -delete;
			# use xargs to avoid spurious build failures
			find ${B} -ignore_readdir_race -name \*.la -type f -print0 | xargs -0 rm -f
		fi
	fi
	if [ -n "${CONFIGURESTAMPFILE}" ]; then
		mkdir -p `dirname ${CONFIGURESTAMPFILE}`
		echo ${BB_TASKHASH} > ${CONFIGURESTAMPFILE}
	fi
}

addtask compile after do_configure
do_compile[dirs] = "${B}"
base_do_compile() {
	if [ -e Makefile -o -e makefile -o -e GNUmakefile ]; then
		oe_runmake || die "make failed"
	else
		bbnote "nothing to compile"
	fi
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I think we should stay without oe_runmake in recipe but with Makefile in Dasharo/dts-v2.

Copy link
Contributor

@macpijan macpijan Apr 9, 2024

Choose a reason for hiding this comment

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

After some digging I have found the answer: If you start using oe_runmake somewhere in a recipe - Yocto automatically adds oe_runmake to do_config and do_compile in poky/meta/classes/base.bbclass:

If this is the only blocker, simply use do_configure[noexec] = "1" for the stage you wish not to execute. I have not looked into detailes of makefile yet, please let me know if I need to at this point.

Copy link
Contributor

@macpijan macpijan Apr 9, 2024

Choose a reason for hiding this comment

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

The best way to learn what is possible (and how it should be done) is to look at existing recipes in other layers. Some random example: https://git.yoctoproject.org/poky/plain/meta/recipes-support/libjitterentropy/libjitterentropy_3.4.1.bb

Please take a look at some, and adjust to how the oe_runmake install is used typically.

Copy link
Contributor Author

@DaniilKl DaniilKl Apr 9, 2024

Choose a reason for hiding this comment

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

Thanks for the tip, added installing via Makefile here d10b32b.

@DaniilKl
Copy link
Contributor Author

DaniilKl commented Apr 8, 2024

Have you managed to test it somehow? At least on QEMU?

@macpijan, I have not tested it by running, only compiling-flashing-checking if present on FS that is demonstrated here. Is there any reason for testing it by running? I have not changed the scripts.

@macpijan
Copy link
Contributor

macpijan commented Apr 8, 2024

I have not tested it by running, only compiling-flashing-checking if present on FS that is demonstrated #115 (comment). Is there any reason for testing it by running? I have not changed the scripts.

If the md5sum / sha256sum of each file is the same as before, and they are located in the same path, that should be enough indeed.

@DaniilKl
Copy link
Contributor Author

DaniilKl commented Apr 8, 2024

If the md5sum / sha256sum of each file is the same as before, and they are located in the same path, that should be enough indeed.

Ok, I confess that I have changed touchpad_info to add a description from its former recipe, so checksum of this file will differ. You can verify it by diff.

@macpijan
Copy link
Contributor

macpijan commented Apr 8, 2024

Ok, I confess that I have changed touchpad_info to add a description from its former recipe, so checksum of this file will differ. You can verify it by diff.

If you prove there is no diff between other files (meta-dts vs dts-v2), and the only diff is some harmless comment, then it is good enough. Right now you have proven the paths are the same, which is good, but not enough.

Move all DTS scripts to another repo and add as git package.

Signed-off-by: Daniil Klimuk <[email protected]>
@DaniilKl DaniilKl force-pushed the scripts-to-sep-repo branch from d10b32b to 795dfc9 Compare April 9, 2024 11:02
@DaniilKl DaniilKl changed the base branch from main to develop April 9, 2024 11:03
@DaniilKl
Copy link
Contributor Author

DaniilKl commented Apr 9, 2024

@macpijan, verification via checksums and diff:

Here is checksums from Dasharo/dts-v2 from commit a74973ce4c91807199f5697e4c7855b4d9c654d0:

91bff8c47334b3ba845715fd56ed6ee7761d3976b0cfa96852b68455fba24836  dts-profile.sh
14819b6b3e82fccf74c146b40e4700e0c4c507d70833ccec3467499591f4f08d  include/dts-environment.sh
d47053f5b9b4b426c1b0b7c852f9e0a272a26d4203c808ef6fee2680bdee0417  include/dts-functions.sh
ac25eeb584ef34e232b170032166ded51a251abf920c930a7c4c1027756e8057  scripts/cloud_list
8c26a1f4e3dc9557bd6d7282e593f0651817caee678ee4ff7af615ba1eaaf355  scripts/dasharo-deploy
f9bc5207089282170f0bc9f18a418a40a6b5419ddc353bf8ea007d3e5fb0e044  scripts/dts
c53078e43ec99ea7af8b613a1143c4a1f5a296db6b1a9f833be2caa28d1c6302  scripts/dts-boot
3e6688795277f7b34ca646bb4354a4208b407e8f7f64bfe5d533e3436939f12b  scripts/ec_transition
c72db53d8d80a987d311d597946cbe4c08d9d351ac9c5c05e2c553f762c39ec5  reports/dasharo-hcl-report
c27525e3fb6a81ae0902e57fa85eaf01c66d4a7ae4e0d7a62bd23d3817d17539  reports/touchpad-info

And from Dasharo/meta-dts from commit 4961fcdb64bbcad1a527b97b2212c09c385dc3e9:

91bff8c47334b3ba845715fd56ed6ee7761d3976b0cfa96852b68455fba24836  dts/dts/dts-profile.sh
14819b6b3e82fccf74c146b40e4700e0c4c507d70833ccec3467499591f4f08d  dts/dts/dts-environment.sh
d47053f5b9b4b426c1b0b7c852f9e0a272a26d4203c808ef6fee2680bdee0417  dts/dts/dts-functions.sh
ac25eeb584ef34e232b170032166ded51a251abf920c930a7c4c1027756e8057  dts/dts/cloud_list
8c26a1f4e3dc9557bd6d7282e593f0651817caee678ee4ff7af615ba1eaaf355  dts/dasharo-deploy/dasharo-deploy
f9bc5207089282170f0bc9f18a418a40a6b5419ddc353bf8ea007d3e5fb0e044  dts/dts/dts
c53078e43ec99ea7af8b613a1143c4a1f5a296db6b1a9f833be2caa28d1c6302  dts/dts/dts-boot
3e6688795277f7b34ca646bb4354a4208b407e8f7f64bfe5d533e3436939f12b  dts/dts/ec_transition
c72db53d8d80a987d311d597946cbe4c08d9d351ac9c5c05e2c553f762c39ec5  reports/dasharo-hcl-report/dasharo-hcl-report
14bed8e2e27cb94aa70de63de173c3a560137b1b4ed7148ba8e90b61106c7258  reports/touchpad-info/touchpad-info

And only one difference in touchpad-info:

λ diff reports/touchpad-info ../meta-dts/meta-dts-distro/recipes-dts/reports/touchpad-info/touchpad-info 
2,3c2
< # A script to get information on the touchpad devices. Currently supports only
< # Clevo devices.
---
> # A script to dump info about the touchpad

@DaniilKl DaniilKl force-pushed the scripts-to-sep-repo branch from 795dfc9 to 9d3125e Compare April 10, 2024 06:47
@macpijan
Copy link
Contributor

@macpijan, verification via checksums and diff:

Thanks, @DaniilKl , that should do it.

@macpijan macpijan merged commit 008f30b into develop Apr 10, 2024
@macpijan macpijan deleted the scripts-to-sep-repo branch April 10, 2024 09:27
@DaniilKl
Copy link
Contributor Author

@macpijan, why have you changed the license?

@macpijan
Copy link
Contributor

@DaniilKl Apache-2.0 is our first choice for own projects. Both are permissive, but Apache-2.0 for instance provide some trademark protection, which is better especially for Dasharo projects: https://www.tldrlegal.com/license/apache-license-2-0-apache-2-0

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.

Move DTS to a separate repository
2 participants