Skip to content

fix(utils): fix resources map becomes directory - #10293

Merged
lucasfernog merged 9 commits into
devfrom
fix/mapped-resources
Aug 17, 2024
Merged

lucasfernog merged 9 commits into
devfrom
fix/mapped-resources

Conversation

@amrbashir

Copy link
Copy Markdown
Member

closes #10187

Fixes the behavior of mapped resources generating extra directory, for example: "../resources/user.json": "resources/user.json" generates this resource resources/user.json/user.json where it should generate resources/user.json

This PR includes a refactor of the Iterator implementation which splits it into more scoped functions and relis on recursing instead of a loop which makes the code a lot more readable and easier to maintain.

closes #10187

Fixes the behavior of mapped resources generating extra directory, for example:
`"../resources/user.json": "resources/user.json"` generates this resource `resources/user.json/user.json`
where it should generate `resources/user.json`

This PR includes a refactor of the Iterator implementation which splits it into more scoped functions and relis on recursing instead of a loop which makes the code a lot more readable and easier to maintain.
@amrbashir
amrbashir requested a review from a team as a code owner July 15, 2024 19:01
@github-actions

github-actions Bot commented Jul 15, 2024 •

Copy link
Copy Markdown
Contributor

Package Changes Through 4dfd836

There are 11 changes which include tauri-cli with prerelease, @tauri-apps/cli with prerelease, tauri-build with prepatch, tauri-codegen with prerelease, tauri-utils with prerelease, tauri-bundler with prerelease, tauri with prerelease, tauri-macos-sign with prepatch, @tauri-apps/api with prepatch, tauri-runtime with prepatch, tauri-runtime-wry with prepatch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
@tauri-apps/api 2.0.0-rc.0 2.0.1-rc.0
tauri-utils 2.0.0-rc.2 2.0.0-rc.3
tauri-macos-sign 0.1.0-beta.0 0.1.1-rc.0
tauri-bundler 2.0.1-rc.1 2.0.1-rc.2
tauri-runtime 2.0.0-rc.2 2.0.1-rc.0
tauri-runtime-wry 2.0.0-rc.2 2.0.1-rc.0
tauri-codegen 2.0.0-rc.2 2.0.0-rc.3
tauri-macros 2.0.0-rc.2 2.0.0-rc.3
tauri-plugin 2.0.0-rc.2 2.0.0-rc.3
tauri-build 2.0.0-rc.2 2.0.1-rc.0
tauri 2.0.0-rc.2 2.0.0-rc.3
@tauri-apps/cli 2.0.0-rc.3 2.0.0-rc.4
tauri-cli 2.0.0-rc.3 2.0.0-rc.4

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@pewsheen

Copy link
Copy Markdown
Contributor

I tested with some folder cases, and it seems to have some differences from the past version when the source has folders inside a folder or the destination is a folder:

src/
├─ assets/
│  ├─ lang/
│  │  ├─ de.json
│  ├─ tauri.svg

Source: Folder, Dest: Folder

{
  "resources": {
    "assets/": "dist/"
  }  
}
# before
Resource { path: "assets/tauri.svg", target: "dist/tauri.svg" }
Resource { path: "assets/lang/de.json", target: "dist/lang/de.json" }
# now
Resource { path: "assets/tauri.svg", target: "dist/" }
Resource { path: "assets/lang/de.json", target: "dist/" }

Source: Folder, Dest: Root

{
  "resources": {
    "assets/": ""
  }  
}
# before
Resource { path: "assets/tauri.svg", target: "tauri.svg" }
Resource { path: "assets/lang/de.json", target: "lang/de.json" }
# now
Resource { path: "assets/tauri.svg", target: "tauri.svg" }
Resource { path: "assets/lang/de.json", target: "de.json" }

Source: Glob, Dest: Folder

{
  "resources": {
    "assets/*": "dist/"
  }  
}
# before
Resource { path: "assets/tauri.svg", target: "dist/tauri.svg" }
Resource { path: "assets/lang/de.json", target: "dist/lang/de.json" }
# now
Resource { path: "assets/tauri.svg", target: "dist/" }
Resource { path: "assets/lang/de.json", target: "dist/" }

Source: Glob, Dest: Root (this one is correct)

