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

make t/spec/foo/bar.t modifies the current installation #6037

Open
p6rt opened this issue Jan 28, 2017 · 12 comments
Open

make t/spec/foo/bar.t modifies the current installation #6037

p6rt opened this issue Jan 28, 2017 · 12 comments

Comments

@p6rt
Copy link

p6rt commented Jan 28, 2017

Migrated from rt.perl.org#130660 (status was 'open')

Searchable as RT130660$

@p6rt
Copy link
Author

p6rt commented Jan 28, 2017

From @toolforger

Testing an installation should not modify it, however make -n for an
individual test file gives me this​:

~/try-out-roast/rakudo$ make -n t/spec/S22-package-format/local.t
rm -f -- perl6
cp -- perl6-m perl6
chmod -- 755 perl6
/usr/bin/perl t/harness5 --fudge --moar --keep-exit-code --verbosity=1
t/spec/S22-package-format/local.t

I.e. it's copying perl6-m over perl6, which it shouldn't do.

@p6rt
Copy link
Author

p6rt commented Jan 28, 2017

From @zoffixznet

On Sat, 28 Jan 2017 02​:10​:47 -0800, toolforger@​durchholz.org wrote​:

I.e. it's copying perl6-m over perl6,

I assume that's to select which backend to use for testing. `perl6` is just a 1-line wrapper bash script.
That make target will also recompile the compiler if you made any changes to it. Very handy during development.

Testing an installation should not modify it

From `make -n test` I see that it doesn't. Roast contains language compliance tests,
not a verification of a installation. You can run them with `t/fudgeandrun` tool
if you don't want an automated recompilation of changes that all the core devs need.

A make target can be made that just runs the roast tests. What benefits will we attain by having such an option?

@p6rt
Copy link
Author

p6rt commented Jan 28, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @toolforger

My approach to solving this would be to have make test use perl-m resp.
perl-j directly.

Adding another make target still leaves "make test" overwriting the
perl6 file, which might be surprising if somebody has explicitly set up
perl6 to mean (say) perl6-j.

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @zoffixznet

On Sun, 29 Jan 2017 01​:56​:16 -0800, jo@​durchholz.org wrote​:

My approach to solving this would be to have make test use perl-m resp.
perl-j directly.

OK. It already does so. As stated above, `make test` doesn't overwrite anything.

Closing.

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

@zoffixznet - Status changed from 'open' to 'rejected'

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @toolforger

Am 29.01.2017 um 14​:58 schrieb Zoffix Znet via RT​:

On Sun, 29 Jan 2017 01​:56​:16 -0800, jo@​durchholz.org wrote​:

My approach to solving this would be to have make test use perl-m resp.
perl-j directly.

OK. It already does so.

Then it does not need to overwrite perl6.

As stated above, `make test` doesn't overwrite anything.

This statement is in direct contradiction to reported observation.
Cf. the relevant section in my initial post​:

~/try-out-roast/rakudo$ make -n t/spec/S22-package-format/local.t
rm -f -- perl6
cp -- perl6-m perl6
chmod -- 755 perl6

Can you explain?

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @zoffixznet

On Sun, 29 Jan 2017 08​:46​:28 -0800, jo@​durchholz.org wrote​:

Can you explain?

You're not running `make test` in that command.

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

@zoffixznet - Status changed from 'rejected' to 'open'

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @zoffixznet

On Sun, 29 Jan 2017 09​:03​:34 -0800, cpan@​zoffix.com wrote​:

On Sun, 29 Jan 2017 08​:46​:28 -0800, jo@​durchholz.org wrote​:

Can you explain?

You're not running `make test` in that command.

A few data points​:
- none of make spectest, make stresstest, nor make test do the overwrite-perl6 thing
- grepping the roast, I see a few places mentioning `./perl6`
- removing `./perl6` and running make stresstest doesn't have any issues
- `make t/some/file` copies over `perl6`, but seemingly pointlessly, as the test is run with t/harness --moar which uses `perl6-m` and not the `perl6`

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @toolforger

Am 29.01.2017 um 18​:03 schrieb Zoffix Znet via RT​:

On Sun, 29 Jan 2017 08​:46​:28 -0800, jo@​durchholz.org wrote​:

Can you explain?

You're not running `make test` in that command.

Oh.
You mentioned `make test` in your first answer, so I was just assuming
any differences would be irrelevant.

@p6rt
Copy link
Author

p6rt commented Jan 29, 2017

From @toolforger

Am 29.01.2017 um 19​:28 schrieb Zoffix Znet via RT​:

On Sun, 29 Jan 2017 09​:03​:34 -0800, cpan@​zoffix.com wrote​:

On Sun, 29 Jan 2017 08​:46​:28 -0800, jo@​durchholz.org wrote​:

Can you explain?

You're not running `make test` in that command.

A few data points​:
- none of make spectest, make stresstest, nor make test do the
overwrite-perl6 thing
- grepping the roast, I see a few places mentioning `./perl6`
- removing `./perl6` and running make stresstest doesn't have any issues

Thanks, I find that kind of lateral information useful.

(Just for the record​: I'd prefer it if roast didn't access ./perl6.)

- `make t/some/file` copies over `perl6`, but seemingly pointlessly,
as the test is run with t/harness --moar which uses `perl6-m` and not
the `perl6`

Agreeing with observation and conclusions.

Maybe the fix is as simple as removing the rm/cp/chmod commands.
I'd try that out myself given my pretty minimal knowledge of perl6
infrastructure, I'd likely miss important workflows so my testing would
be nearly pointless.

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

No branches or pull requests

1 participant