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

Allow react-native and expo to build when using GNU coreutils #35382

Closed
wants to merge 2 commits into from

Conversation

shreeve
Copy link
Contributor

@shreeve shreeve commented Nov 17, 2022

Summary

See the issue at #32432 (comment)

This fixes a bizarre issue when using the GNU coreutils tools. There are very minor differences in the standard BSD based command-line tools on macOS and the GNU coreutils versions. In particular, the cp command has slightly different semantics across these two versions. This commit normalizes those differences and allows the GNU coreutils cp command to work the same as the BSD version, thus fixing a hard to track down bug.

Changelog

[General] [Fixed] - Allow GNU coreutils to be used to build projects

Test Plan

This change allows the use of the system or GNU coreutils verson of cp.

@analysis-bot
Copy link

analysis-bot commented Nov 17, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,103,561 +0
android hermes armeabi-v7a 6,471,886 +0
android hermes x86 7,521,203 +0
android hermes x86_64 7,380,081 +0
android jsc arm64-v8a 8,968,399 +0
android jsc armeabi-v7a 7,699,505 +0
android jsc x86 9,030,706 +0
android jsc x86_64 9,508,749 +0

Base commit: 2d1d61a
Branch: main

@analysis-bot
Copy link

analysis-bot commented Nov 17, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 2d1d61a
Branch: main

@pull-bot
Copy link

PR build artifact for ea27953 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for ea27953 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@react-native-bot react-native-bot added No CLA Authors need to sign the CLA before a PR can be reviewed. Bug labels Nov 22, 2022
@cortinico
Copy link
Contributor

Can you please fix the CLA?

Also you can you please clarify why this is needed. We're copying a folder, I'm not sure we want to copy /.

@shreeve
Copy link
Contributor Author

shreeve commented Nov 23, 2022

@cortinico - Here's a quick summary of how BSD cp (default on macOS) and GNU cp (default on Linux and often used on macOS) behave with the cp -R src dst and cp -R src/. dst commands:

# setup
mkdir -p /tmp/cp/{bad,fix}/{bsd,gnu}/{src,dst}
touch    /tmp/cp/{bad,fix}/{bsd,gnu}/src/{1,2,3,4}.txt

# Starting point, with all files in their 'src' directory:
tree     /tmp/cp

/tmp/cp
├── bad
│   ├── bsd
│   │   ├── dst
│   │   └── src
│   │       ├── 1.txt
│   │       ├── 2.txt
│   │       ├── 3.txt
│   │       └── 4.txt
│   └── gnu
│       ├── dst
│       └── src
│           ├── 1.txt
│           ├── 2.txt
│           ├── 3.txt
│           └── 4.txt
└── fix
    ├── bsd
    │   ├── dst
    │   └── src
    │       ├── 1.txt
    │       ├── 2.txt
    │       ├── 3.txt
    │       └── 4.txt
    └── gnu
        ├── dst
        └── src
            ├── 1.txt
            ├── 2.txt
            ├── 3.txt
            └── 4.txt

14 directories, 16 files

# bad method (uses "src/")
gnu-cp -R /tmp/cp/bad/gnu/src/  /tmp/cp/bad/gnu/dst
bsd-cp -R /tmp/cp/bad/bsd/src/  /tmp/cp/bad/bsd/dst

# fix method (uses "src/.")
gnu-cp -R /tmp/cp/fix/gnu/src/. /tmp/cp/fix/gnu/dst
bsd-cp -R /tmp/cp/fix/bsd/src/. /tmp/cp/fix/bsd/dst

# Show result, see how gnu/src was copied to gnu/dst/src
# But, using "src/." works for both gnu and bsd
tree /tmp/cp

/tmp/cp
├── bad
│   ├── bsd
│   │   ├── dst
│   │   │   ├── 1.txt
│   │   │   ├── 2.txt
│   │   │   ├── 3.txt
│   │   │   └── 4.txt
│   │   └── src
│   │       ├── 1.txt
│   │       ├── 2.txt
│   │       ├── 3.txt
│   │       └── 4.txt
│   └── gnu
│       ├── dst
│       │   └── src
│       │       ├── 1.txt
│       │       ├── 2.txt
│       │       ├── 3.txt
│       │       └── 4.txt
│       └── src
│           ├── 1.txt
│           ├── 2.txt
│           ├── 3.txt
│           └── 4.txt
└── fix
    ├── bsd
    │   ├── dst
    │   │   ├── 1.txt
    │   │   ├── 2.txt
    │   │   ├── 3.txt
    │   │   └── 4.txt
    │   └── src
    │       ├── 1.txt
    │       ├── 2.txt
    │       ├── 3.txt
    │       └── 4.txt
    └── gnu
        ├── dst
        │   ├── 1.txt
        │   ├── 2.txt
        │   ├── 3.txt
        │   └── 4.txt
        └── src
            ├── 1.txt
            ├── 2.txt
            ├── 3.txt
            └── 4.txt

15 directories, 32 files

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 23, 2022
@shreeve
Copy link
Contributor Author

shreeve commented Nov 23, 2022

@cortinico - You are right, that second "/." isn't needed. But, the first one is critical for copying to work correctly across BSD and GNU versions of the cp command.

@react-native-bot react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Nov 23, 2022
@pull-bot
Copy link

PR build artifact for 57d6e1b is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for 57d6e1b is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@cortinico
Copy link
Contributor

/rebase

See the issue at facebook#32432 (comment)

This fixes a bizarre issue when using the GNU coreutils tools. There are very minor changes in the standard command-line tools on macOS and the GNU coreutils. The `cp` command has slightly different semantics across these operating systems, so this commit normalizes those differences and allows GNU coreutils to be used or the system native version of `cp`.
@shreeve
Copy link
Contributor Author

shreeve commented Nov 25, 2022

Should be synced up now 👍🏻

@cortinico
Copy link
Contributor

Should be synced up now 👍🏻

Great 👍 Let's wait for a CI run and we can import/merge it

@pull-bot
Copy link

PR build artifact for d240dbc is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for d240dbc is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @shreeve in f5e5274.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 25, 2022
kelset pushed a commit that referenced this pull request Nov 30, 2022
Summary:
See the issue at #32432 (comment)

This fixes a bizarre issue when using the GNU coreutils tools. There are very minor differences in the standard command-line tools on macOS and the GNU coreutils. The `cp` command has slightly different semantics across these operating systems, so this commit normalizes those differences and allows GNU coreutils to be used or the system native version of `cp`.

Fixes #32432

## Changelog

[General] [Fixed] - Allow GNU coreutils to be used to build projects

Pull Request resolved: #35382

Test Plan: This change allows the use of the system or GNU coreutils verson of `cp`.

Reviewed By: cipolleschi

Differential Revision: D41532472

Pulled By: cortinico

fbshipit-source-id: f0fe5274d3828bf6099deceee797a82a6adfdcab
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…ok#35382)

Summary:
See the issue at facebook#32432 (comment)

This fixes a bizarre issue when using the GNU coreutils tools. There are very minor differences in the standard command-line tools on macOS and the GNU coreutils. The `cp` command has slightly different semantics across these operating systems, so this commit normalizes those differences and allows GNU coreutils to be used or the system native version of `cp`.

Fixes facebook#32432

## Changelog

[General] [Fixed] - Allow GNU coreutils to be used to build projects

Pull Request resolved: facebook#35382

Test Plan: This change allows the use of the system or GNU coreutils verson of `cp`.

Reviewed By: cipolleschi

Differential Revision: D41532472

Pulled By: cortinico

fbshipit-source-id: f0fe5274d3828bf6099deceee797a82a6adfdcab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants