(mirror of https://github.com/vyos/vyos-live-build.git) https://git.amelek.net/vyos/vyos-live-build.git/atom?h=rolling 2020-05-02T13:49:45+00:00 2020-05-02T13:49:45+00:00 Lyndon Brown jnqnfe@gmail.com 2020-05-02T13:49:45+00:00 urn:sha1:2d7ff4934d721f38ac5274c866e44e20d3bda2e8 - added and used Stagefiles_dir() - added and used Stagefile_exists() pretty self explanatory. Gbp-Dch: Short 2020-04-29T09:27:16+00:00 Lyndon Brown jnqnfe@gmail.com 2020-04-29T09:26:49+00:00 urn:sha1:29ab2459c8e8f2287a968567dc6559f3f14eeb4e Gbp-Dch: Ignore 2020-04-23T17:26:15+00:00 Lyndon Brown jnqnfe@gmail.com 2020-04-02T19:23:57+00:00 urn:sha1:d79c96232b40fb082233c97ac8d4f75b821ecefe just as most scripts are skipped if their stagefile exists (indicating that they have already been run to completion), including chroot preparation scripts in install mode, this implements the same guard for chroot prep remove mode, such that they exit early if their stagefile does not exist, indicating that the modification has already been removed. (also override-able by --force in the same way). this basically just uses a tweaked copy of Check_stagefile(). Gbp-Dch: Short 2020-04-23T09:38:01+00:00 Lyndon Brown jnqnfe@gmail.com 2020-04-01T08:29:52+00:00 urn:sha1:11836c0c18b07f4bd1ea22a2163875975f3e6816 $@ when unquoted is subject to further word splitting. this fixes a bunch of instances where it was incorrectly being used unquoted. Gbp-Dch: Short 2020-03-18T08:37:22+00:00 Raphaël Hertzog hertzog@debian.org 2020-03-18T08:37:22+00:00 urn:sha1:bdffaebe12c3012b9eda7ab57551fdfe53a2e412 2020-03-17T22:59:37+00:00 Lyndon Brown jnqnfe@gmail.com 2020-03-15T13:05:44+00:00 urn:sha1:fb0790cc4379280084cc9a4469a515e243f45b9b this function takes one or more required stage fileS _plural_, and exits if any are missing (or at least it does now after the refactor). let's rename it to make things more clear Gbp-Dch: Short 2020-03-17T22:59:34+00:00 Lyndon Brown jnqnfe@gmail.com 2020-03-14T04:37:56+00:00 urn:sha1:fe0d2358b9777703effad9b29a1f54eeb65477f9 2020-03-17T22:57:09+00:00 Lyndon Brown jnqnfe@gmail.com 2020-03-14T04:33:23+00:00 urn:sha1:3bed138fda9b013c22cf9df685624d04e022bd95 the name of the stage is already printed earlier in the output prior to the error here being printed. so the error really does not need to include the script name itself. 2020-03-17T22:57:09+00:00 Lyndon Brown jnqnfe@gmail.com 2020-03-14T03:39:10+00:00 urn:sha1:ea0f6b7810e67cc5176b757db06dc3db8664c657 now having investigated my suspicions of the functionality and use of Require_stagefile(), i conclude that it has been fundamentally broken all the way back to v1.0~a8-1 (or at least usage of it since v1.0.1-2). gah. (╯°□°)╯︵ ┻━┻ ---- very early on in the history of live-build this function took the name of a _single_ stage file only and did `exit 1` should the file not be found. this was simple and clearly accomplished "what was on the tin", so to speak. in bd1a5ddc8203907eb40135303bea5488397ec5d0 (2007, 1.0~a8-1) things got weird. it was modified to support a list of multiple files. but instead of being written to cause failure if _any_ of the listed files were missing as perhaps one might expect, it was instead written to fail only if all files were missing! if you jump to the conclusion that i'm talking about a simple flipped logic from a lack or otherwise of a `!` here, you'd be mistaken; there is a comment inside the function that could not be more clear about what was intended by the author - "Find at least one of the required stages"! this makes me thoroughly confused about what they were thinking. as we'll get to, this was fundamentally flawed (or at least its later use was), but furthermore there were other notable issues introduced at this point (but don't worry too much about these, they've all been addressed): - `NAME` was modified in the loop, using the existing value, but nothing initially set it... - the setting of `NAME` seems related to its use in the subsequent error output, yet they are logically separated; it is only set if a file exists, while the error is only printed if none exist. - it is pointlessly using a messy `CONTINUE="true"` based mechanism, when it could just `return 0`. - it did not handle correctly the bad use case of no params having been supplied. it doesn't seem to have been entirely thought through, despite its pervasive use throughout the build system. note that no change was made in that commit to make actual use of the new multi-param support. it would not be used until about a year later. the function has remained largely untouched since then. in c68c0a270832ca340429878ce6a0ab606d435b06 a notable change was made to add an initial setting of `NAME`, which partially addressed one of the above issues. but it did not really address the issue the change was meant to solve, since the `NAME` as printed in the error was now the name of the script when what was really wanted was the name of the stagefile. this was finally fixed properly in d54990695f334d205fa846c42b6e0f2afd3c47f5. however the weirdly pointless setting of `NAME` persisted in the loop. finally i personally just refactored the function in the commit prior to this one, retaining the same functionality but addressing the remaining of the above minor implementation issues. looking at usage of the new functionality introduced in bd1a5ddc8203907eb40135303bea5488397ec5d0, it does not seem to have been until 0cbbde2b9664b9fafb311f1048db25ea69952222 (2008, almost a year after it was made possible) that changes were made to finally start making use of the ability to pass more than one filename at a time to the function, and it would appear that perhaps the author forgot what it actually was that the function accomplished when used with multiple params, and failed to double check. in this first use of multiple parameters, this commit went from passing single file names to individual calls to the function to passing the files in one single call, in a commit the purpose of which was described as simply tidying things up. it was most certainly not intended to change stage requirements. unfortunately, a change in requirements did occur as a result since the new usage of the function was not accomplishing the same as before. this change completely broke the stage requirements protection mechanism such that only a single one of the listed stages needed to have completed for the check to pass, rather than all as expected. this flaw made it into release v1.0.1-2 and it has existed every since. in the very next commit from that one, 6204dc0e6db02859a07a978d87f1a5231c0214cf things got even worse. here we see the config stage being specified commonly as the first stage listed, which is still the case today. this means that ever since this commit, if you've already got a config before building (which you inevitably do, especially after some later commits introduced automatically creating it if missing), then all other stage requirements are simply ignored. so it seems pretty damn clear that this function is accomplishing completely the wrong objective. it _should_ be checking that _all_ files given to it exist, not just one or more. ¯\_(ツ)_/¯ this FINALLY addresses this mistake. (not that i wish to berate the author; i've made silly mistakes of my own before) 2020-03-17T22:57:04+00:00 Lyndon Brown jnqnfe@gmail.com 2020-03-12T20:12:47+00:00 urn:sha1:1b09b1527744a50baf4e0cd45aacced461e2d862 - count of params is available as $#, we don't need the pipe-to-wc logic. - the whole 'CONTINUE' based logic is silly, we can just return once one of the files is found. - setting of 'NAME' in the loop was completely pointless. - the error message for multiple files was not very clear just injecting a sequence of words into a sentence. - it did not work correctly if no arguments were given (bad usage) note, you might question whether the functionality of this function is correct, as did I; this is tackled in a followup commit whilst this commit retains the existing functionality! Gbp-Dch: Short