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

macos build issue resolved #59

Merged
merged 6 commits into from
Dec 19, 2024
Merged

macos build issue resolved #59

merged 6 commits into from
Dec 19, 2024

Conversation

KMJ-007
Copy link
Contributor

@KMJ-007 KMJ-007 commented Dec 7, 2024

closes #58

image

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 7, 2024

had question,

you are using nix crate, what benefits so far have you came across, and what made you choose it over syscall crate?

how did you decide?

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 7, 2024

and current type conversion is working on macos, i haven't checked on linux and other system,

if you have any other method or safer method in your mind, let me know

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 7, 2024

why CI didn't run?

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 8, 2024

@mihaigalos can you review this in your free time?

@mihaigalos
Copy link
Owner

Hi @KMJ-007, seems you have some clippy errors. Since the code is still in your fork, can you please also fix them?

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 10, 2024

Hi @KMJ-007, seems you have some clippy errors. Since the code is still in your fork, can you please also fix them?

I tried but it still says useless type conversion, but without those on macos it fails to compile cause some statfs function on macos were returning u32

what should i do?

@mihaigalos
Copy link
Owner

mihaigalos commented Dec 17, 2024

Sorry for replying so slow, I was very busy at work.
Ok, so we need some kind of way to tell rust to compile differently for different architectures, right?

Have a look at this, I'm open to new suggestions as well:

https://github.com/mihaigalos/url-install/blob/507a70f1d6f6e19ae6148bfffe02f374b5b6f636/src/url_install.rs#L66

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 19, 2024

Sorry for replying so slow, I was very busy at work. Ok, so we need some kind of way to tell rust to compile differently for different architectures, right?

Have a look at this, I'm open to new suggestions as well:

https://github.com/mihaigalos/url-install/blob/507a70f1d6f6e19ae6148bfffe02f374b5b6f636/src/url_install.rs#L66

done, but i wanted more simpler solution, but at the time i didn't find that, so added target os specific code,

can you review it, in my for clippy action is green

@mihaigalos
Copy link
Owner

This is awesome. We can disregard coverage checks, I think it's something unrelated why they fail.
Thank you.

@mihaigalos mihaigalos merged commit 99c0e9a into mihaigalos:main Dec 19, 2024
2 of 3 checks passed
@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Dec 20, 2024

had question,

you are using nix crate, what benefits so far have you came across, and what made you choose it over syscall crate?

how did you decide?

i am still interested in this question

@mihaigalos
Copy link
Owner

No benefits whatsoever. There is no additional sycall I need to perform and the structure is already populated with nice data instead of needing a syscall for every field.

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.

Not able to install dusage
2 participants