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

modify fds-are-cloexec test to open a file that exists #35885

Merged
merged 2 commits into from
Aug 26, 2016

Conversation

durka
Copy link
Contributor

@durka durka commented Aug 21, 2016

Fixes #35753.

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

r? @nagisa

@retep998
Copy link
Member

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

I'd argue it is not.

@durka
Copy link
Contributor Author

durka commented Aug 22, 2016

Maybe the test should create a temporary file, close it, and then open it again?

@nagisa
Copy link
Member

nagisa commented Aug 22, 2016

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

If such an assumption was “good enough” for the version opening Makefile, I feel it is good enough for the new version as well. You could also use any of the non-autogenerated files from project root, like LICENSE-*.

@eddyb
Copy link
Member

eddyb commented Aug 23, 2016

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Aug 23, 2016

📌 Commit b40754f has been approved by nagisa

eddyb added a commit to eddyb/rust that referenced this pull request Aug 23, 2016
modify fds-are-cloexec test to open a file that exists

Fixes rust-lang#35753.

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

r? @nagisa
eddyb added a commit to eddyb/rust that referenced this pull request Aug 23, 2016
modify fds-are-cloexec test to open a file that exists

Fixes rust-lang#35753.

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

r? @nagisa
Manishearth added a commit to Manishearth/rust that referenced this pull request Aug 25, 2016
modify fds-are-cloexec test to open a file that exists

Fixes rust-lang#35753.

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

r? @nagisa
bors added a commit that referenced this pull request Aug 25, 2016
Rollup of 6 pull requests

- Successful merges: #35238, #35867, #35885, #35916, #35947, #35955
- Failed merges:
@Manishearth
Copy link
Member


failures:

---- [run-pass] run-pass/fds-are-cloexec.rs stdout ----

error: test run failed!
status: exit code: 101
command: x86_64-apple-darwin/test/run-pass/fds-are-cloexec.stage2-x86_64-apple-darwin 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 2, message: "No such file or directory" } }', ../src/libcore/result.rs:789
note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

thread '[run-pass] run-pass/fds-are-cloexec.rs' panicked at 'explicit panic', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/tools/compiletest/src/runtest.rs:2350
note: Run with `RUST_BACKTRACE=1` for a backtrace.

https://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/10287/steps/test/logs/stdio

bors added a commit that referenced this pull request Aug 25, 2016
Rollup of 6 pull requests

- Successful merges: #35238, #35867, #35885, #35916, #35947, #35955
- Failed merges:
@durka
Copy link
Contributor Author

durka commented Aug 25, 2016

I guess... it's not a valid assumption? Is that a non-rustbuild builder?

On Aug 25, 2016 07:36, "Manish Goregaokar" notifications@github.com wrote:

failures:

---- [run-pass] run-pass/fds-are-cloexec.rs stdout ----

error: test run failed!
status: exit code: 101
command: x86_64-apple-darwin/test/run-pass/fds-are-cloexec.stage2-x86_64-apple-darwin

stdout:


stderr:

thread 'main' panicked at 'called Result::unwrap() on an Err value: Error { repr: Os { code: 2, message: "No such file or directory" } }', ../src/libcore/result.rs:789
note: Run with RUST_BACKTRACE=1 for a backtrace.


thread '[run-pass] run-pass/fds-are-cloexec.rs' panicked at 'explicit panic', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/tools/compiletest/src/runtest.rs:2350
note: Run with RUST_BACKTRACE=1 for a backtrace.

https://buildbot.rust-lang.org/builders/auto-mac-64-opt/
builds/10287/steps/test/logs/stdio


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#35885 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC3n76SLVGLkXaQvi8Do_eQMg9FltSZks5qjX4ggaJpZM4JpbCD
.

@nagisa
Copy link
Member

nagisa commented Aug 25, 2016

Thats make. It should operate The same way since there can only ever be one
Makefile in the source tree, in root (since LLVM should not be using
those). The fact that the command is a relative path to the built
executable from the build root, also demonstrates a similar thing.