{
  "resources": {
    "assets/*": ""
  }  
}
# before
Resource { path: "assets/tauri.svg", target: "tauri.svg" }
Resource { path: "assets/lang/de.json", target: "de.json" }
# now
Resource { path: "assets/tauri.svg", target: "tauri.svg" }
Resource { path: "assets/lang/de.json", target: "de.json" }

@amrbashir

amrbashir commented Jul 17, 2024 •

Copy link
Copy Markdown
Member Author

Thanks @pewsheen for extensive tests, I will fix the Source: Folder, Dest: Folder or Root cases.

However Source: Glob, Dest: Folder is basically the same as Source: Glob, Dest: Root and so they should act the same, and so I consider Source: Glob, Dest: Folder got fixed from previous behavior.

@amrbashir

amrbashir commented Jul 17, 2024 •

Copy link
Copy Markdown
Member Author

I just noticed that Source: Glob, Dest: Folder is just plain broken, it should be

# before
Resource { path: "assets/tauri.svg", target: "dist/tauri.svg" }
Resource { path: "assets/lang/de.json", target: "dist/lang/de.json" }
# now
Resource { path: "assets/tauri.svg", target: "dist/tauri.svg" }
Resource { path: "assets/lang/de.json", target: "dist/de.json" }

which is a different behavior than before but consistent with Source Glob, Dest: Root

@amrbashir

Copy link
Copy Markdown
Member Author

@pewsheen all should be good now, please give it another test

@pewsheen

pewsheen commented Jul 17, 2024 •

Copy link
Copy Markdown
Contributor

This one is weird

("../src/sounds/*".into(), "voices".into()),
Resource {
    path: "../src/sounds/lang/fr.wav",
    target: "voices/../src/sounds/lang/fr.wav",
},
Resource {
    path: "../src/sounds/lang/es.wav",
    target: "voices/../src/sounds/lang/es.wav",
},

And if the the glob path is not valid, it'll remain in the glob format

Resource {
    path: "not/exists/path/*",
    target: "voices",
},

Maybe we should check the glob path is valid or return crate::Error::GlobPathNotFound

@amrbashir

Copy link
Copy Markdown
Member Author

@pewsheen these two cases should be fixed now, let me know if you find any more bugs

@pewsheen pewsheen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@amrbashir
amrbashir requested a review from lucasfernog August 14, 2024 12:57
Comment thread .changes/resources-map-becoming-dirs.md Outdated

@lucasfernog lucasfernog left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I opened this PR considering writing tests to validate this, but you already did it ❤️

@lucasfernog
lucasfernog merged commit 9e89193 into dev Aug 17, 2024
@lucasfernog
lucasfernog deleted the fix/mapped-resources branch August 17, 2024 02:03
@Raicuparta

Raicuparta commented Sep 3, 2024 •

Copy link
Copy Markdown

I think this PR is probably what broke the resources setting for me. I have a resources folder that has several other folders inside it, and those folders have a bunch of files at different levels. But the root resources folder doesn't have any files itself. My tauri config had "resources": ["resources/*"], and that stopped working, with the error resource path resources\* doesn't exist. I had to change my setting to "resources": ["resources/**/*"] to get it working again.

I should probably make a separate issue for it, but don't have the time to make it detailed enough. And sorry if this is unrelated (very possible)

@FabianLars

Copy link
Copy Markdown
Member

@Raicuparta

I should probably make a separate issue for it, but don't have the time to make it detailed enough

What you wrote here is already detailed enough for an initial issue. :)

@amrbashir

Copy link
Copy Markdown
Member Author

@Raicuparta I believe "resources": ["resources/**/*"] should be the correct behavior, and "resources": ["resources/*"] should only match files directly under resources dir. This is how globbing behaves in bash as well.

However I think we may need to improve the error.

@Raicuparta

Copy link
Copy Markdown

That makes sense. I guess the error could specifically mention globs when those are used to make it more clear. But maybe the breaking change could be documented in V2 migration docs. Unless it's considered a bug fix because the previous version wasn't following globbing expectations.

@amrbashir

amrbashir commented Sep 3, 2024 •

Copy link
Copy Markdown
Member Author

I will open a PR to improve the error a bit, However for ./resources/* it will no longer print any errors since the glob is correct but it just couldn't find any files there which is the correct behavior here.

The migration docs will be updated.

amrbashir added a commit that referenced this pull request Sep 3, 2024
* fix(resources): enhance error messages for globs

ref: #10293 (comment)

* fmt
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.

[bug]: resources map unintuitively becomes directory

5 participants