On Mon, Nov 1, 2021 at 12:19 PM, Richard Purdie wrote:<br />
<blockquote>On Mon, 2021-11-01 at 04:09 -0700, Manuel Leonhardt wrote:<br />
<blockquote>On Mon, Nov 1, 2021 at 11:52 AM, Richard Purdie wrote:<br />
<blockquote>On Mon, 2021-11-01 at 03:47 -0700, Manuel Leonhardt wrote:<br />
<blockquote>I submitted a patch to fix bb.utils.lockfile to at least break the loop<br />and<br />fail<br />when a too long filename is passed with<br /><a href="https://lists.openembedded.org/g/bitbake-devel/message/12850" target="_blank" rel="noopener">https://lists.openembedded.org/g/bitbake-devel/message/12850</a><br /><br />Nevertheless, you have a good point here: Fixing the code that constructs<br />the<br />filename of the lock files would be the better solution. For the sstate<br />files<br />only lockfiles of .siginfo files can overlap, because the filename of the<br />actual<br />sstate file is already 8 characters shorter than 255 characters. I have<br />one<br />though on that:<br /><br />From what I have seen, the filename of lockfiles is mostly constructed by<br />appending ".lock" without any further check. Truncating the filename of<br />the<br />lockfile inside bb.utils.lockfile could be confusing however, since the<br />function<br />would not use the filename that is was explicitly passed. Hence, I guess<br />bb.utils.lockfile should fail when a too long filename was passed, and the<br />function building the lockfile's filename should ensure to keep it<br />limited;<br />that's bb.fetch2.FetchData.__init__ then if I'm not mistaken. Would you<br />agree?<br />Or would you say, changing the filename inside bb.utils.lockfile is ok, as<br />long<br />as the function is working?<br /><br />The drive-by fix I made with using limit instead of 254 when shortening<br />the<br />filename is still correct and necessary, right?</blockquote>
I think we just let lockfile truncate the filename if necessary since this<br />could<br />affect other users of the funciton too.&nbsp;<br /><br />Looking at the code you changed with the limit value it isn't correct since<br />extension is already accounted for in avail (although I agree that code is<br />confusing and I had to think twice about it).<br /><br />Cheers,<br /><br />Richard</blockquote>
Ok, I'll submit a new patch.<br /><br />For the limit value, my guess was that when always using 254 the reserved<br />characters for siginfo are not taken into account when siginfo=False. For<br />shorter filenames it's ensured that the siginfo file is always the sstate<br />file's<br />name plus ".siginfo". With longer filenames it's possible that the basename of<br />both files is different. This comes unhandy when working with the files<br />outside<br />of bitbake, e.g. running maintenance tasks on the sstate mirror server, e.g.<br />copy sstate files and ensure their respective .siginfo file is also copied.</blockquote>
That is reasonable, the commit message just needs to say that is why it is being<br />changed in that case.<br /><br />Cheers,<br /><br />Richard</blockquote>
Thanks. You ware totally right, I missed that in the commit message. I'll submit a new patch for that as well.<br /><br />Best,<br /><br />Manuel