magefile / mage

a Make/rake-like dev tool using Go
https://magefile.org
Apache License 2.0
4.01k stars 250 forks source link

Add magefiles directory support #405

Closed perrito666 closed 2 years ago

perrito666 commented 2 years ago

If no -d option is passed to mage and there is a folder named magefolder then use that as -d.

The name was arbitrarily chosen and is up for debate.

I did not add this to the -init option because I assumed -d magefolder was not such a big pain to do in an infrequent task (unlike running mage targets which happen a lot).

This is a first attempt at addressing https://github.com/magefile/mage/issues/395 and one that is not really invasive but I am willing to grow this into a more complex feature.

I did not consider using non build tagged files as it is somehow useful to have the mage build tag to be able to exclude these. (As I understand the problem mostly arises from mage files living with other package files.)

natefinch commented 2 years ago

I think it would be beneficial to remove the build tag, since that requires setting it in the language server config. Ideally we'd support it being there or not, so that if you wanted, you could just pop your magefile from your root directory into a subfolder and not need to change anything.

Re: the name, I think magefiles might be better?

perrito666 commented 2 years ago

I think it would be beneficial to remove the build tag, since that requires setting it in the language server config. Ideally we'd support it being there or not, so that if you wanted, you could just pop your magefile from your root directory into a subfolder and not need to change anything.

I proposed a change then. This version accepts non-tagged files when inside a magefiles directory:

Caveats

Re: the name, I think magefiles might be better?

The change also calls this magefiles and for good measure swapped folder with directory in vars and doc so it is consistent with internal usage.

sheldonhull commented 2 years ago

I'm already storing files in a subfolder. Is the main point of this to eliminate any need for an outer mage.go style file that imports from the subdirectory. That would mean just run mage in the project root and it automatically would pull the magefiles packages?

I currently use:

project
- magefile.go
- magefiles/
---- taskpkg1
---- docker
---- kubernetes

Would it support this scenario? I'm not clear if the go listing command shown is recursive without adding ./....

perrito666 commented 2 years ago

I'm already storing files in a subfolder. Is the main point of this to eliminate any need for an outer mage.go style file that imports from the subdirectory. That would mean just run mage in the project root and it automatically would pull the magefiles packages?

Indeed, the main motivation here is that many go tools and editors dislike finding two packages in the same directory (as is often the case when one has a magefile in the main directory along with a different go file). This ends up leading to obscure editor configurations so mage files are treated as go files along with other files.

I currently use:

project
- magefile.go
- magefiles/
---- taskpkg1
---- docker
---- kubernetes

Would it support this scenario? I'm not clear if the go listing command shown is recursive without adding ./....

It is not recursive (I think) but you could put your magefile.go inside the magefiles folder and that would make all your files available pretty much in the same manner. As a side effect all your mage configuration would be constrained to that directory and subdirectories and your editor of choice could pick it without further configuration.

There is more discussion about how we reached this point in this issue.

karlskewes commented 2 years ago

This is great. Here are some experimentation results based on the conversation so far.

IDE support and no build tag

$ find .
.
./magefiles
./magefiles/magefile.go

$ ~/go/bin/mage 
Targets:
  printBase    

Neovim LSP works correctly like a regular Go file.

Finding targets from multiple files without build tag

$ find .
.
./magefiles
./magefiles/magefile.go
./magefiles/3.go
./magefiles/2.go
./magefiles/1.go

$ ~/go/bin/mage 
Targets:
  print1       
  print2       
  print3       
  printBase   

if we find tagged and untagged files in a magefiles folder, we pick the tagged ones

This was experienced as well. Remove tag from all and all found/usable.

No recursive lookups in magefiles directory

It is not recursive (I think)

Correct.

$ find .
.
./magefiles
./magefiles/1
./magefiles/1/magefile.go
./magefiles/2
./magefiles/2/magefile.go
./magefiles/3
./magefiles/3/magefile.go

$ ~/go/bin/mage 
No .go files marked with the mage build tag in this directory

mage -d <dir> requires build tagged files

This version accepts non-tagged files when inside a magefiles directory:

This was experienced and is per the test.

$ find .
.
./magefiles
./magefiles/1
./magefiles/1/magefile.go

$ ~/go/bin/mage -d magefiles/1
No .go files marked with the mage build tag in this directory.

$ sed -i '1 i\//+build mage\n' magefiles/1/magefile.go 

$ ~/go/bin/mage -d magefiles/1
Targets:
  print1  
natefinch commented 2 years ago

I think we're going to update it so that we use build tags in the magefiles directory as per normal. i.e., they're additive. So, we'll get the list of files by using the "mage" build tag, and just include all files both with and without it, as it works with normal go code.

This way, it won't matter if you used tags or not. The only thing it'll hinder is having two different packages in the same folder, which is one of the problems we're trying to fix with this change anyway.

perrito666 commented 2 years ago

I think we're going to update it so that we use build tags in the magefiles directory as per normal. i.e., they're additive. So, we'll get the list of files by using the "mage" build tag, and just include all files both with and without it, as it works with normal go code.

This way, it won't matter if you used tags or not. The only thing it'll hinder is having two different packages in the same folder, which is one of the problems we're trying to fix with this change anyway.

Done, when mixed tagging is found now it will just use all the files.