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.

Sign up for GitHub

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

Jump to bottom

[R]: on_rosetta() should not error #38752

Closed
jonkeane opened this issue Nov 16, 2023 · 0 comments · Fixed by #38754
Closed

[R]: on_rosetta() should not error #38752

jonkeane opened this issue Nov 16, 2023 · 0 comments · Fixed by #38754
Assignees
@jonkeane
Labels
Component: R Type: bug
Milestone
14.0.2

Comments

@jonkeane
Copy link
Member

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

This is happening on CRAN's M1: https://www.stats.ox.ac.uk/pub/bdr/M1mac/arrow.out

* checking loading without being on the library search path ... WARNING
Error in system("sysctl -n sysctl.proc_translated", intern = TRUE, ignore.stderr = TRUE) : 
  error in running command

Attaching package: ‘arrow’

The following object is masked from ‘package:utils’:

    timestamp


It looks like this package has a loading problem when not on .libPaths:
see the messages for details.
* checking whether startup messages can be suppressed ... NOTE
Error in system("sysctl -n sysctl.proc_translated", intern = TRUE, ignore.stderr = TRUE) : 
  error in running command

It looks like this package (or a package it requires) has a startup

I thought that this was already wrapped in tryCatch, but it appears not. We should do this

Component(s)

R
@jonkeane jonkeane self-assigned this Nov 16, 2023
@github-actions github-actions bot mentioned this issue Nov 16, 2023
Merged
assignUser pushed a commit that referenced this issue Nov 17, 2023
@jonkeane
GH-38752: [R] Wrap rosetta detection in tryCatch (#38754)
e543ee6 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: #38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
@assignUser assignUser added this to the 15.0.0 milestone Nov 17, 2023
@assignUser assignUser mentioned this issue Nov 17, 2023
Closed
37 tasks
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Nov 23, 2023
@jonkeane @thisisnic
apacheGH-38752: [R] Wrap rosetta detection in tryCatch (apache#38754)
09deb86 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: apache#38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
@assignUser assignUser modified the milestones: 15.0.0, 14.0.2 Nov 27, 2023
assignUser pushed a commit to thisisnic/arrow that referenced this issue Nov 28, 2023
@jonkeane @assignUser
apacheGH-38752: [R] Wrap rosetta detection in tryCatch (apache#38754)
eaf1dbf 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: apache#38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
raulcd pushed a commit that referenced this issue Nov 28, 2023
@jonkeane @raulcd
GH-38752: [R] Wrap rosetta detection in tryCatch (#38754)
490b7c2 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: #38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
assignUser pushed a commit to assignUser/arrow that referenced this issue Dec 1, 2023
@jonkeane @assignUser
apacheGH-38752: [R] Wrap rosetta detection in tryCatch (apache#38754)
b82181c 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: apache#38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
@jonkeane @dgreiss
apacheGH-38752: [R] Wrap rosetta detection in tryCatch (apache#38754)
38b0d26 
### Rationale for this change

We should never allow rosetta checking from causing an error

### What changes are included in this PR?

~Wrap rosetta checking in a tryCatch~ our use of `try()` wasn't doing what we thought, it actually needs to have `silent = TRUE` specified to _not_ error.

### Are these changes tested?

I tested them locally by manipulating the system call to a mangled command that doesn't exist, observing the error on load, then wrapping in trycatch. We might consider adding a test in CI, though there would be considerable complexity for something like that

### Are there any user-facing changes?

No, though we will need to pull it into any point release
* Closes: apache#38752

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonkeane jonkeane

Labels
Component: R Type: bug
Projects
None yet
Milestone
14.0.2
Development

Successfully merging a pull request may close this issue.

GH-38752: [R] Wrap rosetta detection in tryCatch jonkeane/arrow
2 participants
@jonkeane @assignUser