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

Do not use open64 #326

Merged
merged 1 commit into from
Jan 6, 2023
Merged

Do not use open64 #326

merged 1 commit into from
Jan 6, 2023

Conversation

kraj
Copy link
Contributor

@kraj kraj commented Dec 29, 2022

glibc is providing open64 and other lfs64 functions but musl aliases them to normal equivalents since off_t is always 64-bit on musl, therefore check for target env along when target OS is linux before using open64, this is more available. Latest Musl has made these namespace changes [1]

[1] https://git.musl-libc.org/cgit/musl/commit/?id=246f1c811448f37a44b41cd8df8d0ef9736d95f4

Signed-off-by: Khem Raj raj.khem@gmail.com

@briansmith
Copy link
Contributor

Why do we ever need to use open64 in the first place? Since we never seek it seems like we should just avoid the conditional logic.

@kraj
Copy link
Contributor Author

kraj commented Dec 30, 2022

Why do we ever need to use open64 in the first place? Since we never seek it seems like we should just avoid the conditional logic.

I am fine by using open, I was trying to fix just musl case, but if we think its better to use just open I can update the patch

@newpavlov
Copy link
Member

Why do we ever need to use open64 in the first place?

It's likely we have inherited it from std, but I agree that we probably do not need it. If @josephlr agrees, we should simply use open.

@briansmith
Copy link
Contributor

What does libstd do for musl targets?

@kraj
Copy link
Contributor Author

kraj commented Jan 3, 2023

What does libstd do for musl targets?

The change to drop/deprecate LFS64 functions in musl is recent and has not yet made it to a GA release.
I will be fixing it in libstd-rs as well see rust-lang/rust#106246

@josephlr
Copy link
Member

josephlr commented Jan 4, 2023

Why do we ever need to use open64 in the first place?

It's likely we have inherited it from std, but I agree that we probably do not need it. If @josephlr agrees, we should simply use open.

Looking through the various documentation, the only difference (if any) is the presence of O_LARGEFILE. That flag isn't relevant to us, as we are only opening a device (not a real file), so we never care about overflowing off_t.

Let's change this patch to just unconditionally use open.

@josephlr
Copy link
Member

josephlr commented Jan 4, 2023

If you rebase this PR onto the latest master, the tests should be passing.

@kraj
Copy link
Contributor Author

kraj commented Jan 6, 2023

Why do we ever need to use open64 in the first place?

It's likely we have inherited it from std, but I agree that we probably do not need it. If @josephlr agrees, we should simply use open.

Looking through the various documentation, the only difference (if any) is the presence of O_LARGEFILE. That flag isn't relevant to us, as we are only opening a device (not a real file), so we never care about overflowing off_t.

Let's change this patch to just unconditionally use open.

done

@kraj
Copy link
Contributor Author

kraj commented Jan 6, 2023

If you rebase this PR onto the latest master, the tests should be passing.

cool. Uploaded a new version of PR. Lets see

src/util_libc.rs Outdated Show resolved Hide resolved
glibc is providing open64 and other lfs64 functions but musl aliases
them to normal equivalents since off_t is always 64-bit on musl,
therefore check for target env along when target OS is linux before
using open64, this is more available. Latest Musl has made these
namespace changes [1]

There is no need for using LFS64 open explicitly as we are only using it
for opening device files and not real files

[1] https://git.musl-libc.org/cgit/musl/commit/?id=246f1c811448f37a44b41cd8df8d0ef9736d95f4

Signed-off-by: Khem Raj <raj.khem@gmail.com>
@newpavlov newpavlov changed the title Do not use open64 on linux with musl Do not use open64 Jan 6, 2023
@newpavlov newpavlov merged commit 7f73e3c into rust-random:master Jan 6, 2023
@newpavlov newpavlov mentioned this pull request Apr 2, 2023
orbea added a commit to orbea/gentoo that referenced this pull request May 28, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 1, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 5, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 5, 2023
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 5, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 8, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 8, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 9, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jun 29, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

At this point USE=system-boostrap will be required with >= musl-1.2.4
until uptream merges these patches and updates their boostrap.

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jul 16, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jul 17, 2023
This fixes the build with musl-1.2.4, for those that are still
experiencing issues these steps should resolve them:

1. Downgrade to musl-1.2.3
2. Rebuild dev-lang/rust with these patches
3. Upgrade to musl-1.2.4 again
4. Rebuild rust with USE=system-bootstrap

This was tested with musl-1.2.3, musl-1.2.4 and glibc-2.37-r3.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jul 20, 2023
This fixes the build with musl-1.2.4, may need -system-bootstrap after
updating musl.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-Commit: rust-lang/libc@1e8c55c
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
orbea added a commit to orbea/gentoo that referenced this pull request Jul 21, 2023
This fixes the build with musl-1.2.4, may need -system-bootstrap after
updating musl.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-Commit: rust-lang/libc@1e8c55c
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
jirutka added a commit to jirutka/deno that referenced this pull request Jul 23, 2023
This version fixes compatibility with musl libc
(rust-random/getrandom#326).
bartlomieju pushed a commit to denoland/deno that referenced this pull request Jul 23, 2023
This version fixes compatibility with musl libc
(rust-random/getrandom#326).
mmastrac pushed a commit to denoland/deno that referenced this pull request Jul 26, 2023
This version fixes compatibility with musl libc
(rust-random/getrandom#326).
orbea added a commit to orbea/gentoo that referenced this pull request Aug 7, 2023
This fixes the build with musl-1.2.4, may need -system-bootstrap after
updating musl.

Closes: https://bugs.gentoo.org/903607
Upstream-PR: rust-lang/rust#106246
Upstream-Issue: rust-lang/libc#2934
Upstream-PR: rust-lang/libc#2935
Upstream-Commit: rust-lang/libc@1e8c55c
Upstream-PR: rust-random/getrandom#326
Upstream-Commit: rust-random/getrandom@7f73e3c
Signed-off-by: orbea <orbea@riseup.net>
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.

4 participants