| Message ID | 20250627091604.3275870-1-richard.purdie@linuxfoundation.org |
|---|---|
| State | Accepted, archived |
| Commit | d89e30fb2fb15b09f2cb95c4e5aa9f749ca257ea |
| Headers | show |
| Series | utils: Refactor filemode variable conversion to a function | expand |
Hi Richard, On Fri Jun 27, 2025 at 11:16 AM CEST, Richard Purdie via lists.openembedded.org wrote: > We have other places in the code where we need to take filemode/mask > information from a bitbake variable and turn it into a real python > number. Turn this internal code into public API in bb.utils and > add some tests for it. > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > --- > bin/bitbake-worker | 7 ++----- > lib/bb/tests/utils.py | 11 +++++++++++ > lib/bb/utils.py | 10 ++++++++++ > 3 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/bin/bitbake-worker b/bin/bitbake-worker > index 35fa1ab62be..9f33195176f 100755 > --- a/bin/bitbake-worker > +++ b/bin/bitbake-worker > @@ -182,11 +182,8 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): > elif workerdata["umask"]: > umask = workerdata["umask"] > if umask: > - # umask might come in as a number or text string.. > - try: > - umask = int(umask, 8) > - except TypeError: > - pass > + # Convert to a python numeric value as it could be a string > + bb.utils.to_filemode(umask) > > dry_run = cfg.dry_run or runtask['dry_run'] > > diff --git a/lib/bb/tests/utils.py b/lib/bb/tests/utils.py > index 48e61dfcea0..52b7bf85bf4 100644 > --- a/lib/bb/tests/utils.py > +++ b/lib/bb/tests/utils.py > @@ -692,3 +692,14 @@ class EnvironmentTests(unittest.TestCase): > self.assertIn("A", os.environ) > self.assertEqual(os.environ["A"], "this is A") > self.assertNotIn("B", os.environ) > + > +class FilemodeTests(unittest.TestCase): > + def test_filemode_convert(self): > + self.assertEqual(0o775, bb.utils.to_filemode("0o775")) > + self.assertEqual(0o775, bb.utils.to_filemode(0o775)) > + self.assertEqual(0o775, bb.utils.to_filemode("775")) > + with self.assertRaises(ValueError): > + bb.utils.to_filemode("xyz") > + with self.assertRaises(ValueError): > + bb.utils.to_filemode("999") > + > diff --git a/lib/bb/utils.py b/lib/bb/utils.py > index a2806fd360d..01f7cf81f46 100644 > --- a/lib/bb/utils.py > +++ b/lib/bb/utils.py > @@ -1211,6 +1211,16 @@ def which(path, item, direction = 0, history = False, executable=False): > return "", hist > return "" > > +def to_filemode(input): > + """ > + Take a bitbake variable contents defining a file mode and return > + the proper python representation of the number > + """ It could be nice to keep the same format as the other functions in this file: explicit the arguments and the return value. > + # umask might come in as a number or text string.. > + if type(input) is int: > + return input > + return int(input, 8) > + > @contextmanager > def umask(new_mask): > """ I think this triggered a wall of errors of the autobuilder: https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/1910 Locally I also reproduced the issue: ERROR: Task (/data/yoctoproject/ws/repos/openembedded-core/meta/recipes-bsp/efivar/efivar_39.bb:do_recipe_qa) failed with exit code '1' If I drop this commit (and the two others on OE-Core that use to_filemode), then re-run it becomes successful again. Cleaning the sstate of the recipe then, and re-introducing your commit only, triggers the error. I don't have any log to show because I think the issue happens before do_recipe_qa is run. Antonin
On Mon, 2025-06-30 at 09:42 +0200, Antonin Godard wrote: > Hi Richard, > > On Fri Jun 27, 2025 at 11:16 AM CEST, Richard Purdie via lists.openembedded.org wrote: > > We have other places in the code where we need to take filemode/mask > > information from a bitbake variable and turn it into a real python > > number. Turn this internal code into public API in bb.utils and > > add some tests for it. > > > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > > --- > > bin/bitbake-worker | 7 ++----- > > lib/bb/tests/utils.py | 11 +++++++++++ > > lib/bb/utils.py | 10 ++++++++++ > > 3 files changed, 23 insertions(+), 5 deletions(-) > > > > diff --git a/bin/bitbake-worker b/bin/bitbake-worker > > index 35fa1ab62be..9f33195176f 100755 > > --- a/bin/bitbake-worker > > +++ b/bin/bitbake-worker > > @@ -182,11 +182,8 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): > > elif workerdata["umask"]: > > umask = workerdata["umask"] > > if umask: > > - # umask might come in as a number or text string.. > > - try: > > - umask = int(umask, 8) > > - except TypeError: > > - pass > > + # Convert to a python numeric value as it could be a string > > + bb.utils.to_filemode(umask) > > > > dry_run = cfg.dry_run or runtask['dry_run'] > > > > diff --git a/lib/bb/tests/utils.py b/lib/bb/tests/utils.py > > index 48e61dfcea0..52b7bf85bf4 100644 > > --- a/lib/bb/tests/utils.py > > +++ b/lib/bb/tests/utils.py > > @@ -692,3 +692,14 @@ class EnvironmentTests(unittest.TestCase): > > self.assertIn("A", os.environ) > > self.assertEqual(os.environ["A"], "this is A") > > self.assertNotIn("B", os.environ) > > + > > +class FilemodeTests(unittest.TestCase): > > + def test_filemode_convert(self): > > + self.assertEqual(0o775, bb.utils.to_filemode("0o775")) > > + self.assertEqual(0o775, bb.utils.to_filemode(0o775)) > > + self.assertEqual(0o775, bb.utils.to_filemode("775")) > > + with self.assertRaises(ValueError): > > + bb.utils.to_filemode("xyz") > > + with self.assertRaises(ValueError): > > + bb.utils.to_filemode("999") > > + > > diff --git a/lib/bb/utils.py b/lib/bb/utils.py > > index a2806fd360d..01f7cf81f46 100644 > > --- a/lib/bb/utils.py > > +++ b/lib/bb/utils.py > > @@ -1211,6 +1211,16 @@ def which(path, item, direction = 0, history = False, executable=False): > > return "", hist > > return "" > > > > +def to_filemode(input): > > + """ > > + Take a bitbake variable contents defining a file mode and return > > + the proper python representation of the number > > + """ > > It could be nice to keep the same format as the other functions in this file: > explicit the arguments and the return value. > > > + # umask might come in as a number or text string.. > > + if type(input) is int: > > + return input > > + return int(input, 8) > > + > > @contextmanager > > def umask(new_mask): > > """ > > I think this triggered a wall of errors of the autobuilder: > https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/1910 > > Locally I also reproduced the issue: > > ERROR: Task (/data/yoctoproject/ws/repos/openembedded-core/meta/recipes-bsp/efivar/efivar_39.bb:do_recipe_qa) failed with exit code '1' > > If I drop this commit (and the two others on OE-Core that use to_filemode), > then re-run it becomes successful again. Cleaning the sstate of the recipe > then, and re-introducing your commit only, triggers the error. I don't have any > log to show because I think the issue happens before do_recipe_qa is run. Thanks, I've tweaked the docstring and fixed the issue in v2. Cheers, Richard
diff --git a/bin/bitbake-worker b/bin/bitbake-worker index 35fa1ab62be..9f33195176f 100755 --- a/bin/bitbake-worker +++ b/bin/bitbake-worker @@ -182,11 +182,8 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): elif workerdata["umask"]: umask = workerdata["umask"] if umask: - # umask might come in as a number or text string.. - try: - umask = int(umask, 8) - except TypeError: - pass + # Convert to a python numeric value as it could be a string + bb.utils.to_filemode(umask) dry_run = cfg.dry_run or runtask['dry_run'] diff --git a/lib/bb/tests/utils.py b/lib/bb/tests/utils.py index 48e61dfcea0..52b7bf85bf4 100644 --- a/lib/bb/tests/utils.py +++ b/lib/bb/tests/utils.py @@ -692,3 +692,14 @@ class EnvironmentTests(unittest.TestCase): self.assertIn("A", os.environ) self.assertEqual(os.environ["A"], "this is A") self.assertNotIn("B", os.environ) + +class FilemodeTests(unittest.TestCase): + def test_filemode_convert(self): + self.assertEqual(0o775, bb.utils.to_filemode("0o775")) + self.assertEqual(0o775, bb.utils.to_filemode(0o775)) + self.assertEqual(0o775, bb.utils.to_filemode("775")) + with self.assertRaises(ValueError): + bb.utils.to_filemode("xyz") + with self.assertRaises(ValueError): + bb.utils.to_filemode("999") + diff --git a/lib/bb/utils.py b/lib/bb/utils.py index a2806fd360d..01f7cf81f46 100644 --- a/lib/bb/utils.py +++ b/lib/bb/utils.py @@ -1211,6 +1211,16 @@ def which(path, item, direction = 0, history = False, executable=False): return "", hist return "" +def to_filemode(input): + """ + Take a bitbake variable contents defining a file mode and return + the proper python representation of the number + """ + # umask might come in as a number or text string.. + if type(input) is int: + return input + return int(input, 8) + @contextmanager def umask(new_mask): """
We have other places in the code where we need to take filemode/mask information from a bitbake variable and turn it into a real python number. Turn this internal code into public API in bb.utils and add some tests for it. Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- bin/bitbake-worker | 7 ++----- lib/bb/tests/utils.py | 11 +++++++++++ lib/bb/utils.py | 10 ++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-)