Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Panu Matilainen]

Closed #109.

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#event-927531760___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Panu Matilainen]

but the quilt backend is probably relying on quirks as it is now, need to 
look closer.

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#issuecomment-273474909___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Panu Matilainen]

Oh, I know about the patches_num patch. I don't much like it, and thought 
ordinals aren't that helpful. Which is why there are no backups currently. I 
was kinda hoping you guys had come up with some nicer way to generate backups 
with patch numbers when I saw this ;)

Anyway, on a second thought - if you're stuck on a desert island with nothing 
but patch and gendiff, backups by ordinal are *worlds* better than nothing at 
all.

As for the implementation, I think it'd be better to pass the ordinal as-is to 
the backends who are free to do whatever they wish (including ignore) the 
additional information. Rather than force a specific patch-backup oriented 
format all the way at the top, that is. It also relies on quirks in the rpm 
macro system (referring to an option passed to a calling macro) and breaks at 
least the quilt backend (because of that).

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#issuecomment-273470825___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Panu Matilainen]

pmatilai requested changes on this pull request.

I'm actually in favor of enabling backups by default. However this patch 
clearly hasn't been tested, nor reviewed, at all. It refers to a non-existent 
patches_num variable so any attempt to use %autosetup with this patch ends up 
in:

`error: lua script failed: [string ""]:4: attempt to index a nil value 
(global 'patches_num')`




-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#pullrequestreview-16144566___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[proyvind]

Taking a closer look at the mageia bug, I notice that the backup files lacks 
the '~' backup suffix, for which I'd consider this rather a mageia specific 
shortcoming in their %apply_patches implementation, cooker's not affected by..

The only scare thing about mageia's past use of %apply_patches I see, is faiure 
of producing proper/standardized backup files.

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#issuecomment-266467946___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[proyvind]

backup files included in packages are nothing but a result of faulty packaging, 
where disablingdefault non-intrusive functionality of use are clearly not the 
right solution and really should be considered flawed packaging practices and 
workarounds rather than correctly addressing and fixing the issue, a matter 
applied to software development in general.

Prevention of this should already be ensured through rpmlint checks, where 
graded as error by default IIRC.

Further on, the truly proper, automatic and guaranteed prevention of this to 
even be possible is through brp-* scripts run at end of %install.

For cooker we have an almost two decades old spec-helper package with 
additional scripts to ensure packaging sanity, which I've over the past years 
have implemented even further extensive scripts, while cleaning them up (ie. 
such as spaghetti perl scripts difficult to maintain and read have been 
rewritten in shell scripts easy to understand and maintain), which now really 
is in a shape where not having pushed upstream earlier is almost an outrage.. 
:p 

I will create a branch with these commited to soon and create a pull request 
for.

FWIW: We've *never* actually experienced this as an actual problem in cooker..

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#issuecomment-266466153___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Igor Gnatenko]

@rpm-maint Please hate @jsilhan and @j-mracek for this

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/109#issuecomment-266035398___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[ニール・ゴンパ]

For those looking from GitHub and the URL is being weirdly borked, here's the 
bug: https://bugs.mageia.org/show_bug.cgi?id=9832

-- 
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/pull/109#issuecomment-266032964___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[soig]

On 9 December 2016 at 15:41, Igor Gnatenko  wrote:

> @soig  Fully agree, saw same problem in Fedora
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> ,
> or mute the thread
> 
> .
>
​And here's a scary example where backup files made os-prober do stupid and
dangerous things:
https://bugs.mageia.org/show_bug.cgi?id=9832​


-- 
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/pull/109#issuecomment-266029995___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[ニール・ゴンパ]

@proyvind @soig @ignatenkobrain Maybe it'd be better if it was not on by 
default?

-- 
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/pull/109#issuecomment-266029530___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[Igor Gnatenko]

@soig Fully agree, saw same problem in Fedora

-- 
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/pull/109#issuecomment-266029269___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[soig]

On 9 December 2016 at 04:03, proyvind  wrote:

> …ault
>
> The legacy mandriva %apply_patches macro by default generated backup
> files with unique suffixes by default, the suffix for patch0 being
> .0001~ etc.
>
> As for those of us still using gendiff, this is still convenient, while
> for those using a scm, the '~' at end of suffix will anyways be
> automatically ignored, so enabling it by default won't interfer.
>
> A -B argument passed to the %autopatch will disable this.
> --
> You can view, comment on, or merge this pull request online at:
>
>   https://github.com/rpm-software-management/rpm/pull/109
> Commit Summary
>
>- When using %autopatch, create backup files with .~ suffix by
>default
>
> File Changes
>
>- *M* macros.in
>
>(10)
>
> Patch Links:
>
>- https://github.com/rpm-software-management/rpm/pull/109.patch
>- https://github.com/rpm-software-management/rpm/pull/109.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> , or mute the
> thread
> 
> .
>

​
Actually, in Mageia where we used extensively %apply_patches as the
mandriva legacy, we found out taht the backup behaviour not pleasant as
this makes several packages to silently package backup files
If one really wants backups, (s)he just has to use "%autosetup -S git" and
the like

​See eg: https://bugs.mageia.org/show_bug.cgi?id=15579

I remember having seen other strange and rare build issues due to backup
files, so please no, we don't want backup by default and since we've great
support for SCMs, there's no point in having such an option.
My 2 cents


-- 
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/pull/109#issuecomment-266028997___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[DNF Bot]

Can one of the admins verify this patch?

-- 
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/pull/109#issuecomment-266029013___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] When using %autopatch, create backup files with .~ suffix by def… (#109)

[ニール・ゴンパ]

Conan-Kudo approved this pull request.

Looks good to me.



-- 
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/pull/109#pullrequestreview-12224386___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint