Status of new source formats project

Pierre Habouzit <madcoder.TakeThisOut@madism.org> wrote:

> On Fri, Aug 07, 2009 at 10:45:14AM +0900, Charles Plessy wrote:
>> Le Thu, Aug 06, 2009 at 09:26:02AM -0700, Russ Allbery a écrit :
>> >
>> > (filterdiff comes with patchutils.) Given that, this seems like a tempest
>> > in a teapot to me. Just convert the diff into whatever format the tool
>> > that you're using expects or the reviewer wants to read.
>>
>> Hi Russ and everybody,
>>
>> I already explained that I prefered that the patch stays in the original format
>
> Then you'll need to write your "own" patch system that calls patch(1) to
> apply the patches, à la cdbs-simple-patchsys.

Why should he need to do that? If you'd had written "submit patches to
dpkg", I could get a meaning out of it, but here? He doesn't want to
diverge from upstream.

Regards, Frank

--
Dr. Frank Küster
Debian Developer (TeXLive)
VCD Aschaffenburg-Miltenberg, ADFC Miltenberg
B90/Grüne KV Miltenberg


--
To UNSUBSCRIBE, email to debian-devel-REQUEST.TakeThisOut@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster.TakeThisOut@lists.debian.org

Neil McGovern <neilm.TakeThisOut@debian.org> wrote:

> On Fri, Aug 07, 2009 at 10:45:14AM +0900, Charles Plessy wrote:
>> Giving a standard interface to reviewers is a laudable goal, but I do not see
>> reviewers except in elaborate scenarios about security. Therefore I will not
>> trade a real benefit for a hypothetical one, even if both are neglectible.
>> Also, I think that it is very important in a project of 1,000 persons to stick
>> to facts, and avoid building illusions together. So as long as there is no
>> reviewing process nor package reviews, there is no need to adapt to imaginary
>> reviewers.
>>
>
> /me raises his release team hat.

During a freeze, one typically wouldn't apply the upstream patches
anyway, but just cherry-pick individual changes. In this case, it
wouldn't be a problem to create unified diffs. Or even reformat the
context diff to unified, in case the upstream patch contains nothing but
RC (and translation) fixes.

Regards, Frank

--
Dr. Frank Küster
Debian Developer (TeXLive)
VCD Aschaffenburg-Miltenberg, ADFC Miltenberg
B90/Grüne KV Miltenberg


--
To UNSUBSCRIBE, email to debian-devel-REQUEST.TakeThisOut@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster.TakeThisOut@lists.debian.org

On Thu, Aug 06 2009, Charles Plessy wrote:


> This said, if there were a project-wide momentum for standardising on one patch
> format, I would not oppose. This would probably be a release goal, a
> preparation for a Policy change, or a demand from the security team to the
> package manintainers. Something with a motivation, a plan, some facts and some
> volunteers to get things done.

Why would we want to add that to policy? It seems like that both
the context diff and the unified diffs offer the same information (ie,
the change, and the context), and that each format can be mechanically
transformed from one to the other, and that which form one uses depends
on the individual.

I find that chunks with small changed grouped close together
read better in unified format, but if the chunk is long, with lots of
smallish changes interspersed with common context, then following the
logic of the old code versus the new code is better using context diffs
(you do not have to mentally keep track of two different flows).

This is mostly subjective, I know.

So policy should never promote one personal preference over
another, given that the formats convey the same information, and can be
transformed by tools, and are equally easily applied.

I mean, really, people, we are not the Borg. Why should dpkg
care, as long as the patch applies?

manoj
not inclined to pander to the hobgoblin of small minds
--
Save yourself from the 'Gates' of hell, use Linux." -- like that
one. The_Kind @ LinuxNet
Manoj Srivastava <srivasta.DeleteThis@debian.org> <http://www.debian.org/~srivasta/>
1024D/BF24424C print 4966 F272 D093 B493 410B 924B 21BA DABB BF24 424C


--
To UNSUBSCRIBE, email to debian-devel-REQUEST.DeleteThis@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster.DeleteThis@lists.debian.org

retitle 485330 Allow context diff in debian/patches/ in 3.0 (quilt) format
thanks

On Thu, 06 Aug 2009, Pierre Habouzit wrote:
> That said, yes, using non-unified diff is as laughable as using RCS or
> SCCS nowadays. Though I consider it a bug if dpkg refuses to apply a
> patch that patch(1) (that it uses in the end) would apply fine. I shall
> say that I absolutely don't get why there even is an "analyze()" routine
> in Patch.pm,

analyze() extracts information from the patch in order to:
- create directories that do not exist yet (patch will not do that for
you AFAIK or at least that's the assumption that the current codebase
made, it might have changed since this part of the code has been written)
- have a listing of all modified files in order to harmonize their
timestamp afterwards (required to avoid unwanted rebuilding when
patching auto-generated files and their source file)

Extracting those information require to parse the patch files, so why not
error out when the parser finds something not allowed?

So while I have no opposition to allowing patches in context format (and
not other non-contextual ones), the fix is most likely not the short one
proposed by Charles. Instead the Dpkg::Source::Patch object should have
full support of the context format at least in the analyze() part.

> if you want that, let patch(1) do it using its --dry-run
> mode. It'll report failed hunk and bad syntax the same way.

You'll be surprised how much permissive patch actually is. IIRC missing context
in unidiff is not always fatal for example (even when the numbers on the
@@ lines stipulate that there should be more lines than what there is).

Cheers,
--
Raphaël Hertzog


--
To UNSUBSCRIBE, email to debian-devel-REQUEST DeleteThis @lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster DeleteThis @lists.debian.org

On Thu, 13 Aug 2009, Pierre Habouzit wrote:
> > - create directories that do not exist yet (patch will not do that for
> > you AFAIK or at least that's the assumption that the current codebase
> > made, it might have changed since this part of the code has been written)
>
> According to the man page, and to tests I did, it does

So there's room for code removal, cool. Any hint since when this is the case?

> > - have a listing of all modified files in order to harmonize their
> > timestamp afterwards (required to avoid unwanted rebuilding when
> > patching auto-generated files and their source file)
>
> Can't you get that using lsdiff from patchutils ?

Probably but then I indirectly add patchutils to build-essential
because dpkg-dev needs to depend on it. I have tried to avoid many
new dependencies when refactoring the code. Otherwise I could have
probably used many CPAN modules that deal with this and much more.

Cheers,
--
Raphaël Hertzog


--
To UNSUBSCRIBE, email to debian-devel-REQUEST RemoveThis @lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster RemoveThis @lists.debian.org