Skip to content

Conversation

xnum
Copy link
Contributor

@xnum xnum commented Oct 21, 2017

If I download the zip file and unzip it on Windows, then upload to my workstation.
The permission of executable files and symbolic link file are broken.

If I download the zip file and unzip it on Windows, then upload to my workstation. 
The permission of executable files and symbolic link file are broken.
@mbland mbland requested a review from a team October 21, 2017 16:15
@mbland mbland self-assigned this Oct 21, 2017
@mbland
Copy link
Contributor

mbland commented Oct 25, 2017

Sorry I haven't looked at this yet; super-slammed at work this week. Will try to take a look this weekend.

@mbland
Copy link
Contributor

mbland commented Oct 30, 2017

Sorry I didn't get to this this weekend; got slammed with another project. Will try to take a look later today.

install.sh Outdated
# fix broken symbolic link file
if [ ! -L "$PREFIX"/bin/bats ]; then
rm -f "$PREFIX"/bin/bats
ln -s "$PREFIX"/libexec/bats "$PREFIX"/bin/bats
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried this on Git for Windows, and for some reason ln -s doesn't want to cooperate:

$ ./install.sh ../usr
ln: failed to create symbolic link '../usr/bin/bats': No such file or directory

Since the "link" is just a copy on this platform anyway, perhaps we should just use cp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems some issue on NTFS
ref: 1
I got the solutionfrom 2
assign absolute path instead of relative path.
It works with git-bash.exe

install.sh Outdated
@@ -36,8 +36,9 @@ cp "$BATS_ROOT"/man/bats.7 "$PREFIX"/share/man/man7

# fix broken symbolic link file
if [ ! -L "$PREFIX"/bin/bats ]; then
rm -f "$PREFIX"/bin/bats
ln -s "$PREFIX"/libexec/bats "$PREFIX"/bin/bats
dir=$(readlink -e "$PREFIX")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, but confirmed locally this approach works. Just one thing: please enclose the $(readlink -e "$PREFIX") in double-quotes, in case the result includes spaces.

@mbland mbland merged commit b1da565 into bats-core:master Nov 13, 2017
@ntnn
Copy link

ntnn commented Nov 13, 2017

@mbland As I've noted there are nomerous problems with this PR, why was it merged nonetheless?

See here:
https://github.com/bats-core/bats-core/pull/32/files/062160105638ebb101d867d1e9de160b00977e35#diff-3fbb47e318cd8802bd325e7da9aaabe8

This is quite off-putting for the project if even a simple install.sh as that can't be maintained properly.

I can't get a link to it, but one comment was about the absolute path - You never want an absolute path in an install script, as this will break the installation at least in source-based distributions.

No, that won't help either. Use ln -sf ../libexec/bats "$PREFIX/bin/bats".

For example in source-based distributions the install is done in a sandbox, e.g. $TMPDIR/bats/install and the source code is in $TMPDIR/bats/src. If install.sh $TMPDIR/bats/install is executed the symbolic link will point to $TMPDIR/bats/install/libexec/bats and break once the install tree has been copied to the root filesystem.

@mbland
Copy link
Contributor

mbland commented Nov 13, 2017

@ntnn I never saw that comment...that commit link doesn't show anything, and I can't find it anywhere else in the PR.

That is a fair point about the absolute path and using ln -sf instead. Sorry I missed it. Feel free to submit a follow-up PR.

@ntnn
Copy link

ntnn commented Nov 13, 2017

@mbland Huh, github is weird - I'll post a PR later this week. Thanks for answering so quickly.

@xnum
Copy link
Contributor Author

xnum commented Nov 13, 2017

sorry, but ln -sf couldn't solve the problem

@mbland
Copy link
Contributor

mbland commented Nov 13, 2017

Hmm, that is strange. It seems dependent on the path of the source, not on the path of the target. It's probably best just to copy the file rather than symlink it. Any reason not to do just that?

@xnum
Copy link
Contributor Author

xnum commented Nov 13, 2017

As I mentioned before, the problem is caused with NTFS probably
reference

copy the file is not working because bin/bats links to libexec/bats
and find libexec/bats-exec-test which not exists in bin dir.

mbland added a commit that referenced this pull request Jun 1, 2018
Sidesteps the problems encountered during #32, and with running
`bin/bats` on Windows environments that don't support symlinks.
mbland added a commit that referenced this pull request Jun 1, 2018
Sidesteps the problems encountered during #32, and with running
`bin/bats` on Windows environments that don't support symlinks.
@mbland mbland mentioned this pull request Jun 3, 2018
2 tasks
@dotmpe
Copy link
Contributor

dotmpe commented Jun 5, 2018

Can we not adjust the installation, or package for windows?

Or at least keep and rename bin/bats script as bin/bats.win32 as long as there is no install but only ZIP for windows.

mbland added a commit that referenced this pull request Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves
outstanding issues from #32.

Per all these earlier issues and PRs:

- The original bin/bats symlink was causing problems on Windows.
- The attempted fix from #32 created problems for install.sh.
- Per #90, changing bin/bats to a one-line script in #88 broke some Bats
  installations that rely on symlink schemes (such as when installed via
  https://github.com/basherpm/basher).

For an idea why I wanted to abolish using any symlinks if I could for
Windows's sake, see the following results I discovered during a search
for "git symlink windows":

- https://github.com/git-for-windows/git/wiki/Symbolic-Links
- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
- libgit2/libgit2#4107
- https://stackoverflow.com/q/5917249

Using a method inspired by go-core.bash from
https://github.com/mbland/go-script-bash, I was able to reliably compute
the root of the Bats installation without using `readlink` by making use
of `PWD` and `cd`.

What's more, in the course of doing this, I realized libexec/bats didn't
need `readlink` either, so I removed it, eliminating external
dependencies and subshells. I also removed some extraneous variables and
small bits of logic.

On top of making install.sh, bin/bats, and libexec/bats more
self-contained, resilient, and compact, the existing test suite (before
the new installer and symlink tests) sped up significantly, running
0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM):

  Bash 3.2.57(1)-release before:
  58 tests, 0 failures

  real    0m4.924s
  user    0m3.045s
  sys     0m1.798s

  Bash 3.2.57(1)-release after:
  58 tests, 0 failures

  real    0m4.326s
  user    0m2.793s
  sys     0m1.508s

  Bash 4.4.23(1)-release before:
  58 tests, 0 failures

  real    0m5.228s
  user    0m3.046s
  sys     0m1.952s

  Bash 4.4.23(1)-release after:
  58 tests, 0 failures

  real    0m4.563s
  user    0m2.782s
  sys     0m1.614s
mbland added a commit that referenced this pull request Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves
outstanding issues from #32.

Per all these earlier issues and PRs:

- The original bin/bats symlink was causing problems on Windows.
- The attempted fix from #32 created problems for install.sh.
- Per #90, changing bin/bats to a one-line script in #88 broke some Bats
  installations that rely on symlink schemes (such as when installed via
  https://github.com/basherpm/basher).

For an idea why I wanted to abolish using any symlinks if I could for
Windows's sake, see the following results I discovered during a search
for "git symlink windows":

- https://github.com/git-for-windows/git/wiki/Symbolic-Links
- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
- libgit2/libgit2#4107
- https://stackoverflow.com/q/5917249

Using a method inspired by go-core.bash from
https://github.com/mbland/go-script-bash, I was able to reliably compute
the root of the Bats installation without using `readlink` by making use
of `PWD` and `cd`.

What's more, in the course of doing this, I realized libexec/bats didn't
need `readlink` either, so I removed it, eliminating external
dependencies and subshells. I also removed some extraneous variables and
small bits of logic.

On top of making install.sh, bin/bats, and libexec/bats more
self-contained, resilient, and compact, the existing test suite (before
the new installer and symlink tests) sped up significantly, running
0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM):

  Bash 3.2.57(1)-release before:
  58 tests, 0 failures

  real    0m4.924s
  user    0m3.045s
  sys     0m1.798s

  Bash 3.2.57(1)-release after:
  58 tests, 0 failures

  real    0m4.326s
  user    0m2.793s
  sys     0m1.508s

  Bash 4.4.23(1)-release before:
  58 tests, 0 failures

  real    0m5.228s
  user    0m3.046s
  sys     0m1.952s

  Bash 4.4.23(1)-release after:
  58 tests, 0 failures

  real    0m4.563s
  user    0m2.782s
  sys     0m1.614s

Also tweaks the Dockerfile to update the symlink to point to bin/bats,
not libexec/bats.
mbland added a commit that referenced this pull request Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves
outstanding issues from #32.

Per all these earlier issues and PRs:

- The original bin/bats symlink was causing problems on Windows.
- The attempted fix from #32 created problems for install.sh.
- Per #90, changing bin/bats to a one-line script in #88 broke some Bats
  installations that rely on symlink schemes (such as when installed via
  https://github.com/basherpm/basher).

For an idea of why I wanted to keep bin/bats as a script due to how
symlinks complicate things on Windows, see the following results I
discovered during a search for "git symlink windows":

- https://github.com/git-for-windows/git/wiki/Symbolic-Links
- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
- libgit2/libgit2#4107
- https://stackoverflow.com/q/5917249

In the course of applying these changes, I realized libexec/bats
computed some extraneous variables, so I removed them, eliminating a few
external processes and subshells. I also cleaned up other small bits of
logic.

On top of making install.sh, bin/bats, and libexec/bats more resilient
and compact, the existing test suite (before adding the new
test/installer.bats file) sped up significantly, running 0.6s-0.7s
faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM):

  Bash 3.2.57(1)-release before:
  58 tests, 0 failures

  real    0m4.924s
  user    0m3.045s
  sys     0m1.798s

  Bash 3.2.57(1)-release after:
  58 tests, 0 failures

  real    0m4.341s
  user    0m2.808s
  sys     0m1.540s

  Bash 4.4.23(1)-release before:
  58 tests, 0 failures

  real    0m5.228s
  user    0m3.046s
  sys     0m1.952s

  Bash 4.4.23(1)-release after:
  58 tests, 0 failures

  real    0m4.582s
  user    0m2.791s
  sys     0m1.643s

Also tweaks the Dockerfile to update the symlink to point to bin/bats,
not libexec/bats.
@mbland mbland mentioned this pull request Jun 7, 2018
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants