Skip to content

spec-test-script/runtest.py: don't assume the tmp dir path#3632

Merged
wenyongh merged 1 commit intobytecodealliance:mainfrom
yamt:test-tmpdir
Jul 18, 2024
Merged

spec-test-script/runtest.py: don't assume the tmp dir path#3632
wenyongh merged 1 commit intobytecodealliance:mainfrom
yamt:test-tmpdir

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Jul 16, 2024

The current coding assumes the tmp file is created in /tmp. However,

  • It's somewhere under /var/folders/... by default on macOS.

  • A user can set TMPDIR.

The current coding assumes the tmp file is created in /tmp.
However,

* It's somewhere under /var/folders/... by default on macOS.

* A user can set TMPDIR.
@wenyongh wenyongh requested a review from no1wudi July 17, 2024 11:35
log("Starting interpreter for module '%s'" % tmpfile)

if opts.qemu:
tmpfile = f"/tmp/{os.path.basename(tmpfile)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is better to move L1159 to be after these two lines, or the log may be not so accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

L1159 is the path on the host.
L1162 is the path in qemu.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, thanks.

Copy link
Collaborator

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM

log("Starting interpreter for module '%s'" % tmpfile)

if opts.qemu:
tmpfile = f"/tmp/{os.path.basename(tmpfile)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, thanks.

@wenyongh wenyongh merged commit 7c6fc70 into bytecodealliance:main Jul 18, 2024
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.

3 participants