[cgl_discussion] KGDB and process issues

george anzinger george at mvista.com
Thu Sep 5 18:13:20 PDT 2002


"Shureih, Tariq" wrote:
> 
> OK, sorry for not responding earlier, I've been in meetings most of the day.
> 
> So, for those who have not read the developer's guide and who are new to
> this, the process is as follows:
> 
> Patch is developed against pristine kernel (2.4.18 is the case now).
> Copy of this patch/diff is placed in a directory corresponding to the
> feature name under the "patches" directory in cvs. (patches/ngpt for
> example).
> Now on to Integrating the feature/patch into the progressive tree:
> The developer attempts to patch the tip of the kernel tree with his/her
> patch using patch --dry-run -p? < patches/my_patch.diff.
> 
> If there are no conflicts:
> Then the patch is applied (remove the --dry-run flag) directly on the
> source.
> The kernel is compiled, installed and tested for a minimum of booting the
> system and getting a prompt.
> Then the tree will have to be tagged with
> before__my_patch_version_1__CGLE_001", for example, which stands for
> before__FEATURE_NAME_VERSION__PROJECT_CGLE_FIRST_APPLICATION.
> Then the command "cvs -n ci" is run to see if there are any new files that
> need to be added to the tree.
> If there are new files created by the patch, the need to be added to cvs ,
> personally I always use the -kb flag with cvs add.
> Then another tag is applied and it's the
> "FEATURE_NAME_VERSION__CGLE_REV_NO".

You are saying there should be a tag between the cvs add and
the cvs ci?

> Then you apply the "AFTER" tag as well.
> 
> If there are conflicts, the patch can not be applied and checked in until
> all conflicts have been resolved.  That is managed by the
> developer/maintainer and in coordination with the other maintainers if
> needed.  Bottom line is you can't check the patch is if there are conflicts
> or if it doesn't build and boot the kernel.
> 
> Now, the question is, should we preserve the specific tweaks done in order
> to fix the patch conflict?
> My opinion is no.
> First of all, these are, most of the time, CGLE tree and integration
> specific fixes such as contextual modification, line movement, etc. which
> don't impact the feature.
> Second, even if we had to modify another patch's code in order to make the
> new patch work, I think it's still an implementation detail and not a
> specifications or design change and thus need not be reflected in the
> "pristine" version of the patch.

Uh, I was not suggesting ANY modification of the
"pristine".  The main issue Louis and I are trying to
address is the availability of a concise set of
modifications that were made at this point to someone else's
code, with or without their knowledge.  It is "simple" line
movements, for example, that caused code in smp.c to fail
(variable modified prior to its initialization).  

I am not, at least at this point, even suggesting that
anyone look at the conflict resolution.  If, on the other
hand, you or Rolla send me a bug report that says my feature
no longer builds or boots, I might want to scan the patch
directory looking for changes to code I added.  Like wise,
if the patch shows up in my mail box, I can quickly search
it for files that I changed and check if the resolution
looks good.  In an ideal world, I would have been consulted
prior to the change, but having been there, it is sometimes
hard to divine who or what patch changed the code I am
working on.  Or, one takes a quick look and says, "Oh, that
is simple, I'll just do this", but it really isn't as simple
as he thought.  So when it shows up in my inbox, either as a
patch or a note from Bugzilla :)  I have a place to look for
these sorts of changes.

I look at this as a code review tool, not as something that
will get rolled into a patch for the world.  From this point
of view, it might be useful to put these patches in a
different directory so they do not get confused with the
pristine patches.
> 
> The other question is, what about bug fixes and updates to an existing
> feature which are not a result of patch conflicts?
> 
> Those can only be applied or added to the CGLE tree in the for of a delta
> patch.  In other words, only the new modifications or bug fixes can be in
> the new patch and a copy of the pristine version of the patch (again the
> delta patch) should be added a separate file in the "patches" directory.

You seem to be in conflict with the paper you referenced
yesterday

http://www.developer.osdl.org/projects/docs/CGLE-patching-rules.pdf

which goes to some pain to explain that these patches are to
be complete, i.e. not to be applied serially.  From my point
of view (like I have to do these things) I prefer the delta
patch as this is the way the code is actually developed.  A
complete patch can always be made with "cat" if need be.

> 
> Finally, and I know this is a lengthy email.  Louis's original email has a
> lot or merit, however, this is not something Integration can do about since
> it's a natural occurrence even in normal patch conflict resolution a single
> developer may have to do.

Not sure what you are saying here.  The capital "I"
integration sounds like you are thinking that this group
would get involved in these conflict issues.  I would hope
to catch them before Integration has a chance to trip over
them.
> 
> Makes sense?  I am very open to modification and enhancements.  Remember,
> this is not carved in stone, it's just the best way we've figured so far to
> make a project this large with so many players and hands in the code to
> work.

I think we have a workable system as is.  Louis, I assume,
got tripped up by either modifying someone else's patch or
having his modified and spent some time resolving it.  I
think he has a good point.  I am trying to formalize it to
the point where it works for all.
> 
> Sorry, this email is in HTML :-P

But it wasn't,  Thanks.
> 
> --
> Tariq ¤
> 
> -----Original Message-----
> From: george anzinger [mailto:george at mvista.com]
> Sent: Thursday, September 05, 2002 4:00 PM
> To: Khalid Aziz
> Cc: andyp at osdl.org; Tim Anderson; Zhuang, Louis; cgl_discussion at osdl.org;
> developer at carrierlinux.org
> Subject: Re: [cgl_discussion] KGDB and process issues
> 
> Khalid Aziz wrote:
> >
> > george anzinger wrote:
> > >
> > > This, WAS the way.  As I understand the new rules we are not
> > > tagging the tree at all.
> >
> > Not so. Check the section "Checking in a patch and including it in
> > build" on page 16 of CGLE Developer Guide which requires tagging the
> > tree before and after commiting a patch.
> >
> Hm, sort of conflicts with what Tariq said yesterday, but
> then he did say to read the rules..
> 
> Still, what of my proposal?
> --
> 
> >From Tariq's email of yesterday:
> 
> For patches:
> 
> We recommend you use ".diff" files of your code ONLY and
> apply it using the patch command to the CGLE tree.
> 
> A copy of this patch/diff file needs to be, as always,
> placed in the "patches" directory under the corresponding
> feature.
> 
> You don't need to tag kernel patches or kernel source after
> patching with anything.  Please follow the guidelines
> provided at:
> 
> http://www.developer.osdl.org/projects/docs/CGLE-patching-rules.pdf
> 
> --
> George Anzinger   george at mvista.com
> High-res-timers:
> http://sourceforge.net/projects/high-res-timers/
> Preemption patch:
> http://www.kernel.org/pub/linux/kernel/people/rml
> _______________________________________________
> cgl_discussion mailing list
> cgl_discussion at lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/cgl_discussion

-- 
George Anzinger   george at mvista.com
High-res-timers: 
http://sourceforge.net/projects/high-res-timers/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml



More information about the cgl_discussion mailing list