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

Add magefiles directory support #405

Merged
merged 12 commits into from
Mar 16, 2022

Conversation

perrito666
Copy link
Contributor

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 #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.)

If no directory was passed by the user as an explicit option and there is a folder named "magefolder" use that.

Workdir is kept as it is likely still "."
Also correct os.Stat error checking to expect no error
@natefinch
Copy link
Member

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?

Magefolder was renamed to magefiles
We now accept files that are not tagged too when using a magefiles directory
@perrito666 perrito666 changed the title Add magefolder support Add magefiles directory support Feb 17, 2022
When using magefiles directory, if there are mixed tagging files assume tagging is used for mage files
@perrito666
Copy link
Contributor Author

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

  • If you have a magefiles directory we will use it and asume it is for that
  • You cannot have mix tagging,if we find tagged and untagged files in a magefiles folder, we pick the tagged ones

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
Copy link

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
Copy link
Contributor Author

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
Copy link

karlskewes commented Feb 21, 2022

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  

We support building for older go versions so error formatting should use %v
@natefinch
Copy link
Member

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.

natefinch and others added 2 commits March 11, 2022 12:22
When mixed tagging is found within a magefiles folder, opt to use all files
@perrito666
Copy link
Contributor Author

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.

natefinch
natefinch previously approved these changes Mar 11, 2022
Copy link
Member

@natefinch natefinch left a comment

Choose a reason for hiding this comment

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

This looks great! Needs docs on the website, but otherwise, it's awesome.

Add a temporary preference for mage files over magefiles directories and warn users this is a temporary functionality leading to a change where directory will be preferred.
Copy link
Member

@natefinch natefinch left a comment

Choose a reason for hiding this comment

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

This looks great!

