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

isar-bootstrap.inc::get_distro_primary_source_entry() should not return ["", "", ""] by default #72

Closed
HenningPuttnies opened this issue Oct 1, 2021 · 7 comments

Comments

@HenningPuttnies
Copy link

From my point of view, this function:
https://github.com/ilbers/isar/blob/master/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc#L165

should not return ["", "", ""] by default. Instead, is should throw an error.

The rational is as follows: returning ["", "", ""] leads to ambiguous errors, if, e.g.,:

  • generate_distro_sources(d) does not contain source (i.e. it is not found and thus empty)
  • source[0] != "deb": (i.e. is it to a debian distro)
@ismagulb
Copy link
Contributor

ismagulb commented Oct 1, 2021

Hello Henning,

we'd like to understand the context, can you provide the steps to reproduce?

If you also have a suggestion in form of a patch, feel free to share.

@henning-schild
Copy link
Contributor

I guess the way to repro would be to use an "invalid" i.e. empty sources.list

If that happens we indeed could probably detect that early instead of returing something that will just break in a weird way further down the line.

Maybe return None, None, None or simply raise/bbfatal

@HenningPuttnies
Copy link
Author

@henning-schild I would highly appreciate that. In my case, the list was not empty. However, I made a typo in the distro name. Thus, nothing was read from the list.

It took me a long time to figure out the root cause. I think throwing an error in this function at isar layer would massively ease debugging.

@henning-schild
Copy link
Contributor

In this case i am afraid the example file would be indeed needed. Because you maybe did not even run into the ["", "", ""] but instead the parser returned your invalid distro. Which i think we can not do anything about, because the parser should not know all valid values ... would be too hard to maintain.

@amikan
Copy link
Collaborator

amikan commented Nov 16, 2021

Just have proposed the patch at [PATCH] isar-bootstrap: Fail on invalid apt source list.
The only inconvenience is that the error during the parsing will be followed by do_bootstrap contents which can be confusing.

@HenningPuttnies
Copy link
Author

@amikan Thanks. I will try it and let you know.

@HenningPuttnies
Copy link
Author

@amikan It works perfectly. Thanks.

I had to slightly rework you patch as we actually work with this version:
76eac23

amikan added a commit that referenced this issue Nov 29, 2021
Returning empty list leads to ambiguous errors, so we should fail early
if distro sources.list is empty or incorrect.

Fix issue #72

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
amikan added a commit that referenced this issue Dec 1, 2021
Returning empty list leads to ambiguous errors, so we should fail early
if distro sources.list is empty or incorrect.

Fix issue #72

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
amikan added a commit that referenced this issue Dec 13, 2021
Returning empty list leads to ambiguous errors, so we should fail early
if distro sources.list is empty or incorrect.

Fix #72

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
amikan added a commit that referenced this issue Dec 16, 2021
Returning empty list leads to ambiguous errors, so we should fail early
if distro sources.list is empty or incorrect.

Fix #72

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
robang74 pushed a commit to robang74/isar that referenced this issue Dec 26, 2022
Returning empty list leads to ambiguous errors, so we should fail early
if distro sources.list is empty or incorrect.

Fix ilbers#72

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
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

No branches or pull requests

4 participants