Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-23 Thread Panu Matilainen
> Should be easy to arrange by using poptParseArgvString() instead of 
> argvSplit()

except that poptParseArgvString() strips out empty arguments, so it doesn't 
work for this purpose.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303315607___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-23 Thread Igor Gnatenko
Btw, if you will pass -e or -u (didn't remember) it will fail due to
undefined variable.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303306420___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-23 Thread Panu Matilainen
Hmm. OTOH that "argument shift" is exactly what happens with shell too:

```
[pmatilai@sopuli ~]$ cat argtest.sh 
function foo()
{
echo 1:$1 2:$2
}

foo $none bar
[pmatilai@sopuli ~]$ sh argtest.sh 
1:bar 2:
```

Difference of course being that shell has a quoting mechanism. So maybe the 
saner thing is reverting expand-after-split afterall and supporting quotes 
around arguments instead, to permit empty arguments where necessary. Should be 
easy to arrange by using poptParseArgvString() instead of argvSplit(). And sure 
it's going to break something else in turn, but *something* is going to break 
no matter what, and behaving similar to shell seems like the path of least 
surprises.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303306045___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-22 Thread Panu Matilainen
You pretty much convinced me expand after split is by far the saner thing to do 
with this example:
```
$ rpm --define "%foo() 1:%1 2:%2" --eval "%foo %nil bar"
1:bar 2:%2
```

At least in the %autosetup case, the empty argument is harmless, and I think 
given the %nil example, it could be considered a feature. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303097586___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-22 Thread Michael Schroeder
(but you also have the problem that there will be an empty argument if 
with_int_bdb is not defined. Hmm. So revert that "expand after split" commit? 
Too bad rpm does not have some quoting mechanism.)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303065026___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-22 Thread Panu Matilainen
Yes it'd be a good thing, just not what I'd like to deal with right now. 
Patches would be most welcome :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303060871___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-22 Thread Michael Schroeder
Yes, I also ran into that problem. But it's compatible to before ;)

Anyway, making the split macro aware would IMHO be a good thing. It would also 
make it more consistent in regard to what expandMacro itself does, i.e. call 
matchchar to find the end of a %{ or %( macro.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303050099___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-22 Thread Panu Matilainen
Hmm, expanding after splitting causes different problems (surprise surprise) 
because, for example this breaks:
```%autosetup -n %{name}-%{srcver} %{?with_int_bdb:-a 1} -p1```

In this case it can be worked around by just dropping the space between -a and 
1, but in general the split would now need to be aware of the macro syntax as 
well. Probably quotes too.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-303028790___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-18 Thread Panu Matilainen
...and the initial case fixed in 767d61ca3dba9745d392fa28bbe09a209bd49522.

Again, thanks for spotting and reporting! It's s much nicer to fix such 
things pre-release time at your leisure without having an angry mob of 
packagers yelling "you bastard you broke my stuff" at you :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-302370343___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-18 Thread Panu Matilainen
Closed #217.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#event-1087571377___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-18 Thread Panu Matilainen
The latter case fixed now in commit 9ae7d1df313b7a2b9fd74fef5a176dcdce40b88b, 
thanks for the cases!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-302358315___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-18 Thread Panu Matilainen
Funny how things seem clearer without a headache...

Obviously one needs to be able to pass escaped macros as arguments, another 
example to show current non-sensical behavior:
```
$ rpm --define "%foo() %1" --eval "%foo %{_lib}" --eval "%foo %%{_lib}"
lib64
lib64
```

I have a fix for that, just trying to decide whether the "escape mode" would 
ever be useful outside macro internals. The %nil case is interesting, the 
current behavior where the argument shifts by one is certainly not sensical.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-302349680___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] rpm expands macro args twice (#217)

2017-05-18 Thread Michael Schroeder
I also think you should expand the macros after splitting the arguments, so 
that the behavior is more useful and compatible. Example:
```
%foo() 1:%1 2:%2
```
And `rpm --eval "%foo %nil bar` should return `1: 2:bar` like before.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/217#issuecomment-302345943___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint