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

Publish bundled binary on Windows loses execution permission #8006

Open
BusyJay opened this issue Mar 16, 2020 · 9 comments
Open

Publish bundled binary on Windows loses execution permission #8006

BusyJay opened this issue Mar 16, 2020 · 9 comments
Labels
C-bug Category: bug Command-package O-windows OS: Windows P-low Priority: Low S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@BusyJay
Copy link
Contributor

BusyJay commented Mar 16, 2020

Problem

When publishing a crate with bundled binaries on Windows, all the permission of binaries become 644.

Possible Solution(s)
Publish it on *nix instead.

Notes

Output of cargo version
cargo 1.40.0 (bc8e4c8 2019-11-22)

@BusyJay BusyJay added the C-bug Category: bug label Mar 16, 2020
@ehuss
Copy link
Contributor

ehuss commented Mar 16, 2020

Can you provide a reproduction or more details? I did a quick test with packaging a project with an executable, and the executable retained its permissions.

@BusyJay
Copy link
Contributor Author

BusyJay commented Mar 16, 2020

I published protobuf-build 0.11.0 on Windows and downloaded it on Linux, only to find that binaries in bin directory has permissions 644. I published it again using the same single step cargo publish on Linux, now binaries retain their own original permissions.

@BusyJay
Copy link
Contributor Author

BusyJay commented Mar 16, 2020

You can test it without uploading. Take protobuf-build as an example:

PS C:\Users\jay\protobuf-build> cargo package
...
PS C:\Users\jay\protobuf-build> tar tvf .\target\package\protobuf-build-0.11.0.crate
-rw-r--r--  0 0      0          44 Mar 16 17:52 protobuf-build-0.11.0/.gitignore
-rw-r--r--  0 0      0         825 Mar 16 17:52 protobuf-build-0.11.0/.travis.yml
-rw-r--r--  0 0      0     4867908 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-linux-aarch_64
-rw-r--r--  0 0      0     6887392 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-linux-ppcle_64
-rw-r--r--  0 0      0     4595900 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-linux-x86_32
-rw-r--r--  0 0      0     4969752 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-linux-x86_64
-rw-r--r--  0 0      0    10382660 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-osx-x86_64
-rw-r--r--  0 0      0     2629632 Mar 16 17:52 protobuf-build-0.11.0/bin/protoc-win32.exe
-rw-r--r--  0 0      0        1095 Mar 16 17:52 protobuf-build-0.11.0/Cargo.toml.orig
-rw-r--r--  0 0      0        1655 Mar 16 19:29 protobuf-build-0.11.0/Cargo.toml
-rw-r--r--  0 0      0         902 Mar 16 17:52 protobuf-build-0.11.0/README.md
-rw-r--r--  0 0      0        7752 Mar 16 17:52 protobuf-build-0.11.0/src/lib.rs
-rw-r--r--  0 0      0         716 Mar 16 17:52 protobuf-build-0.11.0/src/prost_impl.rs
-rw-r--r--  0 0      0        6655 Mar 16 17:52 protobuf-build-0.11.0/src/protobuf_impl.rs
-rw-r--r--  0 0      0       23899 Mar 16 17:52 protobuf-build-0.11.0/src/wrapper.rs
-rw-r--r--  0 0      0          74 Mar 16 19:29 protobuf-build-0.11.0/.cargo_vcs_info.json

@ehuss
Copy link
Contributor

ehuss commented Mar 16, 2020

Ah. AFAIK, Windows doesn't exactly have the same concept of POSIX permission modes (there is no "execute" bit). More or less we only know if something is a directory or file, and whether or not it is readonly. If you extract the archive on Windows, you should see that the .exe file should still be executable.

If this is being published for other platforms, I would recommend not publishing from windows.

@BusyJay
Copy link
Contributor Author

BusyJay commented Mar 16, 2020

Or we can fix it by reading files from git. Actually using git to build an archive on Windows preserve the permission bits.

