Skip to content

APWorld Builder: Add .apignore format to not include files in build #5779

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.

Sign up for GitHub

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

Jump to bottom
Open
duckboycool wants to merge 10 commits into
base: main
Choose a base branch
from
Open

APWorld Builder: Add .apignore format to not include files in build #5779

duckboycool wants to merge 10 commits into from

Conversation

@duckboycool
Copy link
Collaborator

@duckboycool duckboycool commented Dec 21, 2025 •
edited
Loading

What is this fixing or adding?

Gives more flexibility to the Build APWorlds tool by allowing a .gitignore-semantics .apignore file to be included in the world, which the build will not add to the .apworld archive. Hopefully this will allow more people to move to using this for their build process since some people have cited the lack of this functionality as a blocker before. Also replaces the current exclusions with a GLOBAL.apignore file in data that could be easily edited in the future.

How was this tested?

Running the build component with a single world and checking output, both from the global ignores and also adding an ignore to its world folder. Running the build on all worlds, which seems to have taken around the same time as before.

If this makes graphical changes, please attach screenshots.

🚫👀

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 21, 2025
duckboycool
duckboycool commented Dec 21, 2025
View reviewed changes
@duckboycool
Edit apworld docs with current ignore behavior
a503c21 
Details could change here slightly, but probably good to get an outline.
FlitPix
FlitPix approved these changes Dec 21, 2025
View reviewed changes
Copy link
Contributor

@FlitPix FlitPix left a comment

Choose a reason for hiding this comment

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

tested, works well! /.git/ doesn't seem to work, but i'm aware my repo is set up in a way that isn't recommended so still approving :)

data/GLOBAL.apignore
Copy link
Contributor

@FlitPix FlitPix Dec 21, 2025

Choose a reason for hiding this comment

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

maybe add .gitignore and .git here too? there's little to no reason for it to be in a built apworld, at least not that i can think of.

Copy link
Collaborator Author

@duckboycool duckboycool Dec 21, 2025

Choose a reason for hiding this comment

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

They could make sense yeah, I had excluded those locally before with an edit to the component since I used a submodule. Although I'm a bit confused on the "/.git/ doesn't seem to work". Maybe you have a .git file instead (which seems to be the case for submodules), in which case it should be /.git instead since the trailing slash makes it only apply to directories.

Copy link
Contributor

@FlitPix FlitPix Dec 21, 2025

Choose a reason for hiding this comment

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

ahh you're right, ignore that part then!

Copy link
Collaborator

@silasary silasary Dec 24, 2025

Choose a reason for hiding this comment

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

I do agree that /.git and /.github/ should absolutely be in the default ignore

FlitPix added a commit to FlitPix/ap-bluesphere that referenced this pull request Dec 24, 2025
@FlitPix
Add .apignore file
cf87475 
ArchipelagoMW/Archipelago#5779 adds a .apignore format to exclude certain files from being included in built APWorlds.
This commit adds a .apignore file to exclude git-related files from release artifacts.

__pycache__ is already excluded globally, so no need to include it.
@FlitPix FlitPix mentioned this pull request Dec 24, 2025
Open
silasary
silasary approved these changes Dec 24, 2025
View reviewed changes
Copy link
Collaborator

@silasary silasary left a comment

Choose a reason for hiding this comment

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

I'd rather lean on pathspec as a tried and tested library, rather than rolling out own hard to maintain regex monstrosity.

But this does work.

benny-dreamly
benny-dreamly approved these changes Dec 24, 2025
View reviewed changes
Copy link
Contributor

@benny-dreamly benny-dreamly left a comment

Choose a reason for hiding this comment

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

I like this

@duckboycool
Copy link
Collaborator Author

duckboycool commented Dec 24, 2025

I'm gonna mark this as draft for the moment since using a library seems to also be what was wanted by core from the discord, so it'd be good for people to test once that's implemented.

@duckboycool duckboycool marked this pull request as draft December 24, 2025 05:06
@github-actions github-actions bot removed the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Dec 24, 2025
@duckboycool duckboycool marked this pull request as ready for review December 25, 2025 01:16
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Dec 25, 2025
@duckboycool
Copy link
Collaborator Author

duckboycool commented Dec 25, 2025

Changed over to pathspec now. It seemed to work the same way from my testing, but others trying as well would be good.

@duckboycool duckboycool added the is: enhancement Issues requesting new features or pull requests implementing new features. label Dec 26, 2025
silasary
silasary approved these changes Dec 30, 2025
View reviewed changes
Copy link
Collaborator

@silasary silasary left a comment

Choose a reason for hiding this comment

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

Tested the new version, it all works perfectly

@duckboycool duckboycool added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 30, 2025
FlitPix
FlitPix approved these changes Dec 30, 2025
View reviewed changes
Copy link
Contributor

@FlitPix FlitPix left a comment

Choose a reason for hiding this comment

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

re-approving new changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@silasary silasary silasary approved these changes

@FlitPix FlitPix FlitPix approved these changes

@benny-dreamly benny-dreamly benny-dreamly approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@duckboycool @silasary @FlitPix @benny-dreamly