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

fixing illumos build for couple of structs #2726

Merged

Conversation

devnexen
Copy link
Contributor

No description provided.

@rust-highfive
Copy link

Some changes occurred in solarish module

cc @jclulow,@pfmooney

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@@ -3125,104 +3125,6 @@ extern "C" {
pub fn sysinfo(command: ::c_int, buf: *mut ::c_char, count: ::c_long) -> ::c_int;
}

#[link(name = "sendfile")]
extern "C" {
pub fn sendfile(out_fd: ::c_int, in_fd: ::c_int, off: *mut ::off_t, len: ::size_t)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you can just remove things?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rust-lang/rust has an issue updating their dependencies to libc due to this (they build their solaris/illumos target on docker).

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, deleting it is not the right answer. We should fix whatever the problem is. What's the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the link. As per this comment:

rust-lang/rust#94052 (comment)

I think we can leave the libsendfile and liblgrp stuff in here, and just fix the other build errors. The Solaris target CI issue should be fixed in the program that produces the build environment there, which it seems is just missing these libraries: https://github.com/rust-lang/rust/blob/83460d5e624e9dff72ea8c8f6e79c10af10a3aa1/src/ci/docker/host-x86_64/dist-various-2/build-solaris-toolchain.sh#L35-L44

Comment on lines 3128 to 3225
#[link(name = "sendfile")]
extern "C" {
pub fn sendfile(out_fd: ::c_int, in_fd: ::c_int, off: *mut ::off_t, len: ::size_t)
-> ::ssize_t;
pub fn sendfilev(
fildes: ::c_int,
vec: *const sendfilevec_t,
sfvcnt: ::c_int,
xferred: *mut ::size_t,
) -> ::ssize_t;
// #include <link.h>
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn dl_iterate_phdr(
callback: ::Option<
unsafe extern "C" fn(
info: *mut dl_phdr_info,
size: usize,
data: *mut ::c_void,
) -> ::c_int,
>,
data: *mut ::c_void,
) -> ::c_int;
pub fn getpagesize() -> ::c_int;
pub fn getpagesizes(pagesize: *mut ::size_t, nelem: ::c_int) -> ::c_int;
pub fn mmapobj(
fd: ::c_int,
flags: ::c_uint,
storage: *mut mmapobj_result_t,
elements: *mut ::c_uint,
arg: *mut ::c_void,
) -> ::c_int;
pub fn meminfo(
inaddr: *const u64,
addr_count: ::c_int,
info_req: *const ::c_uint,
info_count: ::c_int,
outdata: *mut u64,
validity: *mut ::c_uint,
) -> ::c_int;

pub fn strcasecmp_l(s1: *const ::c_char, s2: *const ::c_char, loc: ::locale_t) -> ::c_int;
pub fn strncasecmp_l(
s1: *const ::c_char,
s2: *const ::c_char,
n: ::size_t,
loc: ::locale_t,
) -> ::c_int;
pub fn strsep(string: *mut *mut ::c_char, delim: *const ::c_char) -> *mut ::c_char;

pub fn getisax(array: *mut u32, n: ::c_uint) -> ::c_uint;

pub fn backtrace(buffer: *mut *mut ::c_void, size: ::c_int) -> ::c_int;
pub fn backtrace_symbols(buffer: *const *mut ::c_void, size: ::c_int) -> *mut *mut ::c_char;
pub fn backtrace_symbols_fd(buffer: *const *mut ::c_void, size: ::c_int, fd: ::c_int);
}

#[link(name = "lgrp")]
extern "C" {
pub fn lgrp_init(view: lgrp_view_t) -> lgrp_cookie_t;
pub fn lgrp_fini(cookie: lgrp_cookie_t) -> ::c_int;
pub fn lgrp_affinity_get(
idtype: ::idtype_t,
id: ::id_t,
lgrp: ::lgrp_id_t,
) -> ::lgrp_affinity_t;
pub fn lgrp_affinity_set(
idtype: ::idtype_t,
id: ::id_t,
lgrp: ::lgrp_id_t,
aff: lgrp_affinity_t,
) -> ::lgrp_affinity_t;
pub fn lgrp_cpus(
cookie: ::lgrp_cookie_t,
lgrp: ::lgrp_id_t,
cpuids: *mut ::processorid_t,
count: ::c_uint,
content: ::lgrp_content_t,
) -> ::c_int;
pub fn lgrp_mem_size(
cookie: ::lgrp_cookie_t,
lgrp: ::lgrp_id_t,
tpe: ::lgrp_mem_size_flag_t,
content: ::lgrp_content_t,
) -> ::lgrp_mem_size_t;
pub fn lgrp_nlgrps(cookie: ::lgrp_cookie_t) -> ::c_int;
pub fn lgrp_view(cookie: ::lgrp_cookie_t) -> ::lgrp_view_t;
pub fn lgrp_home(idtype: ::idtype_t, id: ::id_t) -> ::lgrp_id_t;
pub fn lgrp_version(version: ::c_int) -> ::c_int;
pub fn lgrp_resources(
cookie: ::lgrp_cookie_t,
lgrp: ::lgrp_id_t,
lgrps: *mut ::lgrp_id_t,
count: ::c_uint,
tpe: ::lgrp_rsrc_t,
) -> ::c_int;
pub fn lgrp_root(cookie: ::lgrp_cookie_t) -> ::lgrp_id_t;
}

Copy link
Contributor

@pfmooney pfmooney Mar 15, 2022

Choose a reason for hiding this comment

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

We can't just remove all of these definitions. Both libsendfile and liblgrp have existed on both illumos and Solaris long enough to be present in their respective sysroots. (True for the "Solaris" one after the build script is fixed as noted by jclulow)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure I knew it would provoke reactions, easy to rollback as separate commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope it get fixed in their CI side, I wish it s easy as it sounds, did not please me to remove these.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure I knew it would provoke reactions, easy to rollback as separate commit.

Please tag us in the rust-lang/rust discussion next time. We could more quickly address the problem directly.

This should not be committed as-is with plans for a rollback in a later commit. Let's get the ::Copy and '::Clone` fixes in (which are good, and needed), and then address the Solaris build issues separately.

@jclulow
Copy link
Contributor

jclulow commented Mar 15, 2022

In general I'm curious about why the libc crate CI did not catch the missing qualifying colons, but the Rust build did. Is that something CI here could catch in future somehow?

@devnexen devnexen force-pushed the solarish_general_build_fix_for_rustc branch from 0a35cc0 to 8642e84 Compare March 15, 2022 19:51
@Amanieu
Copy link
Member

Amanieu commented Mar 18, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Mar 18, 2022

📌 Commit 8642e84 has been approved by Amanieu

@Amanieu
Copy link
Member

Amanieu commented Mar 18, 2022

It's because the error only happens with the rustc-dep-of-std feature enabled, which is only used when built as part of the standard library.

@bors
Copy link
Contributor

bors commented Mar 18, 2022

⌛ Testing commit 8642e84 with merge fe5e6cc...

@bors
Copy link
Contributor

bors commented Mar 18, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing fe5e6cc to master...

@bors bors merged commit fe5e6cc into rust-lang:master Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants