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.

Sign up for GitHub

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

Jump to bottom

oneliner helper does not allow variables #355

Closed
atoomic opened this issue Jul 20, 2020 · 11 comments · May be fixed by #356
Closed

oneliner helper does not allow variables #355

atoomic opened this issue Jul 20, 2020 · 11 comments · May be fixed by #356

Comments

@atoomic
Copy link
Contributor

atoomic commented Jul 20, 2020

When reading the unit test for the sub oneliner from t/oneliner.t

try_oneliner(q{$PATH = 'foo'; print $PATH},[], q{foo},   'dollar signs' );

You have the feeling that oneliner authorizes the usage of variables, all the most the test is passing.
But in truth it's using references:

# $(ABSPERLRUN)  -e '$$PATH = '\''foo'\''; print $$PATH' --

I wonder if we should teach sub oneliner to allow_variables when calling quote_literal, or detect incorrect usage of variable...

For example you can use %h but then it's going to fail when using $h later to access a value...

Detail:

my %h = (1..4); is a valid oneliner

$mm->oneliner( q[my %h = (1..4);]);
# $(ABSPERLRUN)  -e 'my %h = (1..4); print $$h[1]' --

but my %h = (1..4); print $h[1] is invalid

$mm->oneliner( q[my %h = (1..4);]);
# $(ABSPERLRUN)  -e 'my %h = (1..4); print $$h[1]' --

#!perl

use Test::More;

require ExtUtils::MM;

my $mm = MM->new(
	{ NAME => q[Fake], }
);

my $cmd;

# valid - using ref
my $cmd = $mm->oneliner( q[$PATH = 'foo'; print $PATH]);
note $cmd;
# $(ABSPERLRUN)  -e '$$PATH = '\''foo'\''; print $$PATH' --

# invalid
my $cmd = $mm->oneliner( q[my $PATH = 'foo'; print $PATH]);
note $cmd;
# $(ABSPERLRUN)  -e 'my $$PATH = '\''foo'\''; print $$PATH' --
@Leont
Copy link
Member

Leont commented Jul 20, 2020

But in truth it's using references:

What are you talking about? That looks like a oneliner correctly escaped to be used in a Makefile.

@atoomic
Copy link
Contributor Author

atoomic commented Jul 20, 2020 •
edited
Loading

$mm->oneliner( q[$PATH = 'foo'; print $PATH] ); converts to '$$PATH = '\''foo'\''; print $$PATH'
so this is working fine when not doing something else than that kind of $ usage...
$PATH is a reference and not one scalar value as we could thing

proof in image

╰─> perl -e '$$a = 42; use Devel::Peek; Dump $a; print $$a'
SV = IV(0x7f879c024f00) at 0x7f879c024f10
  REFCNT = 1
  FLAGS = (ROK)
  RV = 0x7f879c00ade0
  SV = IV(0x7f879c00add0) at 0x7f879c00ade0
    REFCNT = 1
    FLAGS = (IOK,pIOK)
    IV = 42
42%

@Leont
Copy link
Member

Leont commented Jul 20, 2020

Given this Makefile

all:
    perl -e '$$a = 42; use Devel::Peek; Dump $$a'

Try running make

@atoomic
Copy link
Contributor Author

atoomic commented Jul 20, 2020

Another way to show the problem is to add a simple my before $PATH = 'foo'; print $PATH
$mm->oneliner( q[my $PATH = 'foo'; print $PATH] );
which convert the code to
# $(ABSPERLRUN) -e 'my $$PATH = '\''foo'\''; print $$PATH' --
which is invalid Perl code... as shown just after

╰─> perl -e 'my $$PATH = '\''foo'\''; print $$PATH'
Can't declare scalar dereference in "my" at -e line 1, near "$PATH ="
Execution of -e aborted due to compilation errors.

@atoomic
Copy link
Contributor Author

atoomic commented Jul 20, 2020

another example is to try one liner using a simple hash
my %h = (1 => 2); print $h{1};
will then convert to
# $(ABSPERLRUN) -e 'my %h = (1 => 2); print $$h{1}; ' --
which is not running what we think should run

@Leont Leont closed this as completed Jul 20, 2020
@Grinnz
Copy link
Contributor

Grinnz commented Jul 20, 2020

As @Leont said this is not generating Perl code, it is generating Perl code escaped for use in a Makefile.

@atoomic
Copy link
Contributor Author

atoomic commented Jul 20, 2020

Thanks for the clarification, once used in a Makefile, there is no issue.

Unfortunately the unit test t/oneliner.t is not consuming it via a Makefile, which explains the confusion.

view https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/blob/master/t/oneliner.t#L33

@mohawk2
Copy link
Member

mohawk2 commented Jul 21, 2020

@atoomic You are asserting as fact that perl -E 'say $$x' in a Makefile recipe ends up as dereferencing a scalar. Read the documentation for make and how it interprets $ in its recipes.

@mohawk2
Copy link
Member

mohawk2 commented Jul 21, 2020

The oneliner function is only supposed to produce things for use in a Makefile. Surely this is not that baffling an idea in a module called "MakeMaker".

atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 21, 2020
@atoomic
Test oneliner without double escaping dollar sign
5b7bb4a 
Fix Perl-Toolchain-Gang#355
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 21, 2020
@atoomic
Test oneliner without double escaping dollar sign
253de6b 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

This change is 'unescaping' the '$$' to '$' so we can perform
some extra checks while using oneliner.

Otherwise as shown in this example a simple 'my $foo' test will fail.
@atoomic atoomic mentioned this issue Jul 21, 2020
Open
@atoomic
Copy link
Contributor Author

atoomic commented Jul 21, 2020

My main concern is the way we perform the test t/oneliner.t and leads to use references.
I've no problem with the double escaping itself which is required for Makefile syntax.

Please view and consider #356

@atoomic
Copy link
Contributor Author

atoomic commented Jul 21, 2020

a simple test like my $x in t/oneliner.t will show the issue, you can for example add a simple my when testing the oneliner function:
try_oneliner(q{my $PATH = 'foo'; print $PATH},[], q{foo}, 'dollar signs' );

which will lead to this error

t/oneliner.t ..
1..16
ok 1 - use ExtUtils::MM;
ok 2 - An object of class 'MM' isa 'ExtUtils::MakeMaker'
ok 3 - An object of class 'MM' isa 'ExtUtils::MM_Any'
ok 4 - quotes
Can't declare scalar dereference in "my" at -e line 1, near "$PATH ="
Execution of -e aborted due to compilation errors.
not ok 5 - dollar signs

#   Failed test 'dollar signs'
#   at t/oneliner.t line 41.
#          got: ''
#     expected: 'foo'
# oneliner:
# "/Users/nicolas/perl5/perlbrew/perls/perl-5.30.1/bin/perl"  -e 'my $$PATH = '\''foo'\''; print $$PATH' --

atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 21, 2020
@atoomic
Test oneliner without double escaping dollar sign
db19381 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

This change is 'unescaping' the '$$' to '$' so we can perform
some extra checks while using oneliner.

Otherwise as shown in this example a simple 'my $foo' test will fail.
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 22, 2020
@atoomic
Test oneliner without double escaping dollar sign
8eda217 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

This change is 'unescaping' the '$$' to '$' so we can perform
some extra checks while using oneliner.

Otherwise as shown in this example a simple 'my $foo' test will fail.

Note we also have to unescape braces '{{' and '}}' used in dmake...
A better solution would be to produce a Makefile and run it directly.
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 22, 2020
@atoomic
Test oneliner using a temporary Makefile
c0319fa 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

We could consider unescpaing '$$', '{{', '}}'... (for dmake)
but a cleaner solution is to test the oneliner in a Makefile
itself.
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 22, 2020
@atoomic
Test oneliner using a temporary Makefile
74d9a09 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

We could consider unescpaing '$$', '{{', '}}'... (for dmake)
but a cleaner solution is to test the oneliner in a Makefile
itself.
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 22, 2020
@atoomic
Test oneliner using a temporary Makefile
ca37cb2 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

We could consider unescpaing '$$', '{{', '}}'... (for dmake)
but a cleaner solution is to test the oneliner in a Makefile
itself.
atoomic added a commit to atoomic/ExtUtils-MakeMaker that referenced this issue Jul 22, 2020
@atoomic
Test oneliner using a temporary Makefile
9a150dc 
Fix Perl-Toolchain-Gang#355

Double escaping '$' for Makefile usage is fine, but
we should not use such a syntax while testing and trying
to run the oneliner output.

We could consider unescpaing '$$', '{{', '}}'... (for dmake)
but a cleaner solution is to test the oneliner in a Makefile
itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Test oneliner without double escaping dollar sign atoomic/ExtUtils-MakeMaker
4 participants
@Leont @atoomic @Grinnz @mohawk2