Aliases are inconsistent in `Variables.options`

Question

Aliases are inconsistent in `Variables.options`

Opened this issue a month ago · 7 comments

Describe the bug
Aliases are inconsistent in Variables.options. Second member of tuple (which represents option) does contain original name if option doesn't have aliases. But it doesn't contain original name if option has aliases.

import SCons

env = Environment()

options = Variables()

options.Add(["option_with_alias", "alias", "alias2"], "", "")
options.Add("option_without_alias", "", "")

print(options)

Output:

Variables(
  options=[
    ('option_with_alias', ['alias', 'alias2'], '', '', validator=None, converter=None),
    ('option_without_alias', ['option_without_alias'], '', '', validator=None, converter=None),
  ],
  args={},
  files=[],
  unknown={},
)

Required information

Version of SCons 4.8.1
Version of Python 3.12.6
How you installed SCons 🤷‍♀️ maybe pip maybe apt from debian testing

Answer 1 · 2024-11-11T20:42:29.000Z

agree, it's inconsistent. when I found that, we didn't want to change existing behavior if nobody was complaining. which version feels more natural? name in aliases or not?

Answer 2 · 2024-11-11T21:11:14.000Z

Today I needed to implement custom UnknownVariables ¹ . I needed to manually check if something provided bu user is inside env or opts. And failed with env because env["alias"] as like "alias" in env failed with KeyError. Ok... I though... Env is env a n object which is used for actual process while Variables object generally is ruleset so ok lets check inside Variables and found this inconsistency... In my case it would be easier if I have ("name" ["name" + aliases] but from perspective of general logic it is better to have empty aliases array for option without aliases to separate them 🤷‍♀️

because it is broken too. It doesn't list variables defined inside files (because I read code and IIUC it pushes to unknown list only options which came from cli). Variables in general is too restrictive... It doesn't allow you to easily distinguish what came from files and what from cli args and etc... ↩

Answer 3 · 2024-11-11T21:14:04.000Z

agree, it's inconsistent. when I found that, we didn't want to change existing behavior if nobody was complaining

Do you need someone to complain? I am ready to complain! I've already seen tons of workarounds and needed to create them myself. I only work with scons for 1~1.5 months... From now I will report everything I find

Answer 4 · 2024-11-11T21:17:01.000Z

Or is it a published api? If no, then it will hurt many people (I will be hurted too), but there is option better. Tuples doesn't have embedded names so is there any difference to do options.options[0] to get actual name or do options.options[0][0]? I propose here to merge these two options so if you doesn't have aliases you will get (["name"] and if you have aliases you will get (["name", "alias1", "alias2"]

Answer 5 · 2024-11-11T21:20:35.000Z

And just formally as option itself is a tuple, why not to use tuple instead of list for aliases? Does SCons allow to modify aliases at runtime? If no why not to use tuple? More constness

Answer 6 · 2024-11-12T16:13:06.000Z

@bdbaddog any opinions here? When I ran across this before, I left it as-is, because tests were written to the existing behavior, and I left a comment about it. I didn't remember, but it looks like I preferred the eventual approach of leaving the name out of the alias list in both cases.

        if SCons.Util.is_Sequence(key):
            option.key = key[0]
            option.aliases = list(key[1:])
        else:
            option.key = key
            # TODO: normalize to not include key in aliases. Currently breaks tests.
            option.aliases = [key,]

The name (key) is added later for processing anyway, so it doesn't really need to be there, e.g. like this:

                if arg in option.aliases + [option.key,]:

I don't have an opinion on tuple-vs-list, which is a separate question above.

Answer 7 · 2024-11-12T17:36:07.000Z

agree, it's inconsistent. when I found that, we didn't want to change existing behavior if nobody was complaining

Do you need someone to complain? I am ready to complain! I've already seen tons of workarounds and needed to create them myself. I only work with scons for 1~1.5 months... From now I will report everything I find

Please don't just file issue without discussing them first. To ensure they are a) actually a issue and not user error, and b) that there isn't already an issue filed.

Footnotes