PS C:\Users\jay\protobuf-build> git archive --format=tar.gz master -o test.tar.gz
PS C:\Users\jay\protobuf-build> tar tvf .\test.tar.gz                                                                                                                                                                           -rw-rw-r--  0 root   root       44 Mar 16 17:11 .gitignore
-rw-rw-r--  0 root   root      825 Mar 16 17:11 .travis.yml
-rw-rw-r--  0 root   root     1095 Mar 16 17:11 Cargo.toml
-rw-rw-r--  0 root   root      902 Mar 16 17:11 README.md
drwxrwxr-x  0 root   root        0 Mar 16 17:11 bin/
-rwxrwxr-x  0 root   root  4867908 Mar 16 17:11 bin/protoc-linux-aarch_64
-rwxrwxr-x  0 root   root  6887392 Mar 16 17:11 bin/protoc-linux-ppcle_64
-rwxrwxr-x  0 root   root  4595900 Mar 16 17:11 bin/protoc-linux-x86_32
-rwxrwxr-x  0 root   root  4969752 Mar 16 17:11 bin/protoc-linux-x86_64
-rwxrwxr-x  0 root   root 10382660 Mar 16 17:11 bin/protoc-osx-x86_64
-rwxrwxr-x  0 root   root  2629632 Mar 16 17:11 bin/protoc-win32.exe
drwxrwxr-x  0 root   root        0 Mar 16 17:11 src/
-rw-rw-r--  0 root   root     7752 Mar 16 17:11 src/lib.rs
-rw-rw-r--  0 root   root      716 Mar 16 17:11 src/prost_impl.rs
-rw-rw-r--  0 root   root     6655 Mar 16 17:11 src/protobuf_impl.rs
-rw-rw-r--  0 root   root    23899 Mar 16 17:11 src/wrapper.rs
drwxrwxr-x  0 root   root        0 Mar 16 17:11 tests/
-rw-rw-r--  0 root   root      431 Mar 16 17:11 tests/Cargo.toml
-rw-rw-r--  0 root   root      138 Mar 16 17:11 tests/build.rs
drwxrwxr-x  0 root   root        0 Mar 16 17:11 tests/proto/
-rw-rw-r--  0 root   root      408 Mar 16 17:11 tests/proto/nested.proto
drwxrwxr-x  0 root   root        0 Mar 16 17:11 tests/src/
-rw-rw-r--  0 root   root      238 Mar 16 17:11 tests/src/lib.rs

@ehuss
Copy link
Contributor

ehuss commented Mar 16, 2020

That sounds interesting! Cargo uses the git2 library, does that have a way to access permissions? Does the tar library support a way to set them?

@alexcrichton
Copy link
Member

We currently slurp up filesystem permissions here and on Windows files are always 0o644 (or 0o444 but that's not too relevant).

We can pretty easily tweak the tar header and set different permissions, but we'll need a location to source them from. If git's got all the information though seems reasonable to read it from there!

@luser
Copy link
Contributor

luser commented May 6, 2020

I understand that this is how the protoc compiler gets packaged effectively everywhere but I'd like to go on record as saying that I think that publishing executables in crate packages is not a good practice and we should not encourage it.

@weihanglo weihanglo added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. P-low Priority: Low labels Jul 16, 2023
@epage
Copy link
Contributor

epage commented Nov 3, 2023

We have #4413 for warning people against distributing binaries.

github-merge-queue bot pushed a commit that referenced this issue Dec 31, 2024
### What does this PR try to resolve?

This adds a special case for checking source files are symlinks
and have been modified when under a VCS control

This is required because those paths may link to a file outside the
current package root, but still under the git workdir, affecting the
final packaged `.crate` file.

### How should we test and review this PR?

Pretty similar to #14966, as a part of #14967.

This may have potential performance issue. If a package contains
thousands of symlinks, Cargo will fire `git status` for each of them.
Not sure if we want to do anything proactively now.

The introduction of the `PathEntry` struct gives us more room for
storing file metadata to satisfiy use cases in the future. For
instances,

* Knowing a source file is a symlink and resolving it when packaging on
Windows
  * #5664
  * #14965
* Knowing more about a file's metadata (e.g. permission bits from Git)
  * #4413
  * #8006
* Provide richer information for `cargo package --list`, for example
JSON output mode
  * #11666
  * #13331
  * #13953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-package O-windows OS: Windows P-low Priority: Low S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
Development

No branches or pull requests

6 participants