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

On the use std::fs::ReadDir.next() #177

Closed
n-eq opened this issue Aug 8, 2022 · 3 comments
Closed

On the use std::fs::ReadDir.next() #177

n-eq opened this issue Aug 8, 2022 · 3 comments

Comments

@n-eq
Copy link

n-eq commented Aug 8, 2022

Hello,

This is more of an open question but I was wondering about the use of std::fs::ReadDir.next() to determine the paths to Android tools binaries.

In my case, it turned out that running a test failed on my Android phone, throwing the following error:
[2022-08-08T07:56:49Z ERROR cargo_dinghy] Not a directory (os error 20).

Further investigation led me to this line:

And it turns out the error comes from the fact that next() took .DS_Store directory instead of the expected bin.
I noticed the same pattern is used in other parts of the project (at least in the same file referenced above).

Although this is not blocking behavior - I managed to figure out quickly and fix it up -, I was wondering if there are good reasons to use next() in this context instead of trying to find the path that is expected, which, in this case, would probably be possible with a combination of joins and exists.

Thanks!
Nabil

@kali
Copy link
Collaborator

kali commented Aug 8, 2022

Hey, thanks for your interest in Dinghy, and sorry for this issue.

I think the reason why we used this trick is to avoid making an assumption on the next token in the path: it's the build host triple, so it is variable across platforms. Not super hard to deal with, but I guess we got lazy.

I'm not sure if we want to hard-code it. With the Apple platforms in-between two architectures at this time, we may very well have one or the other on Apple silicon. Another approach could be to iterate till we find a directory...

@fredszaq
Copy link
Collaborator

fredszaq commented Aug 9, 2022

The name of the folder is indeed dependent on the host system and is different on linux windows and macOS, I made a fix ensuring we only look at directories in this case.

@fredszaq
Copy link
Collaborator

fredszaq commented Aug 9, 2022

fix released in 0.6.2

@fredszaq fredszaq closed this as completed Aug 9, 2022
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

No branches or pull requests

3 participants