@natefinch natefinch merged commit 3504e09 into magefile:master Mar 16, 2022
scottames referenced this pull request in scottames/cmder Apr 22, 2023
….0 (#6)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/magefile/mage](https://github.com/magefile/mage) |
require | minor | `v1.11.0` -> `v1.14.0` |

---

### Release Notes

<details>
<summary>magefile/mage</summary>

###
[`v1.14.0`](https://github.com/magefile/mage/releases/tag/v1.14.0): -
Faster Than Ever

[Compare
Source](https://github.com/magefile/mage/compare/v1.13.0...v1.14.0)

#### What's Changed

- give props to netlify on the main page by
[@&#8203;natefinch](https://github.com/natefinch) in
[https://github.com/magefile/mage/pull/410](https://github.com/magefile/mage/pull/410)
- Update site to build with newer hugo by
[@&#8203;perrito666](https://github.com/perrito666) in
[https://github.com/magefile/mage/pull/411](https://github.com/magefile/mage/pull/411)
- bring docs for use of directives up to date by
[@&#8203;efd6](https://github.com/efd6) in
[https://github.com/magefile/mage/pull/407](https://github.com/magefile/mage/pull/407)
- add blog post about v1.13 by
[@&#8203;natefinch](https://github.com/natefinch) in
[https://github.com/magefile/mage/pull/412](https://github.com/magefile/mage/pull/412)
- fix author links by
[@&#8203;natefinch](https://github.com/natefinch) in
[https://github.com/magefile/mage/pull/413](https://github.com/magefile/mage/pull/413)
- Add variadic support to mg.F by
[@&#8203;perj](https://github.com/perj) in
[https://github.com/magefile/mage/pull/402](https://github.com/magefile/mage/pull/402)
- feat: rename templated imports to avoid collisions by
[@&#8203;ladydascalie](https://github.com/ladydascalie) in
[https://github.com/magefile/mage/pull/421](https://github.com/magefile/mage/pull/421)
- Website: fixing typos by
[@&#8203;deining](https://github.com/deining) in
[https://github.com/magefile/mage/pull/429](https://github.com/magefile/mage/pull/429)
- refactor(listGoFiles): remove go list dependency by
[@&#8203;jaredallard](https://github.com/jaredallard) in
[https://github.com/magefile/mage/pull/440](https://github.com/magefile/mage/pull/440)
- doc: add MacPorts install info by
[@&#8203;herbygillot](https://github.com/herbygillot) in
[https://github.com/magefile/mage/pull/419](https://github.com/magefile/mage/pull/419)

#### New Contributors

- [@&#8203;efd6](https://github.com/efd6) made their first
contribution in
[https://github.com/magefile/mage/pull/407](https://github.com/magefile/mage/pull/407)
- [@&#8203;perj](https://github.com/perj) made their first
contribution in
[https://github.com/magefile/mage/pull/402](https://github.com/magefile/mage/pull/402)
- [@&#8203;ladydascalie](https://github.com/ladydascalie) made their
first contribution in
[https://github.com/magefile/mage/pull/421](https://github.com/magefile/mage/pull/421)
- [@&#8203;deining](https://github.com/deining) made their first
contribution in
[https://github.com/magefile/mage/pull/429](https://github.com/magefile/mage/pull/429)
- [@&#8203;jaredallard](https://github.com/jaredallard) made their
first contribution in
[https://github.com/magefile/mage/pull/440](https://github.com/magefile/mage/pull/440)
- [@&#8203;herbygillot](https://github.com/herbygillot) made their
first contribution in
[https://github.com/magefile/mage/pull/419](https://github.com/magefile/mage/pull/419)

**Full Changelog**:
magefile/mage@v1.13.0...v1.14.0

###
[`v1.13.0`](https://github.com/magefile/mage/releases/tag/v1.13.0): -
Magefiles Directory and more!

[Compare
Source](https://github.com/magefile/mage/compare/v1.12.1...v1.13.0)

#### What's Changed

- feat: allow mage:import alias to be defined for multiple imports by
[@&#8203;viktorvoltaire](https://github.com/viktorvoltaire) in
[https://github.com/magefile/mage/pull/398](https://github.com/magefile/mage/pull/398)
- chore: wrap log.Println in cmd with verbose check by
[@&#8203;viktorvoltaire](https://github.com/viktorvoltaire) in
[https://github.com/magefile/mage/pull/399](https://github.com/magefile/mage/pull/399)
- Detect deps that have already been invoked by
[@&#8203;carolynvs](https://github.com/carolynvs) in
[https://github.com/magefile/mage/pull/346](https://github.com/magefile/mage/pull/346)
- Replace godoc.org URLs by
[@&#8203;JamieEdge](https://github.com/JamieEdge) in
[https://github.com/magefile/mage/pull/342](https://github.com/magefile/mage/pull/342)
- Add magefiles directory support by
[@&#8203;perrito666](https://github.com/perrito666) in
[https://github.com/magefile/mage/pull/405](https://github.com/magefile/mage/pull/405)

#### New Contributors

- [@&#8203;viktorvoltaire](https://github.com/viktorvoltaire) made
their first contribution in
[https://github.com/magefile/mage/pull/398](https://github.com/magefile/mage/pull/398)
- [@&#8203;carolynvs](https://github.com/carolynvs) made their first
contribution in
[https://github.com/magefile/mage/pull/346](https://github.com/magefile/mage/pull/346)
- [@&#8203;JamieEdge](https://github.com/JamieEdge) made their first
contribution in
[https://github.com/magefile/mage/pull/342](https://github.com/magefile/mage/pull/342)
- [@&#8203;perrito666](https://github.com/perrito666) made their first
contribution in
[https://github.com/magefile/mage/pull/405](https://github.com/magefile/mage/pull/405)

**Full Changelog**:
magefile/mage@v1.12.1...v1.13.0

###
[`v1.12.1`](https://github.com/magefile/mage/releases/tag/v1.12.1): -
Second Verse, Same as the First

[Compare
Source](https://github.com/magefile/mage/compare/v1.12.0...v1.12.1)

This is a copy of v1.12.0 ... nothing has changed. However, there was an
initial v1.12.0 that was created accidentally, and then deleted, and
it's causing some go proxies to complain. So hopefully this will fix
that.

#### Changelog

- [`2f1ec40`](https://github.com/magefile/mage/commit/2f1ec40) ci:
migrate from travis to github action
([#&#8203;391](https://github.com/magefile/mage/issues/391))
- [`fe9f942`](https://github.com/magefile/mage/commit/fe9f942)
evidently goreleaseer has changed in the last 4 years :)
([#&#8203;394](https://github.com/magefile/mage/issues/394))
- [`fd5011e`](https://github.com/magefile/mage/commit/fd5011e) Fix the
rollback mechanism for tags during a release
([#&#8203;392](https://github.com/magefile/mage/issues/392))
- [`404c119`](https://github.com/magefile/mage/commit/404c119)
sh.run(): quoted strings before join
([#&#8203;306](https://github.com/magefile/mage/issues/306))
- [`0c5affe`](https://github.com/magefile/mage/commit/0c5affe) Add
asdf installation instructions to docs
([#&#8203;383](https://github.com/magefile/mage/issues/383))
- [`e84bbc1`](https://github.com/magefile/mage/commit/e84bbc1)
[#&#8203;288](https://github.com/magefile/mage/issues/288) add brew
and scoop install to docs
([#&#8203;376](https://github.com/magefile/mage/issues/376))
- [`80953f7`](https://github.com/magefile/mage/commit/80953f7)
[#&#8203;378](https://github.com/magefile/mage/issues/378) bump
travisci go16
([#&#8203;379](https://github.com/magefile/mage/issues/379))
- [`dd94424`](https://github.com/magefile/mage/commit/dd94424) Create
issue templates
([#&#8203;374](https://github.com/magefile/mage/issues/374))
- [`d9e2e41`](https://github.com/magefile/mage/commit/d9e2e41) fix:
deterministic compiled mainfile
([#&#8203;348](https://github.com/magefile/mage/issues/348))
- [`4cf3cfc`](https://github.com/magefile/mage/commit/4cf3cfc) make -h
work with imported targets
([#&#8203;335](https://github.com/magefile/mage/issues/335))
- [`de7ca6c`](https://github.com/magefile/mage/commit/de7ca6c) fix
test for go 1.16
([#&#8203;330](https://github.com/magefile/mage/issues/330))

###
[`v1.12.0`](https://github.com/magefile/mage/releases/tag/v1.12.0): -
Small Fixes

[Compare
Source](https://github.com/magefile/mage/compare/v1.11.0...v1.12.0)

This is our first release in a while, and nothing major is added, but
some small fixes have gone out, like making the mainfile deterministic
and making sure we can use -h with imported targets.

#### Changelog

- [`2f1ec40`](https://github.com/magefile/mage/commit/2f1ec40) ci:
migrate from travis to github action
([#&#8203;391](https://github.com/magefile/mage/issues/391))
- [`fe9f942`](https://github.com/magefile/mage/commit/fe9f942)
evidently goreleaseer has changed in the last 4 years :)
([#&#8203;394](https://github.com/magefile/mage/issues/394))
- [`fd5011e`](https://github.com/magefile/mage/commit/fd5011e) Fix the
rollback mechanism for tags during a release
([#&#8203;392](https://github.com/magefile/mage/issues/392))
- [`404c119`](https://github.com/magefile/mage/commit/404c119)
sh.run(): quoted strings before join
([#&#8203;306](https://github.com/magefile/mage/issues/306))
- [`0c5affe`](https://github.com/magefile/mage/commit/0c5affe) Add
asdf installation instructions to docs
([#&#8203;383](https://github.com/magefile/mage/issues/383))
- [`e84bbc1`](https://github.com/magefile/mage/commit/e84bbc1)
[#&#8203;288](https://github.com/magefile/mage/issues/288) add brew
and scoop install to docs
([#&#8203;376](https://github.com/magefile/mage/issues/376))
- [`80953f7`](https://github.com/magefile/mage/commit/80953f7)
[#&#8203;378](https://github.com/magefile/mage/issues/378) bump
travisci go16
([#&#8203;379](https://github.com/magefile/mage/issues/379))
- [`dd94424`](https://github.com/magefile/mage/commit/dd94424) Create
issue templates
([#&#8203;374](https://github.com/magefile/mage/issues/374))
- [`d9e2e41`](https://github.com/magefile/mage/commit/d9e2e41) fix:
deterministic compiled mainfile
([#&#8203;348](https://github.com/magefile/mage/issues/348))
- [`4cf3cfc`](https://github.com/magefile/mage/commit/4cf3cfc) make -h
work with imported targets
([#&#8203;335](https://github.com/magefile/mage/issues/335))
- [`de7ca6c`](https://github.com/magefile/mage/commit/de7ca6c) fix
test for go 1.16
([#&#8203;330](https://github.com/magefile/mage/issues/330))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/scottames/cmder).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS41Ny4wIiwidXBkYXRlZEluVmVyIjoiMzUuNTcuMCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

4 participants