casey / just

đŸ¤– Just a command runner
https://just.systems
Creative Commons Zero v1.0 Universal
18.08k stars 409 forks source link

Add quick-and-dirty justfile inclusion #1407

Closed casey closed 1 year ago

casey commented 1 year ago

Users have wanted to be able to include other justfiles for a long time, and I've wanted to support this. However, I've been worried about implementing something hacky that would bring problems and be hard to support and debug. I've had a design in my head for a module system, but it's complicated enough that I'm unlikely to implement it any time soon.

As a compromise, I think that we should add a simple include statement, similar to @MadBomber's justprep.

I'm unlikely to implement this myself, because I have other projects which are taking all my time, but if someone wants to pick this up, I can review and help them get it over the line.

The design is this:

The idea behind using the word include instead of use or import is that we want to express that we're doing quick-and-dirty inclusion of the tokens. Later, if someone is feeling super motivated, or I get around to it, we can implement a module system, and users can migrate to the module system if they want to.

Additions and improvements that shouldn't block the initial PR:

casey commented 1 year ago

@MadBomber what do you think of this design?

MadBomber commented 1 year ago

@casey The advantage is that you do not need both justprep and just with this design - its just just :) Reducing the number of utilities is generally a good idea.

OTOH if you do not implement a way to isolate the errors to the place they came from, you can get into a little bit of trouble. The justprep utility does indicate in the final justfile created from the included files via comments which section of the file comes from which included file so that any errors discovered by just are easily traced to the actual cause.

justprep also supports the fake module convention which of course can be easily implemented manually within a justfile but it is som much nicely to just say module followed by the path to the justfile.

casey commented 1 year ago

Thanks for the feedback!

Yeah, we definitely eventually want proper error messages. I think that can wait for a follow-up PR, since it won't be a breaking change to add error messages. I amended the original issue to note this.

kbknapp commented 1 year ago

The company I work for replaced our command runner use of make with just a while back for a large company-wide monorepo. We have a private fork of just that does exactly this naive include (this is the only difference from upstream).

If you're interested, I could put in a PR with those changes. The implementation is about as basic as it gets, but it seems to work really well and has drastically cleaned up our just usage.

Essentially, at the very beginning of parsing we look for #include "path/to/file.just" which includes the contents of that file at the point of #include which as it turns out is kind of what many people tend to expect anyways. Being so early in the process allows just to function exactly as it already expects without change.

The #include syntax is totally changeable if you'd prefer say @include or perhaps set include := "path/to/file.just" that'd just be a simple change. I believe the set ... syntax happens later in parsing from my last looking at the source which is why we didn't go that route ourselves. Additionally we used #include because to upstream just it appears as a comment, although to upstream this change that might be a downside which could create confusion?

If this got upstreamed we'd be able to move back to upstream just which would be great for us too!

casey commented 1 year ago

@kbknapp Nice! Definitely open a PR with the changes, I’ll probably tweak a bit, but it will be a good starting point, and allow me to make sure what eventually gets merged works for your use case.

mihaigalos commented 1 year ago

I want to express concern about potentially including malicious code from other justfiles.

I understand we are discussing pre-PoC here but I would love to help with at least also specifying an optional hash checksum for the included Justfile. That way the inclusion fails if there is a hash mismatch when the file has been changed:

!include /tmp/Justfile dc34e068f11efee830d5676f12b5e2dbc059d45e08f6e5c8880ed208676ab6d4

That's because the main Justfile might belong to the current user (Linux/MacOS permissions 400, and hence potentially cannot be modified by other users), but the included one might not.

casey commented 1 year ago

I think it's outside the scope of this feature to protect against including untrusted input. If you don't trust the file, you shouldn't include it, and an easy workaround it to move the file to the same location/same permissions as the current justfile.

mihaigalos commented 1 year ago

That's a simple workaround, I like it.

If you don't trust the file, you shouldn't include it.

This is a strong statement. Also, Network-mounted paths and URLs still affected.

MadBomber commented 1 year ago

I've always viewed the justfile concept as being local within the context of my own workstation - most likely coming from a repository. Including from a URL is a little over-board IMO - coming from a file server its either mine or my team's. With this context, having the hash of the included file causes more trouble than its worth.

jakubsacha commented 1 year ago

The lack of an include feature is a significant limitation of the tool in my opinion.

In terms of hash validations, local includes are already quite powerful. Remote includes (via http/https) would be a fantastic feature because they would allow you to centralize certain reusable code blocks.

Remote includes, in my opinion, should be used by pointing to a raw file stored in git. If security is a concern, a very specific commit could be used: https://raw.githubusercontent.com/casey/just/66993e874d877e9326de14fbc68c41726567d528/examples/kitchen-sink.just

A similar solution is used, for example, in github actions.

dhendry commented 1 year ago

I found this tool just the other day and it looks incredibly cool and a near perfect fit but the lack of an includes capability is a deal breaker when considering a migration away from makefiles.

We have a large number of services which all have a Makefile but they import a substantial amount of functionality from a common makefile to avoid a ton of copy pasta.

+1 for this capability!

casey commented 1 year ago

Implemented in #1470, and released in 1.12.0. Binaries should be up shortly. Try it out and let me know what you think!

casey commented 1 year ago

Thanks to @neunenak for implementing this!

elygre commented 1 year ago

Implemented in #1470, and released in 1.12.0. Binaries should be up shortly. Try it out and let me know what you think!

I have a justfile that I'd like to decompose a bit. Unfortunately, the restriction that "!include directives are only processed before the first non-blank, non-comment line" is stopping me from using it as I intended. There may be workarounds, but this is the way I was planning to do it:

var1 := "..."
var2 := "..."

_list:
   just --list

run:
   /usr/local/bin/theruncommand --option1 {{var1}} --etc --etc

# Include receipes which can reference var1 and var2
!include src/just/database.just
!include src/just/setup.just

I read your comment https://github.com/casey/just/pull/1470#issuecomment-1368703458, about !include appearing inside a multiline string. I understand that this could create trouble, but I guess we have to trust the script? A bit like https://github.com/casey/just/issues/1407#issuecomment-1368713655, perhaps. If required, I would prefer having a setting similar to set fallback and set dotenv-load.

casey commented 1 year ago

@elygre Justfiles are actually insensitive to order, so you can refer to variables at the top of a justfile that aren't defined until later in the file.

elygre commented 1 year ago

@casey Huh, I did not realize that. OK, that solves my problem :-)

When you revisit the documentation, perhaps add a small comment to this effect in https://just.systems/man/en/chapter_52.html. Perhaps add this sentence:

Note that justfiles are insensitive to order. This means that the included justfiles have access to variables defined in the parent justfile, even if these variables are defined after the !include directive.

(There is also a typo in that chapter, saying "beginning of a line. line.")

casey commented 1 year ago

Good addition to the docs, just opened #1529, and thanks for finding the typo!