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

Architecture independent install files support #184

Closed
naokiri opened this issue Apr 17, 2021 · 20 comments · Fixed by #229
Closed

Architecture independent install files support #184

naokiri opened this issue Apr 17, 2021 · 20 comments · Fixed by #229
Labels
D-Hard A task that takes serveral days to complete enhancement New feature or request T-Install Install process

Comments

@naokiri
Copy link
Contributor

naokiri commented Apr 17, 2021

I want support for installing architecture independent files. For instance, I have a usecase to install configuration file under /usr/local/share/myproject/.

Once datadir is supported, it should be easy to add infodir, mandir, localedir etc other common shared files support.
c.f. https://www.gnu.org/software/make/manual/html_node/Directory-Variables.html

naokiri added a commit to naokiri/cargo-c that referenced this issue Apr 17, 2021
This change adds support for installing read-only architecture-independent data files.
@lu-zero
Copy link
Owner

lu-zero commented Apr 17, 2021

This should be discussed with cargo upstream see rust-lang/cargo#2729 and rust-lang/cargo#2386

@naokiri
Copy link
Contributor Author

naokiri commented Apr 17, 2021

I think cargo upstream have consensus that the original cargo doesn't want to install things on system-wide, and puts everything under ~/.cargo.

C ABI libraries typically uses /usr/share/ and /usr/local/share so I thought it's potentially a scope of this project.

As a owner of the project, it's your call. Feel free to close the issue if this wasn't scope of this project.

@lu-zero
Copy link
Owner

lu-zero commented Apr 23, 2021

I have no problems in adding install.rs support first if you have a specific need for it.

Can you provide more information on which are your use-cases?

@naokiri
Copy link
Contributor Author

naokiri commented Apr 29, 2021

Sorry but I might be missing something. Could you elaborate what install.rs support means?

My usage of /usr/local/share is for my own project: https://github.com/naokiri/cskk porting an existing library into Rust, and need to add /usr/local/share/libcskk/rules directory with some default rule data on install.

@lu-zero
Copy link
Owner

lu-zero commented Apr 29, 2021

There is a postponed rfc about it, I might add something along those lines to give enough flexibility. I guess much depends on how early you'd need it.

@naokiri
Copy link
Contributor Author

naokiri commented Apr 29, 2021

It is just a personal project that no public user exists yet so it's not in hurry at all. And also I'm hesitant to touch the upstream cargo project since I'm not so used to Rust yet and probably cannot help much.

@lu-zero
Copy link
Owner

lu-zero commented Apr 29, 2021

I'll try to get something in place that is more or or less along the lines of the rfc. So something called cinstall.rs that gets inputs and outputs like build.rs.

@mdegans
Copy link

mdegans commented May 19, 2021

@naokiri
I've used cargo-deb in conjunction with cargo-c to generate a debian package for systemwide install. It's not perfect (solib symlinks need fixing with a postinst script), but it mostly works, If you edit the package manually it can be close to perfect, especially for your own use.

@lu-zero lu-zero added D-Hard A task that takes serveral days to complete enhancement New feature or request T-Install Install process labels May 27, 2021
@lu-zero
Copy link
Owner

lu-zero commented May 27, 2021

I'll try to get a prototype ready once I have other internal rework landing :)

@lu-zero
Copy link
Owner

lu-zero commented Jun 18, 2021

The workspace support seems working so I'd start considering supporting installing arbitrary information.

We could add a package.metadata.capi.install section with specific keys, initially I'd support only data, include and man, if you can think of more I'd welcome pull requests to expand it.
For every key you can assume either the data is pre-generated and lives in {rootdir}/assets or it is generated by some means (in build.rs) and can be found in {outdir}/{key}/{path}.
The cli gets additional --mandir and --datadir overrides.

Example:

[package.metadata.capi.install]
# install assets/some_data in in ${prefix}/share/${crate_name}
# install ${OUTDIR}/data/generated_data in ${prefix}/share/${crate_name}
data = {assets=["some_data"], generated="generated_data"}
# install pre-generated headers or headers generated by a non-bindgen generator
include = {assets=["some.h", "header.h"]}
# man pages
man = {...}

@naokiri would that work for your purpose?

@olivierlemasle
Copy link

  1. Could it be possible to select files outside assets (but already in the crate)?
  2. And could it be possible to use globs to select files?

My use case is Wasmtime C API, which has pre-generated headers located in include/**/*.h and wasm-c-api/include/*.h

@lu-zero
Copy link
Owner

lu-zero commented Jun 22, 2021

1. Could it be possible to select files outside `assets` (but already in the crate)?

Yes, it is an open question if it is more ergonomic having {assets, ${OUTDIR}/capi/{include,data,man,}/as default search paths, putting in{includedir,...}/` everything stored in those directories.

While letting you override everything and using the default paths from from the root of the

[package.metadata.capi.install]
include = {assets=[{src="{path from the root of the crate}", dst="{path to be appended to ${includedir}}"}]}
2. And could it be possible to use globs to select files?

I do not see problems in that.

My use case is Wasmtime C API, which has pre-generated headers located in include/**/*.h and wasm-c-api/include/*.h

With

[package.metadata.capi.install]
include = {assets=[{src="include/*"}, {src="wasm-c-api/include/*"}]}

You'd end up with

/usr/local/include/wasmtime/wasmtime.h
/usr/local/include/wasmtime/wasmtime/wasmtime/config.h
...
/usr/local/include/wasmtime/wasm.h
/usr/local/include/wasmtime/wasm.hh

Does it sound good to you?

@olivierlemasle
Copy link

olivierlemasle commented Jun 22, 2021

While letting you override everything and using the default paths from from the root of the

[package.metadata.capi.install]
include = {assets=[{src="{path from the root of the crate}", dst="{path to be appended to ${includedir}}"}]}

I really like it 👍
That would be a great addition to cargo-c!

In your example, you also include the .hh header file, but that's not an issue.

Just to be sure:

  • I guess you meant: include = {assets=[{src="include/*"}, {src="wasm-c-api/include/*"}]} (no ..) (edited)
  • In /usr/local/include/wasmtime/wasmtime/wasmtime/config.h, there's a /wasmtime too many, right?

@lu-zero
Copy link
Owner

lu-zero commented Jun 22, 2021

That's correct, the .. were spurious.

@lu-zero
Copy link
Owner

lu-zero commented Jun 22, 2021

While letting you override everything and using the default paths from from the root of the

[package.metadata.capi.install]
include = {assets=[{src="{path from the root of the crate}", dst="{path to be appended to ${includedir}}"}]}

I really like it 👍
That would be a great addition to cargo-c!

If everything goes well it could land before or after the workspaces support, depending on when cargo 0.56 gets releases.
Help testing it is welcome :)

In your example, you also include the .hh header file, but that's not an issue.

I would copy everything in the paths by default.

@olivierlemasle
Copy link

olivierlemasle commented Jun 23, 2021

Help testing it is welcome :)

Sure, of course!

lu-zero added a commit that referenced this issue Jul 1, 2021
The default patterns are:
- pre-generated in: `{root_dir}/assets/capi/include/**/*`
- generated in: `{out_dir}/capi/include/**/*`

See #184
@lu-zero
Copy link
Owner

lu-zero commented Jul 1, 2021

You may try #212 and tell me if it is ergonomic enough see example-project for the usage for now, if it works fine for you the README could be updated with the documentation later.

lu-zero added a commit that referenced this issue Jul 1, 2021
The default patterns are:
- pre-generated in: `{root_dir}/assets/capi/include/**/*`
- generated in: `{out_dir}/capi/include/**/*`

See #184
@olivierlemasle
Copy link

@lu-zero You already merged #212 so I tested on master.

It works perfectly for my usecase (Wasmtime), thanks!

My configuration:

[package.metadata.capi.header]
generation = false
[package.metadata.capi.install.include]
asset = [{from = "include/**/*"}, {from = "wasm-c-api/include/*.h"}]

@lu-zero
Copy link
Owner

lu-zero commented Jul 2, 2021

I guess I can add the documentation bits and if @federicomenaquintero is also happy with it cut a release.

@naokiri I'd add the support for other files (e.g. data) after some refactor in a following release, help is welcome :)

@lu-zero
Copy link
Owner

lu-zero commented Jul 3, 2021

@olivierlemasle cargo-c 0.9.0 is out, once wasmtime has support for it please send a PR to update the readme :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-Hard A task that takes serveral days to complete enhancement New feature or request T-Install Install process
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants