[pacman-dev] [PATCH] repo-add: don't break if delta package sources contain epoch

2018-12-27 Thread Eli Schwartz
Our sed parser for xdelta3 headers will greedily match on ":" which
coincidentally is also the character we use to define a version with an
epoch.

While we are at it, simply use sed for the whole pipeline, rather than
using both grep and sed.

Fixes FS#61195

Signed-off-by: Eli Schwartz 
---
 scripts/repo-add.sh.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index bccf2f37..b9c6a6cc 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -159,8 +159,8 @@ db_write_delta() {
md5sum=${md5sum%% *}
csize=$(wc -c "$deltafile" | cut -d' ' -f1)
 
-   oldfile=$(xdelta3 printhdr "$deltafile" | grep "XDELTA filename 
(source)" | sed 's/.*: *//')
-   newfile=$(xdelta3 printhdr "$deltafile" | grep "XDELTA filename 
(output)" | sed 's/.*: *//')
+   oldfile=$(xdelta3 printhdr "$deltafile" | sed -n 's/XDELTA filename 
(source):\s\+\(\.*\)/\1/p')
+   newfile=$(xdelta3 printhdr "$deltafile" | sed -n 's/XDELTA filename 
(output):\s\+\(\.*\)/\1/p')
 
if grep -q "$oldfile.*$newfile" "$deltas"; then
sed -i.backup "/$oldfile.*$newfile/d" "$deltas" && rm -f 
"$deltas.backup"
-- 
2.20.1


Re: [pacman-dev] [PATCH] repo-add: don't break if delta package sources contain epoch

2019-01-03 Thread Allan McRae
On 28/12/18 9:15 am, Eli Schwartz wrote:
> Our sed parser for xdelta3 headers will greedily match on ":" which
> coincidentally is also the character we use to define a version with an
> epoch.
> 
> While we are at it, simply use sed for the whole pipeline, rather than
> using both grep and sed.
> 
> Fixes FS#61195
> 
> Signed-off-by: Eli Schwartz 

OK.

As an aside, the fact we are rewriting package version parsing code in
bash highlights again that repo-add really should be converted to a
libalpm program.

Allan


Re: [pacman-dev] [PATCH] repo-add: don't break if delta package sources contain epoch

2019-01-03 Thread Eli Schwartz
On 1/3/19 8:20 PM, Allan McRae wrote:
> On 28/12/18 9:15 am, Eli Schwartz wrote:
>> Our sed parser for xdelta3 headers will greedily match on ":" which
>> coincidentally is also the character we use to define a version with an
>> epoch.
>>
>> While we are at it, simply use sed for the whole pipeline, rather than
>> using both grep and sed.
>>
>> Fixes FS#61195
>>
>> Signed-off-by: Eli Schwartz 
> 
> OK.
> 
> As an aside, the fact we are rewriting package version parsing code in
> bash highlights again that repo-add really should be converted to a
> libalpm program.

Ha, no arguments there. :p

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] repo-add: don't break if delta package sources contain epoch

2019-01-04 Thread Dave Reisner
On Fri, Jan 04, 2019 at 11:20:04AM +1000, Allan McRae wrote:
> On 28/12/18 9:15 am, Eli Schwartz wrote:
> > Our sed parser for xdelta3 headers will greedily match on ":" which
> > coincidentally is also the character we use to define a version with an
> > epoch.
> > 
> > While we are at it, simply use sed for the whole pipeline, rather than
> > using both grep and sed.
> > 
> > Fixes FS#61195
> > 
> > Signed-off-by: Eli Schwartz 
> 
> OK.
> 
> As an aside, the fact we are rewriting package version parsing code in
> bash highlights again that repo-add really should be converted to a
> libalpm program.

Not sure I understand this -- isn't the problem here the crappy output
format of xdelta3? We're not really parsing versions, just contending
with the human readable, colon-delimited table that we're forced to use.
Rewriting against libalpm won't fix anything here, though if we were to
rewrite in C, we could link to the "hidden" xdelta3 library or implement
our own crappy vcdiff parser (eww, please no).

I don't want to open a can of worms (maybe I do), but I think delta
handling is the least of the problems with repo-add. I'm aware of many
users (myself previously included) that would either replace repo-add
entirely, or wrap it substantially in order to make it truly useful. One
such example is respose [0]. If we want to consider a rewrite, maybe we
should rethink our approach to repo management and make a better tool,
not just an equivalent one.

dR

[0] https://github.com/vodik/repose


Re: [pacman-dev] [PATCH] repo-add: don't break if delta package sources contain epoch

2019-01-04 Thread Allan McRae
On 5/1/19 9:23 am, Dave Reisner wrote:
> On Fri, Jan 04, 2019 at 11:20:04AM +1000, Allan McRae wrote:
>> On 28/12/18 9:15 am, Eli Schwartz wrote:
>>> Our sed parser for xdelta3 headers will greedily match on ":" which
>>> coincidentally is also the character we use to define a version with an
>>> epoch.
>>>
>>> While we are at it, simply use sed for the whole pipeline, rather than
>>> using both grep and sed.
>>>
>>> Fixes FS#61195
>>>
>>> Signed-off-by: Eli Schwartz 
>>
>> OK.
>>
>> As an aside, the fact we are rewriting package version parsing code in
>> bash highlights again that repo-add really should be converted to a
>> libalpm program.
> 
> Not sure I understand this -- isn't the problem here the crappy output
> format of xdelta3? We're not really parsing versions, just contending
> with the human readable, colon-delimited table that we're forced to use.
> Rewriting against libalpm won't fix anything here, though if we were to
> rewrite in C, we could link to the "hidden" xdelta3 library or implement
> our own crappy vcdiff parser (eww, please no).

Yes - my initial through was this was fixing parsing of versions from
nicer output, but after I sent that emails I saw that was confounded
with xdelta output "goodness"...

> I don't want to open a can of worms (maybe I do), but I think delta
> handling is the least of the problems with repo-add. I'm aware of many
> users (myself previously included) that would either replace repo-add
> entirely, or wrap it substantially in order to make it truly useful. One
> such example is respose [0]. If we want to consider a rewrite, maybe we
> should rethink our approach to repo management and make a better tool,
> not just an equivalent one.

I agree.  I was going to come back to the design of the new replacement
after the writer had been added to libalpm.  I like repose, but I found
it does not really work if you wanted more than one repo in a package
directory.  It is very focused to a single workflow.  Saying that, I
think we can do better than the current repo-add which is far from great...

Allan