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

fix(cli): --ignore getting ignored #9071

Merged
merged 7 commits into from
Apr 25, 2024
Merged

fix(cli): --ignore getting ignored #9071

merged 7 commits into from
Apr 25, 2024

Conversation

mertalev
Copy link
Contributor

Description

The internal typing for this flag expects exclusionPatterns, so it will always consider it to be undefined.

How Has This Been Tested?

Tested that it actually ignores the pattern specified in the command.

@mertalev mertalev added the cli Tasks related to the Immich CLI label Apr 25, 2024
@mertalev mertalev requested a review from jrasm91 April 25, 2024 01:55
Copy link

cloudflare-pages bot commented Apr 25, 2024

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0ece226
Status: ✅  Deploy successful!
Preview URL: https://2e9f870f.immich.pages.dev
Branch Preview URL: https://fix-cli-ignore.immich.pages.dev

View logs

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Can you add an e2e test for this?

@mertalev
Copy link
Contributor Author

It turns out there was more than one issue here:

  • --ignore has confusing behavior: it's sensitive to the order of the flags (particularly whether it comes before or after the upload path(s)) because it can take multiple arguments. The behavior is predictable if it can only accept one string.
  • The behavior is different when a search pattern has ..: it ignores the --ignore flag in this case. Resolving the path fixes this.
  • Lastly, the glob library still failed to ignore files even with the above changes. fast-glob does behave as expected. As a nice bonus, it seems something else depends on fast-glob anyway so replacing glob with it shrunk the build by about 65kb.

@mertalev mertalev merged commit c14a2ed into main Apr 25, 2024
25 checks passed
@mertalev mertalev deleted the fix/cli-ignore branch April 25, 2024 14:48
@TransRapid
Copy link

immich upload -n -i /Recently/ -r /media/Vault/Apple\ iCloud\ Photos/7/iCloud\ Photos\ Part\ 68\ of\ 68/
Crawling for assets...
Checking files | ████████████████████████████████████████ | 100% | ETA: 0s | 2/2 assets
Found 2 new files and 0 duplicates
Would have uploaded 2 assets (9.5 MB)

Expected behavior would be 0 assets as the only 2 things are in a folder titled. Recently Deleted

@danieldietzler
Copy link
Member

The CLI does not randomly ignore folders because of their name. And IMO that's a good thing, in case that's what you're referring to @TransRapid

@TransRapid
Copy link

TransRapid commented Apr 30, 2024

No, the issue is the --ignore just does not work as described . The ignore pattern does not ignore using the CLI, it does however work using the GUI, that is the more unusual part.

@mertalev
Copy link
Contributor Author

Can you confirm that you're actually using a CLI build that has the fixes in this PR? It isn't in the latest CLI release.

@macross5
Copy link

macross5 commented May 24, 2024

Do we know when the fix will be released?

For me I did a script with a "find" workaround to avoid the unwanted directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Tasks related to the Immich CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants