import targets from another magefile about mage HOT 14 CLOSED

From an initial trace through the current implementation it looks like a way to achieve this is to detect dot imports in the AST from any magefiles found and then recursively parse those as magefiles, adding
the exported functions to the list of targets. The dot import would need to be added then also to the mage_output_file.go. The full list of found exported functions are then checked for dupes as they currently are and the list of all functions is then added to the switch statement. Because we copy over the dot import it should Just Work TM.

Assuming this rough sketch is deemed a reasonable path to take, there is the question of whether or not to support all dot imports recursively or only support a single level. In other words, my magefile dot imports a base magefile which itself dot imports yet another base magefile, etc. A user might expet this to just work. It seems supporting this wouldn't add much implementation complexity: add another dot import to the generated mage_output_file.go and take care to ensure all targets are added to the list to check for dupes.

If this seems like the most straightforward way to implement it I'll take a crack at supporting the full nested dot import resolution approach. If it starts to reveal a bunch of dragons I'll pull back and try to a simpler single level of dot import resolution approach.

from mage.

So, I don't think copying the . import into the output file will be necessary. I'm assuming you'll have a magefile in the current directory that does a . import of somewhere else. Since the output file will be compiled with that file, you can reference the functions from it directly, without an import.

Good point about multiple depths of imports... good luck with that, I hope there aren't dragons, but no big deal if we can only do a single level to start.

from mage.

Found a wrinkle: if you dot import a base magefile just to signal that you want its functions merged into the current scope of your magefile but you don't explicitly reference any of those functions in the magefile the compiler will produce an imported but not used error. In retrospect, this is obvious.

A wildcard import would, of course, not produce this error but is a bit of a semantic perversion compared with the dot import.

Alternative approaches that come to mind:

1. Use a wildcard import to signal what the dot import would have. This is very succinct but feels a bit off for the reason listed above.
2. Add a custom import declaration comment such as //mage:import example.come/base/magefile that we look for in a magefile's AST. This feels somewhat brittle because we have to parse the loosely structured string. Also it effectively adds to the mage "interface" unlike a dot import which just piggy backs on existing semantics. Feels too bespoke.
3. Add something like a WithTargetImports([]string) or WithTargetImport(string) function option to mage.Main which would have one or more import paths to magefiles. This would require that you define a zero install style shim. If the WithTargetImport(s) option is provided then it would use each import provided as the base set of targets. It would then do what it currently does which is look for magefiles in the current directory. If it finds any their targets are merged in. If it finds none then it just generates mage_output_file.go with the imports listed.

Without trying each one on for size to see which feels the best from a user's perspective my first impression is that I would probably go for option 3. It requires the most boilerplate of the 3 but would likely have the simplest and most robust implementation. It's also the most explicit and least magical.

If anyone has other ideas or has arguments against #3 let me know. In the meantime I'll proceed down that route.

from mage.

Hmm, that's unfortunate. It won't be unused when you compile, because there will be references in mage_output_file, but it fails during the parsing...

The underscore import would work. I think that's the cleanest way to do it. The only wacky thing is, that if you wanted to actually reference functions or whatever from that package, you'd need both the underscore import and a normal import. Which evidently compiles.. I wasn't sure it would, but I tried it.

from mage.

Yeah, I tried that too and was surprised it compiled since the dot import would accomplish the same thing.

Either way, if you are fine with the sort of misappropriation of the wildcard import I'll take that approach. Adding options to mage.Main would have been a breaking change anyway because the exported (though seemingly private) ParseAndRun would need to change to thread the options through.

from mage.

A murky edge case arises when someone has a wildcard import in their magefile that they simply want to treat as a wildcard import. That would likely be rare in the context of a magefile but the result would be surprising and the only work around would be to not wildcard import it. It doesn't seem worth trying to solve for this but I wanted to flag it.

from mage.

I think we'll be ok, because I think we should still be checking for files with the //+build mage tag. Which means if you blank import that, then it has a definite meaning.

from mage.

That's what I thought at first but if you import a package that only has a //+build mage file(s) in it and no other buildable files then you'll get a no buildable source files error at parse time which was one of the reasons I was leaning toward the WithTargetImport route since it would avoid this parse time error while still constraining what it imports to only files with the mage build tag.

from mage.

That seems like a bug, since we should be specifying the mage build tag when we parse those files.

from mage.

Looping back on this: after the initial exploration of an implementation proved non-trivial I came across grift (https://github.com/markbates/grift) which supports importing tasks (and has namespaces!) so it looks like it suits my use case.

from mage.

I just had a good idea about this.. I think if we just require a //mage:import comment on the import statement, we can use either the underscore import if you don't need to reference the functions, or dot import if you do.

from mage.

As an aside, this feature plus composable namespaces (#152) would open some pretty fantastic doors.

I'm using mage in a monorepo project with a small set of core targets (e.g., test, build, package, deploy, etc.) that are consistent across all "sub-projects" in the monorepo. Each sub-project has its own magefile.go and there is a root-level magefile.go that "aggregates" the targets of the sub-projects' magefile.go files with some namespacing. (I say "aggregates" because currently the aggregation is hand-curated.)

My project layout looks like...

my-project/
  sub-project-A/
    magefile.go
  sub-project-B/
    magefile.go
  sub-project-C/
    magefile.go
  magefile.go

Each sub-project's targets are essentially...

$ mage -l
Targets:
  test           Runs unit tests.
  build         Compiles the project.
  package    Packages the project.
  deploy      Deploys the project.

And the (hand-curated) root-level targets are of the form...

$ mage -l
Targets:
  test:all    Runs all projects' unit tests.
  test:a      Runs project A unit tests.
  test:b      Runs project B unit tests.
  test:c      Runs project C unit tests.
  build:all  Compiles all projects.
  build:a    Compiles the A project.
  build:b    Compiles the B project.
  build:c    Compiles the C project.

It would be super helpful if mage's target import mechanism could do this all on a user's behalf by aggregating imported targets of the same name under a namespace (for example).

from mage.

Ooh, that is a super interesting idea. I'd been thinking about importing as package:function, but this is importing as function:package ... which makes total sense for something like build targets that are likely to be the same across a lot of different magefiles.

I think we'd need a way to configure it to choose whether to group by function name or package name.

Maybe something like

var MageImports = map[string]string{
     "proja" : "github.com/foo/bar/subdir", 
     "projb" : "github.com/foo/bar/subdir2",
}
var ImportByTarget=["Build", "Test"]

The above would then say, "Create namespaces named by the key of the MageImports map, and add targets for each target in the subdirectory (maybe except those specified in ImportByTarget, but maybe do include them?). For each target name in ImportByTarget, produce a namespace with the given name that has methods named by the keys in MageImports, which call the Build and Test targets for that import."

Which is a long way of describing what I hope would be obvious.

from mage.

@jonasrmichel I am trying to do the same thing! I don't suppose you have your current version around, do you?

from mage.