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

doc: add first documentation draft #237

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

jkloetzke
Copy link
Member

@jkloetzke jkloetzke commented Nov 22, 2024

No description provided.

@rhubert
Copy link
Contributor

rhubert commented Nov 25, 2024

Thanks. That already helps a lot.

What I found still missing:

  • how to name variables. (BASEMENT_FOO vs. ${RECIPE_NAME}_FOO vs.FOO`)
  • -bin packages (or -tool??) next to -dev and -tgt

@jkloetzke
Copy link
Member Author

Sure, this is just a early draft. Most of the required documentation is still missing. We'll certainly expand it in the near future.

The question for this PR is mainly if you think the recipe categories make sense from your point of view. I'm really open for suggestions so that we avoid having to rename settled categories later on...

@jkloetzke
Copy link
Member Author

What I found still missing:

  • how to name variables. (BASEMENT_FOO vs. ${RECIPE_NAME}_FOO vs.FOO`)
  • -bin packages (or -tool??) next to -dev and -tgt

Before I write this with more words in the documentation: how about the following rules?

  • Variables should be named like the recipe. So given a recipe recipes/foo/bar.yaml, a variable should be called BAR_SOMETHING. Rationale: the foo is just the category and there are usually no two recipes with the same name in different categories.
  • Variables in privateEnvironment could be named as needed without any restrictions.
  • We already started with the habit of prefixing variables that configure a recipe by CONFIG_ in basement-gnu-linux. I found this appealing at first but now I'm wondering if this would not apply to all "public" variables of a recipe anyway. This makes the CONFIG_ prefix kind of redundant. OTOH, it can make them discoverable by grep or other means in the future. I'm not sure yet what is the best approach here. I'm curios what you think about this.

Regarding package names:

  • Even though we now have some -bin packages, I tentatively think this was a mistake. The -dev and -tgt suffixes are for library related packages. Regular application should not have any suffix (like utils::bash does not have any either).
  • If a recipe foo builds a shared library and an application using this library, the packages should be called foo-dev for the library development package, foo-tgt for the shared library that is supposed to hit rootfs and foo for the application. libs::zstd is a good example of this.
  • Usually, no separate -tool package is required. If the package is also usable as tool, just add the provideTools to the regular package. (devel::dtc being a example) But if it makes sense to have a different build configuration for the tool, then there still could be a -tool or -tools package. I guess there is some gray area in this respect.

@sixtyfourktec @mahaase Please chime in if you have any thoughts on this.

@sixtyfourktec
Copy link
Contributor

sixtyfourktec commented Nov 26, 2024

We already started with the habit of prefixing variables that configure a recipe by CONFIG_ in basement-gnu-linux. I found this appealing at first but now I'm wondering if this would not apply to all "public" variables of a recipe anyway. This makes the CONFIG_ prefix kind of redundant. OTOH, it can make them discoverable by grep or other means in the future. I'm not sure yet what is the best approach here. I'm curios what you think about this.

Because we talked about documentation earlier I think being able to really know what a config var is will be helpful. It will make it possible to find those and maybe add some info about it in any automatic created documentation. But than again, maybe bob should have an idea about what config variables there are as a first level build-in. I vision some auto generated website showing all available packages with name, desc, version, ... but also all possible config vars and some description of those.

I agree on the package target naming you proposed.

@rhubert
Copy link
Contributor

rhubert commented Nov 27, 2024

We already started with the habit of prefixing variables that configure a recipe by CONFIG_ in basement-gnu-linux. I found this appealing at first but now I'm wondering if this would not apply to all "public" variables of a recipe anyway. This makes the CONFIG_ prefix kind of redundant. OTOH, it can make them discoverable by grep or other means in the future. I'm not sure yet what is the best approach here. I'm curios what you think about this.

I looked over my recipes and found I wide range of names. :) Some of them are prefixed with CONFIG_${RECIPE_NAME} some are using FEATURE_, less widely used we have IS_ and USE_.... Sometimes the naming is inverted like ${RECIPE_NAME}_CONFIG_,.. but in the end these are only variables configuring the build somehow. Either by enabling / disabling features or controlling the package version, ...