Anyhow, if a path to the test does not work, then in order to avoid writing
code to create temp file converting the test to a run-make test could be
done.

On Aug 25, 2016 4:53 PM, "Alex Burka" notifications@github.com wrote:

I guess... it's not a valid assumption? Is that a non-rustbuild builder?

On Aug 25, 2016 07:36, "Manish Goregaokar" notifications@github.com
wrote:

failures:

---- [run-pass] run-pass/fds-are-cloexec.rs stdout ----

error: test run failed!
status: exit code: 101
command: x86_64-apple-darwin/test/run-pass/fds-are-cloexec.stage2-
x86_64-apple-darwin

stdout:


stderr:

thread 'main' panicked at 'called Result::unwrap() on an Err value:
Error { repr: Os { code: 2, message: "No such file or directory" } }',
../src/libcore/result.rs:789
note: Run with RUST_BACKTRACE=1 for a backtrace.


thread '[run-pass] run-pass/fds-are-cloexec.rs' panicked at 'explicit
panic', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-
opt/build/src/tools/compiletest/src/runtest.rs:2350
note: Run with RUST_BACKTRACE=1 for a backtrace.

https://buildbot.rust-lang.org/builders/auto-mac-64-opt/
builds/10287/steps/test/logs/stdio


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#35885 (comment),
or mute
the thread
<https://github.com/notifications/unsubscribe-
auth/AAC3n76SLVGLkXaQvi8Do_eQMg9FltSZks5qjX4ggaJpZM4JpbCD>
.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35885 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AApc0lKVSeCYp3C4eDVuzD1i9UUF0ictks5qjZ5fgaJpZM4JpbCD
.

@arielb1
Copy link
Contributor

arielb1 commented Aug 25, 2016

This will break out-of-tree builds.

@durka
Copy link
Contributor Author

durka commented Aug 25, 2016

@arielb1 I guess they didn't work before either.

@durka
Copy link
Contributor Author

durka commented Aug 25, 2016

The test already doesn't run on Windows, how about opening /dev/null?

@arielb1
Copy link
Contributor

arielb1 commented Aug 25, 2016

@durka

Out-of-tree builds did work before. I was using them all the time.

I imagine you could open file!().

@durka
Copy link
Contributor Author

durka commented Aug 25, 2016

Well, it was assuming that the Makefile was in the current directory. I
don't see how the builder failed so I could be missing something glaringly
obvious still.

On Thu, Aug 25, 2016 at 12:33 PM, arielb1 notifications@github.com wrote:

@durka https://github.com/durka

Out-of-tree builds did work before. I was using them all the time.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35885 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC3nzNPR6VnwdgoGg5z92C83rZwfXcYks5qjcPjgaJpZM4JpbCD
.

@arielb1
Copy link
Contributor

arielb1 commented Aug 25, 2016

@durka

The way you do an out-of-tree build is

$ mkdir out-of-tree
$ cd out-of-tree
$ PATH_TO_RUST_ROOT/configure FLAGS
$ make

That way, you end up with a Makefile in your builddir.

@arielb1
Copy link
Contributor

arielb1 commented Aug 25, 2016

You can do File::open(file!()).unwrap() for maximum robustness.

@durka
Copy link
Contributor Author

durka commented Aug 25, 2016

Let's try that.

@arielb1
Copy link
Contributor

arielb1 commented Aug 25, 2016

@bors r+

I don't want to rollup this

@bors
Copy link
Contributor

bors commented Aug 25, 2016

📌 Commit b2cd3e5 has been approved by arielb1

@bors
Copy link
Contributor

bors commented Aug 25, 2016

⌛ Testing commit b2cd3e5 with merge e07dd59...

bors added a commit that referenced this pull request Aug 25, 2016
modify fds-are-cloexec test to open a file that exists

Fixes #35753.

Is it a valid assumption that the current directory is always the root of the repo when the tests are run?

r? @nagisa
@bors bors merged commit b2cd3e5 into rust-lang:master Aug 26, 2016
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.

None yet

7 participants