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

Fix no-codegen test on musl64 tester #42827

Merged
merged 2 commits into from
Nov 1, 2021
Merged

Conversation

staticfloat
Copy link
Sponsor Member

Apparently, busybox has strange behavior when you copy $pfx/.. and the destination directory exists:

$ docker run -ti alpine
# mkdir -p /tmp/foo/bar; touch /tmp/foo/bar/a /tmp/foo/bar/b
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/bar'
'/tmp/foo/bar/..' -> '/tmp/foo2'
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/../bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/../bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/../bar'
'/tmp/foo/bar/..' -> '/tmp/foo2/..'

We'll dodge this by using Julia's cp() function.

Apparently, busybox has strange behavior when you copy `$pfx/..` and the destination directory exists:

```
$ docker run -ti alpine
# mkdir -p /tmp/foo/bar; touch /tmp/foo/bar/a /tmp/foo/bar/b
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/bar'
'/tmp/foo/bar/..' -> '/tmp/foo2'
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/../bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/../bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/../bar'
'/tmp/foo/bar/..' -> '/tmp/foo2/..'
```

We'll dodge this by using Julia's `cp()` function.
@@ -640,7 +640,7 @@ U41096 = Term41096{:U}(Modulate41096(:U, false))

# test that we can start julia with libjulia-codegen removed; PR #41936
mktempdir() do pfx
run(`cp -r $(Sys.BINDIR)/.. $pfx`)
cp(dirname(Sys.BINDIR), pfx; force=true)
run(`rm -rf $pfx/lib/julia/libjulia-codegen\*`)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

we are not allowed to assume rm exists either

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

What's your favorite way to do glob-removal? filter(readdir())?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

That or just a loop sounds fine to me.

Note that we use `recursive=true` as we might encounter a `libjulia-codegen.X.Y.dylib.dSYM` directory.  Although deleting that directory is not critical for this test, not throwing an exception is, so we just remove it as well.  Also, this test would silently fail to test what we want it to if run out of a build directory, so we first teach it to dynamically find the location of `libjulia-codegen`, then secondly, cause it to fail if it does not remove any `libjulia-codegen` libraries.
@JeffBezanson JeffBezanson added the test This change adds or pertains to unit tests label Nov 1, 2021
@JeffBezanson JeffBezanson merged commit 52ff195 into master Nov 1, 2021
@JeffBezanson JeffBezanson deleted the sf/preexisting_copy_behavior branch November 1, 2021 20:47
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
Apparently, busybox has strange behavior when you copy `$pfx/..` and the destination directory exists:

```
$ docker run -ti alpine
# mkdir -p /tmp/foo/bar; touch /tmp/foo/bar/a /tmp/foo/bar/b
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/bar'
'/tmp/foo/bar/..' -> '/tmp/foo2'
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/../bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/../bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/../bar'
'/tmp/foo/bar/..' -> '/tmp/foo2/..'
```

We'll dodge this by using Julia's `cp()` function.

* Eliminate usage of `rm` command-line program

Note that we use `recursive=true` as we might encounter a `libjulia-codegen.X.Y.dylib.dSYM` directory.  Although deleting that directory is not critical for this test, not throwing an exception is, so we just remove it as well.  Also, this test would silently fail to test what we want it to if run out of a build directory, so we first teach it to dynamically find the location of `libjulia-codegen`, then secondly, cause it to fail if it does not remove any `libjulia-codegen` libraries.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Apparently, busybox has strange behavior when you copy `$pfx/..` and the destination directory exists:

```
$ docker run -ti alpine
# mkdir -p /tmp/foo/bar; touch /tmp/foo/bar/a /tmp/foo/bar/b
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/bar'
'/tmp/foo/bar/..' -> '/tmp/foo2'
# cp -vr /tmp/foo/bar/.. /tmp/foo2
'/tmp/foo/bar/../bar/a' -> '/tmp/foo2/../bar/a'
'/tmp/foo/bar/../bar/b' -> '/tmp/foo2/../bar/b'
'/tmp/foo/bar/../bar' -> '/tmp/foo2/../bar'
'/tmp/foo/bar/..' -> '/tmp/foo2/..'
```

We'll dodge this by using Julia's `cp()` function.

* Eliminate usage of `rm` command-line program

Note that we use `recursive=true` as we might encounter a `libjulia-codegen.X.Y.dylib.dSYM` directory.  Although deleting that directory is not critical for this test, not throwing an exception is, so we just remove it as well.  Also, this test would silently fail to test what we want it to if run out of a build directory, so we first teach it to dynamically find the location of `libjulia-codegen`, then secondly, cause it to fail if it does not remove any `libjulia-codegen` libraries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants