Message ID | cover.1700600804.git.martin.jansa@gmail.com |
---|---|
State | Not Applicable, archived |
Headers | show |
On Wed, 2023-11-22 at 13:44 +0100, Martin Jansa wrote: > This is the final part of changes for [YOCTO #12937]. > > I've run complete selftest with this and didn't see any failures. > > Only these 4 fail once, but pass when re-executed (and the same is > reproducible here with master): > pkgdata.OePkgdataUtilTests.test_lookup_recipe > spdx.SPDXCheck.test_spdx_base_files > esdk.oeSDKExtSelfTest.test_image_generation_binary_feeds > esdk.oeSDKExtSelfTest.test_install_libraries_headers > > runtime_test.TestImage.test_testimage_virgl_gtk_sdl and this one > needs extra "xhost +local" otherwise fails with: > runqemu - ERROR - Failed to run qemu: Invalid MIT-MAGIC-COOKIE-1 key > qemu-system-x86_64: OpenGL is not supported by the display > > The short description of these changes is that instead of symlinks > it creates hardlinks in deploy dir and the kernel do_deploy creates > the artifacts without version suffix and the do_deploy_links task > adds those versioned hardlinks (this way do_deploy can be reused from > sstate and only quick do_deploy_links is re-executed when the > IMAGE_VERSION_SUFFIX changes - before that if you cannot re-use do_deploy > from sstate due to different artifact filenames you had to re-run e.g. > do_compile as well if you haven't built the same in the same TMPDIR > before). I am a bit worried about this change since there were uses for having the symlinks present and this unconditionally moves everything over to hardlinks. With the symlink, you can see the pointer quite clearly, with hardlinks, it is unclear which files are duplicates of each other withouth diving into comparing inodes. Part of the reasoning was due to the way OE used to work where it would stack images, each build would add a new one and it would update the end symlink to point at the latest. Once sstate started removing old entries, that became less needed but the pointers still help runqemu and other tooling find the latest. This change is trying make the code do something different and it to change versioning and do that in a way which allows maximal reuse from sstate. Both are valid usages so we gain some things with the change but lose others. I'm not sure how users in general are going to find things overall :/. Cheers, Richard
On Wed, Nov 22, 2023 at 2:19 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Wed, 2023-11-22 at 13:44 +0100, Martin Jansa wrote: > > This is the final part of changes for [YOCTO #12937]. > > > > I've run complete selftest with this and didn't see any failures. > > > > Only these 4 fail once, but pass when re-executed (and the same is > > reproducible here with master): > > pkgdata.OePkgdataUtilTests.test_lookup_recipe > > spdx.SPDXCheck.test_spdx_base_files > > esdk.oeSDKExtSelfTest.test_image_generation_binary_feeds > > esdk.oeSDKExtSelfTest.test_install_libraries_headers > > > > runtime_test.TestImage.test_testimage_virgl_gtk_sdl and this one > > needs extra "xhost +local" otherwise fails with: > > runqemu - ERROR - Failed to run qemu: Invalid MIT-MAGIC-COOKIE-1 key > > qemu-system-x86_64: OpenGL is not supported by the display > > > > The short description of these changes is that instead of symlinks > > it creates hardlinks in deploy dir and the kernel do_deploy creates > > the artifacts without version suffix and the do_deploy_links task > > adds those versioned hardlinks (this way do_deploy can be reused from > > sstate and only quick do_deploy_links is re-executed when the > > IMAGE_VERSION_SUFFIX changes - before that if you cannot re-use do_deploy > > from sstate due to different artifact filenames you had to re-run e.g. > > do_compile as well if you haven't built the same in the same TMPDIR > > before). > > I am a bit worried about this change since there were uses for having > the symlinks present and this unconditionally moves everything over to > hardlinks. > > With the symlink, you can see the pointer quite clearly, with > hardlinks, it is unclear which files are duplicates of each other > withouth diving into comparing inodes. > Yes, it's definitely disadvantage of hardlinks (especially if someone forgets to preserve hardlinks when cp or rsync the deploy directory). But having the version in symlink would be even worse (as it could point to different artifact already). And having the version in the artifact itself requires do_deploy to re-run and without prior build it would re-run do_compile for kernel, bootloader and other artifacts as well. This is also why I've made sure you can set IMAGE_VERSION_SUFFIX to empty to prevent all of these hardlinks to be created, if all you care is just whatever is latest to be in the deploy directory. It might be interesting to have the versioned and version-less artifacts in different directories, so that you always cp/rsync only one set of them, but I fear that it would require even more oeqa changes and this area is already a bit too complicated I think. FWIW: we're using this for webOS builds since 2015 with webos_deploy task mentioned in the first patch, but to do this from "outside" is a bit difficult to maintain as webos_deploy needs to know about all possible artifacts other layers might create and also to inject dependency on webos_deploy task from all the right places. Thanks for review Richard, lets hope that someone else will also share an opinion about this. Cheers, Part of the reasoning was due to the way OE used to work where it would > stack images, each build would add a new one and it would update the > end symlink to point at the latest. Once sstate started removing old > entries, that became less needed but the pointers still help runqemu > and other tooling find the latest. > > This change is trying make the code do something different and it to > change versioning and do that in a way which allows maximal reuse from > sstate. > > Both are valid usages so we gain some things with the change but lose > others. I'm not sure how users in general are going to find things > overall :/. > > Cheers, > > Richard > > >