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

[R] create_package_with_all_dependencies leaves files unexecutable #40227

Closed
hutch3232 opened this issue Feb 26, 2024 · 3 comments
Closed

[R] create_package_with_all_dependencies leaves files unexecutable #40227

hutch3232 opened this issue Feb 26, 2024 · 3 comments
Assignees
Milestone

Comments

@hutch3232
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

I used create_package_with_all_dependencies on a Windows machine and then copied the tarball to a Linux machine. When I went to install the tarball, I received this error:

ERROR: 'configure' exists but is not executable -- see the 'R Installation and Administration Manual' [r]

Reviewing the code, we see the files are simply tar'd back after downloading the dependencies into the untarred package:

tar_successful <- utils::tar(dest_file, compression = "gz") == 0

I found that if I replace the above line with a pkgbuild::build call, the missing executable bits are automatically fixed:

pkgbuild::build(path = "arrow", dest_path = dest_file)
── R CMD build ──────────────────────────────────────────────────────────────────────────────────────────────────────────
✔  checking for file 'C:\Users\hutch3232\AppData\Local\Temp\RtmpodDbIB\file37344a4258a8\arrow/DESCRIPTION' (986ms)
─  preparing 'arrow': (32.5s)
✔  checking DESCRIPTION meta-information ...cleaning srcchecking for LF line-endings in source and make files and shell scripts (1.3s)
─  checking for empty or unneeded directories (579ms)
─  building 'arrow_14.0.2.1.tar.gz' (494ms)
   Warning: file 'arrow/cleanup' did not have execute permissions: corrected
   Warning: file 'arrow/configure' did not have execute permissions: corrected

After making this change, the install on the Windows and Linux machines worked great. Perhaps there could be a step that sets these as executable manually, or replace tar with build, which includes other checks to ensure a valid package build.

BTW thanks for making this script, it's incredibly useful!

Component(s)

R

@jonkeane
Copy link
Member

Would you be interested in submitting a pull request with this change?

@hutch3232
Copy link
Contributor Author

I'd be happy to!

thisisnic pushed a commit that referenced this issue Mar 6, 2024
…endencies` (#40232)

### What changes are included in this PR?

-  Set the `utils::tar` `extra_flags` argument to `NULL` (default `""`). This benefits from logic baked in (also used by `R CMD BUILD`) that checks the mode of `configure` and `cleanup` files and sets them to be executable if they are not already.

### Are these changes tested?

No. I didn't see any existing tests for this function (which perhaps is not a valid reason). 

### Are there any user-facing changes?

No. Although if `configure` and/or `cleanup` are not executable, that will be corrected with a user-facing `warning`.

* GitHub Issue: #40227

edit: revised the description based on the new approach in 40e8add.

Authored-by: Paul Donnelly <donnelpa@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
@thisisnic thisisnic added this to the 16.0.0 milestone Mar 6, 2024
@thisisnic
Copy link
Member

Issue resolved by pull request 40232
#40232

mapleFU pushed a commit to mapleFU/arrow that referenced this issue Mar 7, 2024
…ll_dependencies` (apache#40232)

### What changes are included in this PR?

-  Set the `utils::tar` `extra_flags` argument to `NULL` (default `""`). This benefits from logic baked in (also used by `R CMD BUILD`) that checks the mode of `configure` and `cleanup` files and sets them to be executable if they are not already.

### Are these changes tested?

No. I didn't see any existing tests for this function (which perhaps is not a valid reason). 

### Are there any user-facing changes?

No. Although if `configure` and/or `cleanup` are not executable, that will be corrected with a user-facing `warning`.

* GitHub Issue: apache#40227

edit: revised the description based on the new approach in 40e8add.

Authored-by: Paul Donnelly <donnelpa@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
…ll_dependencies` (apache#40232)

### What changes are included in this PR?

-  Set the `utils::tar` `extra_flags` argument to `NULL` (default `""`). This benefits from logic baked in (also used by `R CMD BUILD`) that checks the mode of `configure` and `cleanup` files and sets them to be executable if they are not already.

### Are these changes tested?

No. I didn't see any existing tests for this function (which perhaps is not a valid reason). 

### Are there any user-facing changes?

No. Although if `configure` and/or `cleanup` are not executable, that will be corrected with a user-facing `warning`.

* GitHub Issue: apache#40227

edit: revised the description based on the new approach in 40e8add.

Authored-by: Paul Donnelly <donnelpa@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants