Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Can needs_compilation detect changes on Rust files? #115

Closed
yutannihilation opened this issue Mar 6, 2021 · 8 comments
Closed

Can needs_compilation detect changes on Rust files? #115

yutannihilation opened this issue Mar 6, 2021 · 8 comments
Labels
feature a feature request or enhancement

Comments

@yutannihilation
Copy link

yutannihilation commented Mar 6, 2021

Currently, sources() consider only the files that ends with .c* or .f. May I send a pull request to add .rs here? Or, can this provide more general mechanism (e.g. via options()) that allows users to specify the files to watch on?

dir(srcdir, "\\.(c.*|f)$", recursive = TRUE, full.names = TRUE)

If this is possible, our workflow to develop R packages with Rust code becomes much easier. c.f. extendr/rextendr#56

@gaborcsardi
Copy link
Member

Do you still need this? Would this allow you to use pkgload::load_all()? Or you need it for another reason?

@gaborcsardi gaborcsardi added the feature a feature request or enhancement label Dec 8, 2021
@yutannihilation
Copy link
Author

Do you still need this? Would this allow you to use pkgload::load_all()?

We ended up implementing another version of devtools::document(), it might be less important now. But, I still believe it's nicer if we can customize pkgload::load_all(), which should be generally useful for other compile languages.

@gaborcsardi
Copy link
Member

gaborcsardi commented Dec 11, 2021

OK, do you need anything else apart from the file name extensions? I can add *.rs files, and also a mechanism to define extra files the DLL depends on.

@yutannihilation
Copy link
Author

Thanks. From a perspective of a Rust user, I'd like to detect the changes on Cargo.toml, which defines the configurations of the Rust project (e.g. dependencies, compilation flags), in addition to *.rs. But, if this would make the implementation complex, only *.rs should be fine.

@gaborcsardi
Copy link
Member

Is Cargo.toml in src as well? I guess R CMD check will not let you put it in /?

I am thinking about adding support for a Config/ field in DESCRIPTION that would let you define the sources of the dll, something like this, with comma separated globs:

Config/pkgbuild/dll-sources: Cargo.toml, *.rs

This would be probably in addition to the current *.c* and *.f, for simplicity.

@yutannihilation
Copy link
Author

Is Cargo.toml in src as well?

Yes, this is the typical structure of an R packages using extendr:

.
├── R
│   └── extendr-wrappers.R
...
└── src
    ├── Makevars
    ├── Makevars.win
    ├── entrypoint.c
    └── rust
        ├── Cargo.toml
        └── src
            └── lib.rs

Config/ field in DESCRIPTION

Sounds good to me!

@gaborcsardi
Copy link
Member

@yutannihilation I am sorry this has taken so long. I think I'll skip the config option in DESCRIPTION and will just also detect /src/**/Cargo.toml and /src/**/*.rs files. Is this still OK?

@yutannihilation
Copy link
Author

I think so, thanks for addressing this!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants