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

firebase.json: functions.ignore not including deeper directories #8131

Open
rhodgkins opened this issue Jan 23, 2025 · 2 comments · May be fixed by #8132
Open

firebase.json: functions.ignore not including deeper directories #8131

rhodgkins opened this issue Jan 23, 2025 · 2 comments · May be fixed by #8132

Comments

@rhodgkins
Copy link
Contributor

[REQUIRED] Environment info

firebase-tools: 13.29.1

Platform: macOS

[REQUIRED] Test case

See simple setup here https://github.com/rhodgkins/firebase-tools-functions-ignore

I have a directory named deployment at the root inside the functions.source directory, and also another directory deployment in the foobar directory at the root. I want the root deployment directory to not be uploaded with the functions.

  • functions/
    • deployment/ - ignore this whole directory
    • foobar/
      • deployment/ - want this directory (and everything inside it) included

[REQUIRED] Steps to reproduce

Deploy the functions:

firebase --project=XXX deploy --only=functions

Found this due to having a deployment directory (with some config files not related to the function runtime execution) and another sub directory called deployment which was named after some function functionality. Couldn't work out why the directory wasn't being uploaded.
I've tried using deployment/* in the ignore instead but then the root deployment directory gets uploaded.

I think it's mainly due to how paths are filtered here:

const fullPaths = dirContents.map((n) => join(options.path, n));

It shouldn't include base sourceDir and then the minimatch filters will work correctly with deployment/*.

Going to attempt a fix, or at least start off with a PR with a failing test case for readdirRecursive

[REQUIRED] Expected behavior

For functions/foobar/deployment directory to be uploaded, but functions/deployment to not be included.

[REQUIRED] Actual behavior

functions/foobar/deployment and everything in it, is excluded.

@rhodgkins rhodgkins changed the title firebase.json: functions.ignore including deeper directories firebase.json: functions.ignore not including deeper directories Jan 23, 2025
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 23, 2025
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 23, 2025
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 23, 2025
@rhodgkins rhodgkins linked a pull request Jan 23, 2025 that will close this issue
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 23, 2025
@aalej
Copy link
Contributor

aalej commented Jan 24, 2025

Thanks for the detailed report and for providing a test case, @rhodgkins! I’m able to reproduce the behavior you mentioned. It looks like you have a fix for this ready in #8132. I think it would be better to include the fix along with the tests so that our engineers can verify and review the changes in the fix. I’ll also raise this to our engineering team to get their feedback.

A not so elegant workaround I can see here(since sourceDir is included in the path) is to use a pattern like **/functions/deployment/** which would match with <PATH>/functions/deployment but not <PATH>/functions/foobar/deployment.

rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 24, 2025
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 24, 2025
rhodgkins added a commit to bookcreator/firebase-tools that referenced this issue Jan 24, 2025
@rhodgkins
Copy link
Contributor Author

Thanks for getting back to me @aalej - fix pushed

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

Successfully merging a pull request may close this issue.

2 participants