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

Better install subcommand #5

Merged
merged 2 commits into from
Dec 20, 2020
Merged

Better install subcommand #5

merged 2 commits into from
Dec 20, 2020

Conversation

kubkon
Copy link
Member

@kubkon kubkon commented Dec 14, 2020

Install subcommand now re-uses hashing algorithm from process_headers tool to work out how the common headers should be distributed among various architecture-dependent include directories such as aarch64-macos-gnu, x86_64-macos-gnu or any-macos-any for shared directory between the architectures.

The idea behind fetch_them_macos_headers program is now as follows:

  • running bare (no args, or with cflags) still fetches the headers into cwd/<arch>-macos-gnu
  • running install subcommand however now does the following:
    1. descend 3 potential libc paths: cwd/<this_arch>-macos-gnu, install_dir/<other_arch>-macos-gnu, install_dir/any-macos-any
    2. calculate hash for each encountered file according to the same logic as in process_headers tool
    3. put the files into 3 directories (<this_arch>-macos-gnu, <other_arch>-macos-gnu, any-macos-any) inside temp directory with the shared bits going into any-macos-any
    4. copy the directories from the temp directory into install_dir (note we copy here instead of deleting and renaming to avoid the potential situation where some headers might be deleted with the newer version of libc on a newer platform).

Install subcommand now re-uses hashing algorithm from `process_headers`
tool to work out how the common headers should be distributed among
various architecture-dependent include directories such as
`aarch64-macos-gnu`, `x86_64-macos-gnu` or `any-macos-gnu` for shared
directory between the architectures.
@kubkon kubkon requested a review from andrewrk December 14, 2020 17:02
@andrewrk
Copy link
Member

andrewrk commented Dec 18, 2020

The process to update headers is going to break the moment we merge ziglang/zig#7318 because it treats the install directory both as the unprocessed input and the post-processed output.

I suggest that we commit both aarch64-macos-gnu and x86_64-macos-gnu headers (unprocessed) to this repo, so that someone with access to one macos laptop but not the other can still run the tool and produce the output that will go upstream into the zig repo. Then the tool will will have 2 different functions:

  • Update -macos-gnu for committing to this repo
  • Process both aarch64-macos-gnu and x86_64-macos-gnu into an install directory, producing the 3 directories that we want for upstream zig repo.

How does that sound?

@kubkon
Copy link
Member Author

kubkon commented Dec 19, 2020

The process to update headers is going to break the moment we merge ziglang/zig#7318 because it treats the install directory both as the unprocessed input and the post-processed output.

Yeah, that's one of the things I struggled with. Hence why I only analyse the local version of libc fetched with this tool here:

inline for (targets) |target| {
    const path = if (comptime target.eql(dest_target)) "." else dest_path;
    const res = try findDuplicates(target, allocator, path, &path_table, &hash_to_contents);
    savings.max_bytes_saved += res.max_bytes_saved;
    savings.total_bytes += res.total_bytes;
}

I suggest that we commit both aarch64-macos-gnu and x86_64-macos-gnu headers (unprocessed) to this repo, so that someone with access to one macos laptop but not the other can still run the tool and produce the output that will go upstream into the zig repo. Then the tool will will have 2 different functions:

  • Update -macos-gnu for committing to this repo
  • Process both aarch64-macos-gnu and x86_64-macos-gnu into an install directory, producing the 3 directories that we want for upstream zig repo.

How does that sound?

Excellent! I like your idea far more than my hack above. Let's go with it!

We will use that as the source for establishing the common ground
between libc headers on `aarch64` and `x86_64`.

For the install step, we will use the libc headers committed to this
repo to generate the three directories: `any-macos-any`,
`aarch64-macos-gnu`, and `x86_64-macos-gnu` which will be then installed
into a specified target install dir. Upon installation, we will
proactively remove the three directories from the installation path
so that we avoid obsoletes.
@kubkon
Copy link
Member Author

kubkon commented Dec 19, 2020

@andrewrk OK, the PR should ready for the next round of reviews. We now commit libc headers to this repo, which we then use as the source for establishing the common ground between libc headers on aarch64 and x86_64.

For the install step, we use the libc headers committed to this repo to generate the three directories: any-macos-any,
aarch64-macos-gnu, and x86_64-macos-gnu which will then be installed into a specified target install dir. Upon installation, we will proactively remove the three directories from the installation path so that we avoid obsoletes.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

nice work!

@kubkon kubkon merged commit db8331f into ziglang:main Dec 20, 2020
@kubkon kubkon deleted the better-install branch December 20, 2020 07:47
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.

2 participants