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

Allow overriding when an optional import took place #2523

Open
redsun82 opened this issue Dec 12, 2024 · 6 comments
Open

Allow overriding when an optional import took place #2523

redsun82 opened this issue Dec 12, 2024 · 6 comments

Comments

@redsun82
Copy link

redsun82 commented Dec 12, 2024

Hi!

I would like to be able to detect within a justfile whether an optional import took place.

I found out a workaround to do that within recipe bodies, namely

# ./maybe.just
set export

IMPORTED := "true"

# ./justfile
import? 'maybe.just'

imported:
    ${IMPORTED:-false} && echo yes || echo no

however I did not find a way to have that information available in expressions.

If there's no way currently, here's a couple of things I tried that might be good features in their own right, and would solve this problem:

  • when allow-duplicate-variables is set, one could have the position of the import be relevant for overriding. Reading the documentation of allow-duplicate-variables, I actually expected this to work:

    # ./maybe.just
    imported := "true"
    # ./justfile
    imported := ""
    import? "maybe.just"
    default:
        echo {{ if imported == "true" { "yes" } else { "no" } }}

    If imports are allowed anywhere in a just file, I would expect duplicates from imports to override previous definitions (if duplicates are allowed).
    If this held for recipes as well, I would probably even not need to use an if, and would just overwrite some recipes in maybe.just.

    # ./maybe.just
    default:
        echo yes
    # ./justfile
    default:
        echo no
    import? "maybe.just"

    Afterwards I could see this goes directly in contrast with the way imports are specified in the documentation... so maybe this route could be taken with a new directive like include/include??

  • a ? postfix operator could allow using undefined variables: var? would evaluate to the value of var if var is defined, and to "" otherwise.

    # ./maybe.just
    imported := "true"
    # ./justfile
    import? "maybe.just"
    default:
        echo {{ if imported? != "" { "yes" } else { "no" } }}
  • an imported function. It could be specified to return "true" for modules that were imported

    # ./maybe.just
    
    # ./justfile
    import? "maybe.just"
    default:
        echo {{ if imported('maybe.just') == "true" { "yes" } else { "no" } }}
  • for completeness, though I don't particularly like such a solution, an optional variable name after an import? could be filled with "true" or "" depending on whether the import took place

    # ./maybe.just
    
    # ./justfile
    import? "maybe.just" imported
    default:
        echo {{ if imported == "true" { "yes" } else { "no" } }}
@casey
Copy link
Owner

casey commented Dec 12, 2024

Thanks for the issue!

I think of these, some way to get the value of a variable, or a fallback if undefined, would be pretty reasonable.

I'm not sure about var?, because then i looks like an operator which operates on the variable var, when in fact what it really does is operate on the string "var", and see if it exists in the local scope.

Maybe a var("name") function which takes a string and returns the value of the variable named "name" in the current scope, if defined?

@redsun82 redsun82 changed the title Detect whether an optional import took place Allow overriding when an optional import took place Dec 13, 2024
@redsun82
Copy link
Author

I'm not sure about var?, because then i looks like an operator which operates on the variable var, when in fact what it really does is operate on the string "var", and see if it exists in the local scope.

Maybe a var("name") function which takes a string and returns the value of the variable named "name" in the current scope, if defined?

That sounds very reasonable to me! var("name") (and maybe var("name", "default") mirroring env) would be quite good.

However, I also thought a bit more about my use case, and it might be that some way to have imported recipes override local ones would actually suit me better, so let me describe it (sorry if I'm changing the original issue a bit here and posting another wall of text!)

In our case, we have two repositories, one public and one private. The public one is also a submodule of the private one. Now, we have two ways of building things. Very roughly speaking, parts of the public repo can be built standalone using host toolchains, but when we build those parts as a checked out submodule of the private repo, we want to build those parts integrated into the build system of the private repo.

So in a nutshell, just build (and other commands) run within the public repository should behave differently whether we are in a standalone working copy or as part of the wider private repo checkout.

The answer to my original question (and the var("name") solution) would allow me to solve this problem, but would also mean that:

  • recipes would be somewhat boilerplat-y to write, all with if blocks (whether justfile ifs or bash ifs)
  • more importantly I would need to inject into the public repo information about how we build in the private one: that would be problematic when we want to update that (changes synchronized across git submodules generally suck). I would prefer to keep things related to the private build system encapsulated in the private repo.

So all in all we would probably benefit most from some mechanism achieving what I described in the first bullet point, where I would be able to override the build and other recipes in the optionally imported file from the private repo. Again, I have some ideas about going at that:

  • One would be the the include? directive I already mentioned, paired with allow-duplicate-* settings, which would allow overriding recipes and variables with an optionally present just file. Incidentally, that would also allow uncommitted local just files to be able to tweak committed configurations, provided those add something like an include? 'local.just' (with local.just .gitignored) at their very end. Contrarry to import, that would allow local.just to not only add but also change existing recipes.

  • A solution with attributes, combined with the var("name") idea, would be adding enable_if and enable_if_not attributes to allow enabling a recipe under a given condition, like cfg in Rust. This would require Allow expressions as arguments to attributes #2521, although I guess here it would be a matter of using a condition rather than an expression, which I think are different entities in the grammar, right? That would make attributes like windows just syntactic sugar for enable_if(os() == "windows"). I guess this could be handy in other scenarios as well. For my use case, I could for example disable the build recipe on the internal repo when detecting being a submodule (via the var("name") thing), and then the build recipe imported from the private repo would take the lead.

  • Another possibility I thought of is an override attribute, allowing a recipe to be chosen without errors over another recipe with the same name. However one drawback I can see of this, is that it would work quite well for recipes but wouldn't allow overriding variables (not without extending attributes to variables I guess). That could be covered by the var("name") idea though

By the way, as a reward for reading this all, let me congratulate you for this project 🙂. I heard about it first time this week, and started trying it out yesterday, and I really find it awesome, kudos!

If you prefer to keep this issue scoped to the var("name") feature, I'll be happy to undo the title change here and open a separate one for allowing recipe overrides.

@redsun82
Copy link
Author

redsun82 commented Dec 16, 2024

Hey @casey, I just found out that you can actually achieve what I was aiming for with the existing version: the trick is to put the things one wants to override (recipes or variables) behind an import as well. The ordering is somewhat unintuitive, as the topmost imports seem to override bottom ones, which seems to contradict the documentation (last definition is used). But here's a full working example:

# maybe.just
hi := 'hello override!'

foo:
   echo "I'm overridden"

# base.just
hi := 'hi'

foo:
    echo "I'm not overridden"

# justfile
set allow-duplicate-variables
set allow-duplicate-recipes

import? 'maybe.just'  # notice overriding import must come first!
import 'base.just'

hi:
    echo {{ hi }}

Then just foo will print I'm overridden if maybe.just is present, I'm not overridden otherwise. Similarly, just hi will take the overridden value of hi depending on the presence of maybe.just.

The order in which duplicate definitions are resolved from imports might need a look though: I guess making it more intuitive (last import takes the spot) might break some configurations out there, but maybe a note in the docs might help?

Feel free to take whatever idea I've thrown at you that seems worthwhile. For example I still think var("name") and var("name", "default") (or `var("name") || "default" maybe?) could be useful. But for the rest we can now do what we wanted already, so also feel free to close.

@casey
Copy link
Owner

casey commented Dec 22, 2024

Hmm, yah that's definitely a bit odd. I dug in and it's because:

  • Just uses a stack to process imports, when it encounters an import, it pushes it onto a stack.
  • As a result, it processes imports in reverse order, since maybe.just gets pushed to the stack, base.just gets pushed to the stack, and then the next to be popped from the stack is base.just.
  • When set allow-duplicate-recipes is set, recipes are first compared by depth (how deep they were imported), and if they have equal depth, the new recipe overrides the old recipe (however because of this stack behavior, recipes in modules are considered in opposite of source order)

This is not great behavior. When set allow-duplicate-recipes is set and there are no imports involved, later recipes override earlier ones:

set allow-duplicate-recipes

foo:
  # overridden

foo:
  # not overridden

When the duplicate recipes are behind imports, however, recipes in earlier imports override recipes in earlier imports.

just has a very strong backwards compatibility guarantee, but it's really tempting to declare this a bug, since the behavior is so unintuitive. However, annoyingly, any user that encounters this can work around it and get things working, and any user that's done that will be broken by any change in behavior.

I'm going to create a separate issue to discuss this.

Weird ordering aside, is this a good enough for your use-case? I'm a little bit hesitant about var("name"), I think it would actually have to be something with new syntax, like name? (having a function that takes a string complicates static analysis enormously, because the string argument could itself be the result of evaluating other variables), and I'm not sure introducing new syntax for a niche feature is worth it.

@casey
Copy link
Owner

casey commented Dec 22, 2024

Created an issue in #2540 to track this weird order issue.

@casey
Copy link
Owner

casey commented Dec 22, 2024

Documented the weirdness in #2541.

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

2 participants