Skip to Content.
Sympa Menu

sm-commit - Re: [SM-Commit] GIT changes to master grimoire by Ismael Luceno (ebb3d4ea0344f74a552a75a2b3f4df78275aa44c)

sm-commit AT lists.ibiblio.org

Subject: Source Mage code commit list

List archive

Chronological Thread  
  • From: Ismael Luceno <ismael.luceno AT gmail.com>
  • To: Thomas Orgis <thomas-forum AT orgis.org>
  • Cc: sm-commit AT lists.ibiblio.org
  • Subject: Re: [SM-Commit] GIT changes to master grimoire by Ismael Luceno (ebb3d4ea0344f74a552a75a2b3f4df78275aa44c)
  • Date: Mon, 15 Aug 2016 15:30:31 -0300

On 15/Aug/2016 08:48, Thomas Orgis wrote:
> Am Sun, 14 Aug 2016 02:52:53 +0000
> schrieb Ismael Luceno <scm AT sourcemage.org>:
>
> > #---------------------------------------------------------------------
> > +## Apply patches from a directory
> > +#---------------------------------------------------------------------
> > +function apply_patch_dir() {
> > + [ -d "$SPELL_DIRECTORY/$1" ] || return 0
> > + find "$SPELL_DIRECTORY/$1" \( -name \*.patch -o -name \*.diff \)
> > -print0 |
> > + sort -zV | xargs -0 -n 1 -t patch -fp1 -i
> > +}
>
> Several questions come to mind:
>
> 1. Why does `apply_patch_dir nonexisting_dir` not return an error?

Practical matter, to avoid repeating the condition.

Since git doesn't track dirs, after removing the last upstreamed
patch things break without the test.

> 2. Why does it always use -fp1?
> - I never use -f … it still fails if the patch doesn't fit, right,
> especially if it doesn't find the file to patch?

Yes, it just avoids prompting the user.

> - Why not -N, too?

-f implies -N

Now I notice I forgot adding -l.

> - Is p1 always the correct parent length? That should be mentioned as a
> prerequisite for patch dirs.
<...>

It is just the most common case, but I would like the function to be
more general.

I do not like the idea of letting apply_patch_dir's user specify -p
directly, because it's global to the directory, and so we would still
need a directory per patch type, which isn't nice.

What I've been thinking about is:

We could try to parse a patch's header, if it contains one
(RFC822-like), and take patch parameters from there, as well as
dependencies.

Headers we would want:
* X-Patch-Strip (default = 1)
* X-Patch-Label (a shorthand, mandatory for optional patches)
* X-Patch-Deps (list of shorthands)
* X-Patch-Conflict (list of shorthands)

If a patch is optional, it would need a label, and this would be
complemented by code to automatically build questions about optional
patches, and pass that information to the code that applies the
patches to decide which not to apply.

If a patch is not depended upon by others, it doesn't need a label.

Patch application should happen in a deterministic order, with as
little variation as possible.

That's basically the plan.

> (And bonus: I wonder if we really should encourage file names with
> carriage returns in them.)

It's not encouragement. We don't want things falling apart just
because someone made a mistake (or the grimoire got corrupted).




Archive powered by MHonArc 2.6.24.

Top of Page