[Freeipa-devel] [PATCHES] Fix getkeytab operation

[Simo Sorce]

On Thu, 20 Nov 2014 10:47:35 -0500
Simo Sorce <simo at redhat.com> wrote:

> On Thu, 20 Nov 2014 16:47:29 +0200
> Alexander Bokovoy <abokovoy at redhat.com> wrote:
> 
> > On Thu, 20 Nov 2014, Nathaniel McCallum wrote:
> > >On Thu, 2014-11-20 at 09:12 -0500, Simo Sorce wrote:
> > >> On Thu, 20 Nov 2014 12:36:45 +0200
> > >> Alexander Bokovoy <abokovoy at redhat.com> wrote:
> > >>
> > >> > On Wed, 19 Nov 2014, Simo Sorce wrote:
> > >> > >----- Original Message -----
> > >> > >> From: "Alexander Bokovoy" <abokovoy at redhat.com>
> > >> > >[...]
> > >> > >
> > >> > >> Regarding the patchset itself:
> > >> > >>
> > >> > >> Patch 0001: fix 'wuld' in the commit message. The rest is
> > >> > >> fine.
> > >> > >
> > >> > >Fixed.
> > >> > >
> > >> > >> Patch 0002:
> > >> > >>  - ticket number is missing in the commit message
> > >> > >
> > >> > >Added.
> > >> > >
> > >> > >>  - perhaps, an instruction how to regenerate asn1 code can
> > >> > >> be made a Makefile target? We don't need to call it
> > >> > >> ourselves but this would simplify things in future
> > >> > >
> > >> > >Added make regenerate target to asn1c makefile
> > >> > >
> > >> > >>  - I'm little uncomfortable how ASN_DEBUG() output goes
> > >> > >> explicitly to stderr but I guess this is something we
> > >> > >> currently cannot override with DS-specific log printing, so
> > >> > >> no big deal right now
> > >> > >
> > >> > >ASN_DEBUG() is currently disabled as EMIT_ASN_DEBUG is
> > >> > >undefined, we can later provide a replacement ASN_DEBUG
> > >> > >function to hook debugging, but given the same code is used in
> > >> > >both DS plugins and ipa-getkeytab binary I did not want to
> > >> > >assume anything, and how to wire it up (if we even want to)
> > >> > >should probably be discussed at a later time.
> > >> > >
> > >> > >>  - any specific need to get asn1/compile committed? We don't
> > >> > >> commit it in the client code (ipa-client/compile).
> > >> > >
> > >> > >Added 'compile' to .gitignore in second patch
> > >> > >
> > >> > >> Patch 0003: OK
> > >> > >
> > >> > >Nothing changed here.
> > >> > >
> > >> > >I also remembered the patch naming policy :-) so new patch
> > >> > >names/numbers are 514,515,516, third revision.
> > >> > Thanks. The only complaint I have left is number of whitespace
> > >> > errors that git says are in the 515th patch.
> > >>
> > >> Yeah the autogenerated code is not a pretty sight style-wise, do
> > >> we want to run an automatic indenter on it ?
> > >> I was hesitant to do so, but I wouldn't mind adding that, if we
> > >> feel strongly about it.
> > >
> > >Let's please not try to correct autogenerated code.
> > I'm not tied to this but Simo now thinks it is better to run
> > indenter in the generator rule as this will give less problems in
> > actual comparison noise that git diff would give.
> > 
> > I'll make sure to talk back to asn1c author to see if we can improve
> > its generators upstream.
> 
> So given Nathaniel doesn't like to touch autogenerated code I'll leave
> it as it is.
> I am going to push with the only change being to remove
> asn1/config.h.in~ with was added to the second commit by mistake.

Pushed to

master:
b170851058d6712442d553ef3d11ecd21b282443
c6afc489a1c9d86fd593bd47c4a8dae6d9a008d2
b1a30bff04fe9763b8b270590ec37084fd19b4e0

ipa-4-1:
f065cec8a58bf4fee0334afdfb63db02f76c1ff7
45ceef14f9ffa5f3abf19088e991f427b7c5bd92
dd3e91639bc3e87b5a95e344b7d190136ad30de0

ipa-4-0:
55578e9cb33924085969102186250ee60c0a9d85
598b54716c6e177a6b5bfdbccf483d28bf40e0b8
aa988311d1b5eefe16eb60c04227900814468e9f

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York