So for me it would be fine to name them BAR_SOMETHING without any additional CONFIG, FEATURE, ...

Because we talked about documentation earlier I think being able to really know what a config var is will be helpful. It will make it possible to find those and maybe add some info about it in any automatic created documentation. But than again, maybe bob should have an idea about what config variables there are as a first level build-in. I vision some auto generated website showing all available packages with name, desc, version, ... but also all possible config vars and some description of those.

I like the idea, but I'm unsure if it really needs a bob-feature for this. I think a comment-section would be sufficient. Something like:

# Config-variables:
# FOO_VERSION : overrides the default package version
# FOO_DEBUG   :  1 - debug mode enabled
#             :  0 - debug mode disabled (default)
#
metaEnvironment:
  PKG_VERSION: "${FOO_VERSION:-1.2.3}

buildVars: [FOO_DEBUG]

@jkloetzke
Copy link
Member Author

I like the idea, but I'm unsure if it really needs a bob-feature for this. I think a comment-section would be sufficient. Something like:

# Config-variables:
# FOO_VERSION : overrides the default package version
# FOO_DEBUG   :  1 - debug mode enabled
#             :  0 - debug mode disabled (default)
#
metaEnvironment:
  PKG_VERSION: "${FOO_VERSION:-1.2.3}

buildVars: [FOO_DEBUG]

This can be done by a plugin and I went ahead to actually implemented it. See the last two, new commits. The new Config key in the recipe can be used to describe variables. The example above would become:

Config:
    FOO_VERSION:
        help: overrides the default package version
    FOO_DEBUG:
        type: bool # This allows only "0" and "1"
        help: Enable debugging. Disabled by default.

There are also other types:

Config:
    EDK2_PLATFORM:
        type: choice
        help: Choose platform for EDK2 build.
        choice:
            OVMF_I386:
                help: >
                    Platform configuration for a generic i386 target.
                    This platform will boot from flash address 0x0.
                    It should therefore be used as the first bootloader.

            OVMF_RISCV:
                ...

    REQUIRED_VAR:
        type: str # this is the default type anyway
        required: True # But variable must be present

    FOO_BASE_ADDRESS:
        type: hex
        required: True
        range: [0x00, 0xffffffff] # The range is optional

    NUM_USERS:
        type: int
        range: [1, 10]

The benefit of having such a description machine-readable in the recipe is that variables can be checked to some extent. The plugin will already check for required variable, refuse invalid choices, see if numbers are convertible and are in the required range. Later down the road this can certainly be extended.

I also changed the gcc recipe to showcase how it can look like. Let me know what you think.

@rhubert
Copy link
Contributor

rhubert commented Dec 1, 2024

Nice!

Copy link
Contributor

@sixtyfourktec sixtyfourktec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also have an optional "default" entry?

def handleStr(var, val):
pass

def handleHex(var, val):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we handle that in the int function below. I am not a fan of having dedicated methods for converting those.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the error strings are different too. Not sure if we gain too much to deduplicate the remaining code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it is not for saving code. It is just more user friendly, because numbers are simply numbers. However, if it is too complicated, then fine.

@jkloetzke
Copy link
Member Author

Can we also have an optional "default" entry?

I knew this was coming and it makes a lot of sense. 😄 Unfortunately, this requires changing Bob. The onFinish method is called after the internal package has already been created. So this will have to wait until 1.0 is released.

The config plugin defines a new recipe keyword: "Config". It is used to
describe variables that configure the behaviour of the recipe. This is
obviously optional but should be used in the future to make
configuration knobs of recipes discoverable and to provide some basic
sort of consistency checking.
Add the definition of the main configuration variables with some small
documentation.
@jkloetzke jkloetzke merged commit 4ccb01a into BobBuildTool:master Dec 4, 2024
2 checks passed
@jkloetzke jkloetzke deleted the add-docs branch December 4, 2024 21:19
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

Successfully merging this pull request may close these issues.

3 participants