Re: 3.0.STABLE2 patch candidates

[Amos Jeffries]

>
> tis 2008-02-26 klockan 01:08 +0100 skrev Henrik Nordström:
>> tis 2008-02-26 klockan 13:01 +1300 skrev Amos Jeffries:
>>
>> > Henrik:
>> >is there something special to do with individual patches not to be
>> > merged from a group of patches which as a whole are?
>> >   The include directive group which I have just done had 3 in the
>> middle
>> > which you may recall fixing-removing strtok_r usage.
>> >   For now I have made them a seperate group not-merged and gone
>> straight
>> > past to your strwordtok fix.
>>
>> I usually merge groups of related HEAD patches in one single commit on
>> the STABLE branch. This question gets a lot easier then as it's the
>> whole patch group that was merged, not the individual incremental
>> steps.. and this also simplifies release maintenance of the STABLE
>> branch as there is less patches to read up on when updating changelogs,
>> release notes etc.
>
> Forgot.. splitting the groups to move out changes which has been backed
> out is not a good idea as you then easily loose context on why these was
> added and then backed out. Causes more work later in the initial
> maintenance of the next cycle.

I considered it, but wasn't too keen on loosing the author attributions
where a feature is added then fixed by others. if you have found that
okay, then I'm all for less work.

>
> Also in some cases it's not so easy to separate "bad & redone" things
> from the good in a specific subproject. Therefore merging them all
> together is highly preferable unless there is good reasons to have them
> split on the STABLE branch as well.

Ok, I've marked the removed ones as merged into the real fix patch.

Amos

Re: 3.0.STABLE2 patch candidates

[Henrik Nordström]

tis 2008-02-26 klockan 01:08 +0100 skrev Henrik Nordström:
> tis 2008-02-26 klockan 13:01 +1300 skrev Amos Jeffries:
> 
> > Henrik:
> >is there something special to do with individual patches not to be
> > merged from a group of patches which as a whole are?
> >   The include directive group which I have just done had 3 in the middle
> > which you may recall fixing-removing strtok_r usage.
> >   For now I have made them a seperate group not-merged and gone straight
> > past to your strwordtok fix.
> 
> I usually merge groups of related HEAD patches in one single commit on
> the STABLE branch. This question gets a lot easier then as it's the
> whole patch group that was merged, not the individual incremental
> steps.. and this also simplifies release maintenance of the STABLE
> branch as there is less patches to read up on when updating changelogs,
> release notes etc.

Forgot.. splitting the groups to move out changes which has been backed
out is not a good idea as you then easily loose context on why these was
added and then backed out. Causes more work later in the initial
maintenance of the next cycle.

Also in some cases it's not so easy to separate "bad & redone" things
from the good in a specific subproject. Therefore merging them all
together is highly preferable unless there is good reasons to have them
split on the STABLE branch as well.

Regards
Henrik



signature.asc
Description: Detta är en digitalt signerad	meddelandedel

Re: 3.0.STABLE2 patch candidates

[Henrik Nordström]

tis 2008-02-26 klockan 13:01 +1300 skrev Amos Jeffries:

> Henrik:
>is there something special to do with individual patches not to be
> merged from a group of patches which as a whole are?
>   The include directive group which I have just done had 3 in the middle
> which you may recall fixing-removing strtok_r usage.
>   For now I have made them a seperate group not-merged and gone straight
> past to your strwordtok fix.

I usually merge groups of related HEAD patches in one single commit on
the STABLE branch. This question gets a lot easier then as it's the
whole patch group that was merged, not the individual incremental
steps.. and this also simplifies release maintenance of the STABLE
branch as there is less patches to read up on when updating changelogs,
release notes etc.

Regards
Henrik


signature.asc
Description: Detta är en digitalt signerad	meddelandedel

Re: 3.0.STABLE2 patch candidates

[Amos Jeffries]

> On Sat, 2008-02-23 at 14:35 +0100, Henrik Nordström wrote:
>
>> Please go to
>> http://www.squid-cache.org/Versions/v3/HEAD/changesets/merge.html and
>> inspect the list of patches to merge and not merge and give your opinion
>> if you think some patch is should be given another priority or if there
>> is an error in the grouping of related patches.
>
> The following patches are listed as "to be merged" (I think), but should
> not be blindly merged until they are reviewed, polished, and committed.
>
> 11462 Bug #2230 possible fix: segmentation fault with a "pure
> virtual method called"
>
> 11461 Bug #2224 fix: reentrant debugging crashes Squid
>
> 11387 Bug #2038: check reply_body_max_size before ICAP
> 11388 Bug #2038: check reply_body_max_size before ICAP
> 11389 Bug #2038: check reply_body_max_size before ICAP
>
>
> The following patch is listed as "not to be merged", but probably should
> be merged as it is a bug fix:
>
> 11360 Assert that checklist and request are set instead of
> segfaulting as in bug 2168
>
>
> Thank you,
>
> Alex.
>
>
>

Henrik:
   is there something special to do with individual patches not to be
merged from a group of patches which as a whole are?
  The include directive group which I have just done had 3 in the middle
which you may recall fixing-removing strtok_r usage.
  For now I have made them a seperate group not-merged and gone straight
past to your strwordtok fix.

Amos

Re: 3.0.STABLE2 patch candidates

[Amos Jeffries]

Alex Rousskov wrote:

On Sun, 2008-02-24 at 15:56 +0100, Henrik Nordström wrote:

mån 2008-02-25 klockan 01:31 +1300 skrev Amos Jeffries:


Just done a short span of updates.

One thats standing out now is the myportname ACL mini-feature.
I'm doubtful, but agnostic. Are there any actual needs for this in 3.0?

It's trivial and a feature found in 2.7. But it's also a configuration
change so moving it to 3.1 is fine.


Since we are including a much bigger #include-support change, I think we
should include this small change as well, but since I am not the one who
is going to patch 3.0, I cannot have a strong opinion here :-)



I've looked it over in more detail. Most of it is new code addition. 
With some minor tweaking to back-port. Existing 3.0 configs don't seem 
to be affected.


Looks like I'll do it then.

Taking any vetos now ...

Amos
--
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.

Re: eCAP: expose Squid or link with eCAP lib?

[Robert Collins]

On Sat, 2008-02-23 at 15:03 +0100, Henrik Nordström wrote:
> fre 2008-02-15 klockan 09:07 +1100 skrev Robert Collins:
> > Its more work both at code and at runtime. The only thing it really
> > allows that 1) doesn't is non-GPL eCAP modules.
> 
> I don't see how 2) can allow non-GPL eCAP modules. We can't add a
> linking excemption to the license even if there is a well defined API.
> 
> The only way to "link" non-GPL code to Squid is by not linking it into
> the runtime binary, statically or dynamically. non-GPL code needs to run
> in it's own process image, and for that we have ICAP already.

Split out runtime linking and compiling.

Runtime linking is done on the users machine, no distribution is taking
place and thus the GPL is not relevant.

Compiling uses header information (and on some platforms information
about the library (e.g. dll symbol name mappings) to cause the generated
binary to be one that will link at runtime correctly. This can (but
doesn't always) cause a copy of information to be placed into the binary
that was compiled. Distributing that binary requires a copying licence
for the copied information - which is where the GPL kicks in.

Now, when you have a well defined interface (e.g. readline), its
entirely possible to have *two* implementations that are binary
compatible.

Implementation one: GPL. Feature complete.
Implementation two: BSD. Feature complete from an interface perspective
(you can compile anything you can compile with the GPL implementation),
but its runtime support is shockingly bad.

And the loophole should now be obvious:
 - vendor A writes against the GPL version in private, to test.
 - they then create a binary module for platform X by compiling against
the BSD library.
 - the binary module runs against the GPL version.

-Rob
-- 
GPG key available at: .
> 


signature.asc
Description: This is a digitally signed message part

Re: 3.0.STABLE2 patch candidates

[Henrik Nordström]

mån 2008-02-25 klockan 10:59 -0700 skrev Alex Rousskov:

> The following patches are listed as "to be merged" (I think), but should
> not be blindly merged until they are reviewed, polished, and committed.
> 
> 11462 Bug #2230 possible fix: segmentation fault with a "pure
> virtual method called"
> 
> 11461 Bug #2224 fix: reentrant debugging crashes Squid
> 
> 11387 Bug #2038: check reply_body_max_size before ICAP
> 11388 Bug #2038: check reply_body_max_size before ICAP
> 11389 Bug #2038: check reply_body_max_size before ICAP
> 
> 
> The following patch is listed as "not to be merged", but probably should
> be merged as it is a bug fix:
> 
> 11360 Assert that checklist and request are set instead of
> segfaulting as in bug 2168

All 4 patch sets adjusted per your comments.

Regards
Henrik


signature.asc
Description: Detta är en digitalt signerad	meddelandedel

Re: async-calls squid3/src debug.cc,1.18.4.4,1.18.4.5 Debug.h,1.10.4.3,1.10.4.4 main.cc,1.89.4.7,1.89.4.8 structs.h,1.116.4.3,1.116.4.4 cf.data.pre,1.155.4.3,1.155.4.4

[Alex Rousskov]

On Sat, 2008-02-23 at 03:34 -0700, Adrian Chadd wrote:
> Can someone please explain the logic behind this?
> I think I understand, but I'd like to know what people are trying
> to achieve.

The overall goal is robustness: make production Squid installations
crash less often. In this particular case, we are trying to kill the
transaction or job that hit an internal error (a.k.a., assertion) rather
than killing the whole Squid. If our attempts to isolate the failure
fail, Squid aborts as before.

The assert_burst_max configuration knob controls whether Squid tries to
be robust and how hard it tries to do that. The logic is experimental
and is likely to change once we get more experience with this feature.

HTH,

Alex.

> On Sat, Feb 23, 2008, chtsanti wrote:
> > Update of cvs.devel.squid-cache.org:/cvsroot/squid/squid3/src
> > 
> > Modified Files:
> >   Tag: async-calls
> > debug.cc Debug.h main.cc structs.h cf.data.pre 
> > Log Message:
> > - Add TheAssertsPerStep counter to count the number of assertions per
> > single main loop iteration. This counter is reset to zero at the
> > beginning of every main loop iteration.
> > 
> > - To reset the TheAssertsPerStep counter a new AsyncEngine based class used,
> > the XAssertsEngine.
> > 
> > - Add "assert_burst_max " to squid.conf. If TheAssertsPerStep
> > counter exceeds a non-negative assert_burst_max, we abort().
> > If set to zero, the first assertion aborts Squid, giving users the old
> > behavior. If set to a negative number, there is no limit.
> > 
> > 
> > 
> > Index: cf.data.pre
> > ===
> > RCS file: /cvsroot/squid/squid3/src/cf.data.pre,v
> > retrieving revision 1.155.4.3
> > retrieving revision 1.155.4.4
> > diff -C2 -d -r1.155.4.3 -r1.155.4.4
> > *** cf.data.pre 12 Feb 2008 19:04:39 -  1.155.4.3
> > --- cf.data.pre 23 Feb 2008 10:31:18 -  1.155.4.4
> > ***
> > *** 5610,5612 
> > --- 5610,5628 
> >   DOC_END
> >   
> > + NAME: assert_burst_max
> > + TYPE: int
> > + LOC: Config.assert_burst_max
> > + DEFAULT: 100
> > + DOC_START
> > +   When this is set to a possitive number then Squid will abort 
> > +   if an assertions burst exceeds this number. 
> > +   If set to zero, the first assertion aborts Squid, giving users
> > +   the old behavior. If set to a negative number, there is no 
> > +   limit.
> > +   An asssertions burst defined as the number of assertions per 
> > +   single Squid main loop iteration.
> > +   WARNING! This is an experimental feature and the definition 
> > +   of a "burst" can change
> > + DOC_END
> > + 
> >   EOF
> > 
> > Index: main.cc
> > ===
> > RCS file: /cvsroot/squid/squid3/src/main.cc,v
> > retrieving revision 1.89.4.7
> > retrieving revision 1.89.4.8
> > diff -C2 -d -r1.89.4.7 -r1.89.4.8
> > *** main.cc 12 Feb 2008 19:05:00 -  1.89.4.7
> > --- main.cc 23 Feb 2008 10:31:18 -  1.89.4.8
> > ***
> > *** 144,147 
> > --- 144,158 
> >   };
> >   
> > + class XAssertsEngine : public AsyncEngine
> > + {
> > + 
> > + public:
> > + int checkEvents(int timeout)
> > + {
> > +   TheAssertsPerStep = 0;
> > + return EVENT_IDLE;
> > + };
> > + };
> > + 
> >   class SignalEngine: public AsyncEngine
> >   {
> > ***
> > *** 1297,1300 
> > --- 1308,1314 
> >   mainLoop.registerEngine(&signalEngine);
> >   
> > + XAssertsEngine xassertsEngine;
> > + mainLoop.registerEngine(&xassertsEngine);
> > + 
> >   /* TODO: stop requiring the singleton here */
> >   mainLoop.registerEngine(EventScheduler::GetInstance());
> > 
> > Index: debug.cc
> > ===
> > RCS file: /cvsroot/squid/squid3/src/debug.cc,v
> > retrieving revision 1.18.4.4
> > retrieving revision 1.18.4.5
> > diff -C2 -d -r1.18.4.4 -r1.18.4.5
> > *** debug.cc16 Feb 2008 21:06:42 -  1.18.4.4
> > --- debug.cc23 Feb 2008 10:31:18 -  1.18.4.5
> > ***
> > *** 45,48 
> > --- 45,49 
> >   int TheCascadingAsserts = 0;
> >   int TheSalvagedAsserts = 0;
> > + int TheAssertsPerStep = 0;
> >   
> >   static char *debug_log_file = NULL;
> > ***
> > *** 591,606 
> >   xassert(const char *msg, const char *file, int line) {
> >   
> > ! if (TheCascadingAsserts < MAX_CASCADING_ASSERTS && 
> > AsyncCall_Handling_Exceptions) {
> > TheCascadingAsserts++;
> > TheSalvagedAsserts++;
> > !   debugs(0, 0, "assertion failed: " << file << ":" << line << ": \"" << 
> > msg << "\". Trying to survive. Salvaged assertions: " << 
> > TheSalvagedAsserts);
> >   
> > throw TextException(msg, file, line);
> >   }
> >   
> > - debugs(0, 0, "assertion failed: " << file << ":" << line << ": \"" << 
> > msg << "\"");
> >   
> >   if(TheCascadingAsserts >= MAX_CASCADING_ASSERTS)
> > !   debugs(0

Re: 3.0.STABLE2 patch candidates

[Alex Rousskov]

On Sat, 2008-02-23 at 14:35 +0100, Henrik Nordström wrote:

> Please go to
> http://www.squid-cache.org/Versions/v3/HEAD/changesets/merge.html and
> inspect the list of patches to merge and not merge and give your opinion
> if you think some patch is should be given another priority or if there
> is an error in the grouping of related patches.

The following patches are listed as "to be merged" (I think), but should
not be blindly merged until they are reviewed, polished, and committed.

11462   Bug #2230 possible fix: segmentation fault with a "pure
virtual method called"

11461   Bug #2224 fix: reentrant debugging crashes Squid

11387   Bug #2038: check reply_body_max_size before ICAP
11388   Bug #2038: check reply_body_max_size before ICAP
11389   Bug #2038: check reply_body_max_size before ICAP


The following patch is listed as "not to be merged", but probably should
be merged as it is a bug fix:

11360   Assert that checklist and request are set instead of
segfaulting as in bug 2168


Thank you,

Alex.

Re: 3.0.STABLE2 patch candidates

[Alex Rousskov]

On Sun, 2008-02-24 at 15:56 +0100, Henrik Nordström wrote:
> mån 2008-02-25 klockan 01:31 +1300 skrev Amos Jeffries:
> 
> > Just done a short span of updates.
> > 
> > One thats standing out now is the myportname ACL mini-feature.
> > I'm doubtful, but agnostic. Are there any actual needs for this in 3.0?
> 
> It's trivial and a feature found in 2.7. But it's also a configuration
> change so moving it to 3.1 is fine.

Since we are including a much bigger #include-support change, I think we
should include this small change as well, but since I am not the one who
is going to patch 3.0, I cannot have a strong opinion here :-)

Thank you,

Alex.

Re: What's in the NT branch

[Alex Rousskov]

On Sun, 2008-02-24 at 21:26 +0100, Henrik Nordström wrote:
> Guido,
> 
> what's actually in the NT branch today? Is it only the "makefiles", or
> is there any actual source changes which should not be merged to the
> main branch?
> 
> If it's only the "makefiles" then I propose those are stored in the main
> branch (HEAD and SQUID_3_0) directly
> 
> Looking at a diff...
> 
> port directory tree with "makefiles" [generally OK]
> 
> lib/getopt.c. Copy from NetBSD with a license incompatible with GPL.
> 
> port/win32/src/encrypt.c. 56 bit DES encryption. Still under export
> control in some regoins of the world, but not really a problem. Could be
> in lib/ to support other platforms without crypt().
> 
> port/win32/src/readdir.c. Unknown copyright or license.
> 
> I don't see a good reason why the port tree cannot be in the main
> branch, except for the trivial reservations above...

FWIW, I asked Guido a related question and made a similar "sore in the
main branch" suggestion a few months(?) ago. At that time, Guido was
positive he needs Windows-specific branches; he explained why he thought
so. Please try to find that thread in the squid-dev archive for more
info so that Guido does not have to repeat himself.

Thank you,

Alex.

Re: Co-Advisor run for 2.7?

[Alex Rousskov]

On Mon, 2008-02-25 at 15:53 +1100, Mark Nottingham wrote:
> Just curious -- has anyone run Co-Advisor against 2.7? I'd be  
> especially interested in the results with the HTTP/1.1 support turned  
> on...

I have not, but a Squid3 test is on the to-do list.

Alex.

Re: Patches and the STABLE branches

[Henrik Nordström]

sön 2008-02-24 klockan 22:23 -0700 skrev Alex Rousskov:
> On Sun, 2008-02-24 at 21:48 +0100, Henrik Nordström wrote:
> > Patches with a bugzilla reference: close the bug report but have it
> > targeted for the stable release.
> 
> Only if the person closing the bug report thinks the fix should be
> ported/applied to that stable release, right?

Right.

Regards
Henrik


signature.asc
Description: Detta är en digitalt signerad	meddelandedel

Re: squid3 future directory structure

[Alex Rousskov]

On Sat, 2008-02-23 at 14:47 +0100, Henrik Nordström wrote:
> fre 2008-02-22 klockan 11:29 -0700 skrev Alex Rousskov:
> > On Fri, 2008-02-22 at 19:23 +0100, Guido Serassio wrote:
> > 
> > > Changing the case of files/dir will not be a problem if we will avoid 
> > > upper/lower case collisions.
> > 
> > This only applies to files in the same directory, right? AFAICT,
> > filenames from different directories may still collide and even have
> > identical case.
> 
> Bad idea even then. The files is easily confused by humans. If someone
> says file.cc in a dicsussion do he mean src/File.cc or lib/File.cc?

> And a really really bad idea for include files unless in a specific
> include subdir (#include "subdir/file.h"). For example case-insensitive
> filesystems will fail if both include/ and src/ has a .h file of the
> same name differing only in case.
> 
> Also not sure what happens if we (or libtool) build a common lib archive
> of objects with the same name from differnt directories, but I think
> that will fail as well.

The only real problem that bothers me personally is that __FILE__ does
not include the directory name and, hence, is less useful in assert
statements and such.

Humans can use module/Foo almost as well as current MODULEFoo and
libraries appear to work fine in my experience. Include statements must
use module/ directory, of course.

Cheers,

Alex.

Re: Patches and the STABLE branches

[Alex Rousskov]

On Sun, 2008-02-24 at 21:48 +0100, Henrik Nordström wrote:
> Patches with a bugzilla reference: close the bug report but have it
> targeted for the stable release.

Only if the person closing the bug report thinks the fix should be
ported/applied to that stable release, right?

Thanks,

Alex.

Co-Advisor run for 2.7?

[Mark Nottingham]

Just curious -- has anyone run Co-Advisor against 2.7? I'd be  
especially interested in the results with the HTTP/1.1 support turned  
on...


Cheers,


--
Mark Nottingham   [EMAIL PROTECTED]