Re: [PATCH] remove squid-old.h
On Fri, Aug 17, 2012 at 2:53 AM, Amos Jeffries squ...@treenet.co.nz wrote: On 17/08/2012 8:49 a.m., Kinkie wrote: On Thu, Aug 16, 2012 at 6:00 PM, Alex Rousskov rouss...@measurement-factory.com wrote: On 08/16/2012 12:56 AM, Kinkie wrote: I am guessing this is the patch that broke build in numerous places. I am surprised it was not reverted upon the first signs of trouble. Here are some of the errors that I see: Prior to committing I have run a test-builds.sh on my kubuntu machine and it turned out fine; apparently the more recent kernel and/or glibc and/or gcc triggers some more permissive include paths. AFAICT, the problems are related to missing #includes (a header that was included via squid-old.h is no longer included). For example, src/adaptation/ecap/MessageRep.cc is now missing protos.h, which provides a urlParse() declaration. A test build that enables ssl_crtd or eCAP could not succeed so I am guessing your test-builds.sh does not enable them. I did not check whether any other features are affected. I agree, that is the expected source of problems right now. My test-builds.sh is actually the whole build farm, whose status is: CentOs-5, CentOS-5-icc, ubuntu-precise, debian-squeeze-arm, freebsd-6.4 (west), mandriva, debian-unsatable: OK. FreeBSD-9, OpenSUSE, MacOS-Leopard: fail due to problems in the Rock unit test FreeBSD-9-clang: fail due to strtoll being misdetected OpenBSD(all): fail due to library circular dependencies (Msg/CacheMgr) Freebsd-7.2 (east), debian-sid-clang: fail due to system issues Mswin: compat/ failing to completely fulfill its role. In other words, none in 15 tested platforms shows the issue that you're highlighting. If anything, this means that must the build tests must be furhter improved: what can't be seen can't be fixed, despite the best efforts one can put in place. This sounds like the CRTD was not added to test-builds maximus listing of switches to turn on. I guess its time for another audit of ./configure flags to see what the build levels should contain. I'm testing a farm-build containing the switch and a fix for the error Alex spotted in my staging branch. Here's the patch: - === modified file 'src/ssl/helper.cc' --- src/ssl/helper.cc 2012-08-06 17:21:57 + +++ src/ssl/helper.cc 2012-08-17 08:35:33 + @@ -4,6 +4,7 @@ #include squid.h #include anyp/PortCfg.h +#include protos.h #include ssl/Config.h #include ssl/helper.h #include SquidTime.h === modified file 'test-suite/buildtests/layer-02-maximus.opts' --- test-suite/buildtests/layer-02-maximus.opts 2010-12-10 09:59:02 + +++ test-suite/buildtests/layer-02-maximus.opts 2012-08-17 08:01:54 + @@ -108,6 +108,7 @@ --with-pic \ --with-pthreads \ --enable-build-info=squid\ test\ build \ + --enable-ssl-crtd \ -- May I merge it if the test-suite succeeds in staging? -- /kinkie
Re: [PATCH] remove squid-old.h
I am not annoyed at the failures -- we all commit broken code once in a while, and there is currently no good way to prevent even simple build failures. I am somewhat annoyed that some failures result in commit reversal (with unusable or contradictory information as to the reason for the reversal) while others are not (even after detailed reasons have been provided), but this is _not_ the reason I am asking who is responsible for fixing trunk -- I do really need to know whether this is something I should be working on (to avoid two different people working on the same bug). Now that I know what to look for, I'm on it. -- /kinkie
Re: Policy for reverting changes
tor 2012-08-16 klockan 23:11 -0600 skrev Alex Rousskov: I am not annoyed at the failures -- we all commit broken code once in a while, and there is currently no good way to prevent even simple build failures. Yes.. I am somewhat annoyed that some failures result in commit reversal (with unusable or contradictory information as to the reason for the reversal) while others are not (even after detailed reasons have been provided), Reversals are not routine. There is no automatic reversal, instead it is up to each and everyone to decide on when reversal is appropriate and when not. Ok, here is a proposal on a little policy on the topic on how to handle others bad commits: trunk: 1. If possible, fix it. 2. If not, nag the person who committed the problematic code. 3a. If no response in 24 hours and it's blocking other things (including test farm builds) then revert the commit with a notice to squid-dev. 3b. If still broken after 7 days of discussion and attempts to fix then revert the change. 4. Let it mature in a development branch, and submit for merge again when the issues have been resolved. numbered branches: 1. If possible, fix it. 2. If not, nag the person who committed the problematic code. 3. Let the release manager decide on reverting It's a real pity bzr do not have a unmerge operation. Currently reversals are quite intrusive and may merge to the branch where the feature was developed erasing it from there as well. but this is _not_ the reason I am asking who is responsible for fixing trunk -- I do really need to know whether this is something I should be working on (to avoid two different people working on the same bug). responsible.. is the one who takes on the bug in question. Regards Henrik
Re: Policy for reverting changes
On Fri, Aug 17, 2012 at 6:49 PM, Henrik Nordström hen...@henriknordstrom.net wrote: tor 2012-08-16 klockan 23:11 -0600 skrev Alex Rousskov: I am not annoyed at the failures -- we all commit broken code once in a while, and there is currently no good way to prevent even simple build failures. Yes.. I am somewhat annoyed that some failures result in commit reversal (with unusable or contradictory information as to the reason for the reversal) while others are not (even after detailed reasons have been provided), Reversals are not routine. There is no automatic reversal, instead it is up to each and everyone to decide on when reversal is appropriate and when not. Ok, here is a proposal on a little policy on the topic on how to handle others bad commits: trunk: 1. If possible, fix it. 2. If not, nag the person who committed the problematic code. 3a. If no response in 24 hours and it's blocking other things (including test farm builds) then revert the commit with a notice to squid-dev. 3b. If still broken after 7 days of discussion and attempts to fix then revert the change. 4. Let it mature in a development branch, and submit for merge again when the issues have been resolved. Sounds very reasonable to me. In this case, I took path 1, complicated by the fact that the breakage wasn't visible to me. (the patchlet I sent earlier on is confirmed as good to fix the last remaining issues, including making the breakage visible in layer-02-maximus.) It's a real pity bzr do not have a unmerge operation. Currently reversals are quite intrusive and may merge to the branch where the feature was developed erasing it from there as well. Have you considered bzr uncommit? Uncommitting the last revision(s) it's very easy: bzr uncommit -r revno-to-return-to, bzr revert But a process such as the follwoing should do the trick to uncommit a merge which has occurred at revision N: bzr uncommit -r N+1 bzr shelve --all bzr uncommit bzr revert bzr unshelve --apply bzr commit Note: this doesn't take into account commit messages, conflicts etc. Robert can certainly comment on the feasibility of the approach, or any better approaches. -- /kinkie
Re: bzr unmerge
fre 2012-08-17 klockan 19:59 +0200 skrev Kinkie: Have you considered bzr uncommit? uncommit do not cut it. uncommit simply moves the branch head to a given revision discarding any later revisions, same as git reset --hard for those familiar with git. It's ok to use on a private branch to clean up mistakes, but MUST NOT, repeat MUST NOT be used after the revisions have been pushed to a shared repository. what I want is a unmerge operation that is a commit in itself much like revert, preserving full history, but which requires attention to resolve when propagated to other branches, enabling other branches to keep the changes or revert them per user choice. But a process such as the follwoing should do the trick to uncommit a merge which has occurred at revision N: bzr uncommit -r N+1 bzr shelve --all bzr uncommit bzr revert bzr unshelve --apply bzr commit And you'll loose all history of the later commits, and screws up merging to any other branches. Please DO NOT EVER do a trick like this on the master Squid repository. Regards Henrik
Re: Squid 3.2.1 is dumping core
Alex?:
Did I catch some unfinished code?
Looks like I am failing after the TODO's with asserts to check
that structures are coming back empty'ed...and they aren't.
*hmmm*
(store.cc failed code section below..)
void
StoreEntry::startWriting()
{
Packer p;
/* TODO: when we store headers serparately remove the header portion */
/* TODO: mark the length of the headers ? */
/* We ONLY want the headers */
packerToStoreInit(p, this);
assert (isEmpty());###--assertion
failed...#1854; TODO?
assert(mem_obj);
const HttpReply *rep = getReply();
assert(rep);
rep-packHeadersInto(p);
mem_obj-markEndOfReplyHeaders();
rep-body.packInto(p);
packerClean(p);
}
Linda W wrote:
Amos Jeffries wrote:
The Squid HTTP Proxy team is very pleased to announce the availability
of the Squid-3.2.1 release!
---
BTW ---
I finally managed to get a good (I think) stack traceback.
(N.B. - earlier versions were dumping core, but never got the
debug symbols in correctly for a good stack trackback). This time
I did...
This look familiar for someone who's doing something wrong or should
squid be more tolerant? ;-)
Core was generated by `(squid-6)'.
Program terminated with signal 6, Aborted.
#0 0x003000434d95 in __GI_raise (sig=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
(gdb) where
#0 0x003000434d95 in __GI_raise (sig=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x0030004362ab in __GI_abort () at abort.c:93
#2 0x005042e5 in xassert (msg=0x698648 isEmpty(),
file=optimized out, line=1854) at debug.cc:567
#3 0x005938b4 in StoreEntry::startWriting (this=0x27942d0)
at store.cc:1854
#4 0x005a380c in ServerStateData::setFinalReply
(this=0x27ae6c8, rep=
0x27b9890) at Server.cc:173
#5 0x0054b0b0 in HttpStateData::processReply (this=0x27ae6c8)
at http.cc:1168
#6 0x0054b28a in HttpStateData::readReply (this=0x27ae6c8,
io=optimized out) at http.cc:1145
#7 0x0054d0eb in JobDialerHttpStateData::dial
(this=0x2814240, call=
...) at base/AsyncJobCalls.h:175
#8 0x005f9b74 in AsyncCall::make (this=0x2814210) at
AsyncCall.cc:36
#9 0x005fcbb7 in AsyncCallQueue::fireNext (this=optimized
out)
at AsyncCallQueue.cc:54
#10 0x005fcd10 in AsyncCallQueue::fire (this=0x2340480)
at AsyncCallQueue.cc:40
#11 0x0051168c in EventLoop::runOnce (this=0x7fff6fb3d720)
at EventLoop.cc:131
#12 0x00511758 in EventLoop::run (this=0x7fff6fb3d720)
at EventLoop.cc:95
#13 0x0056a29f in SquidMain (argc=optimized out,
argv=optimized out) at main.cc:1500
#14 0x004beea6 in SquidMainSafe (argv=optimized out,
argc=optimized out) at main.cc:1215
#15 main (argc=optimized out, argv=optimized out) at main.cc:1207
Re: bzr unmerge
On Sat, Aug 18, 2012 at 6:58 AM, Henrik Nordström hen...@henriknordstrom.net wrote: fre 2012-08-17 klockan 19:59 +0200 skrev Kinkie: Have you considered bzr uncommit? uncommit do not cut it. uncommit simply moves the branch head to a given revision discarding any later revisions, same as git reset --hard for those familiar with git. It's ok to use on a private branch to clean up mistakes, but MUST NOT, repeat MUST NOT be used after the revisions have been pushed to a shared repository. what I want is a unmerge operation that is a commit in itself much like revert, preserving full history, but which requires attention to resolve when propagated to other branches, enabling other branches to keep the changes or revert them per user choice. bzr merge N..N-1 . will back out revision N from this branch in the way you describe.
Re: Policy for reverting changes
On 08/17/2012 10:49 AM, Henrik Nordström wrote: Reversals are not routine. There is no automatic reversal, instead it is up to each and everyone to decide on when reversal is appropriate and when not. Ok, here is a proposal on a little policy on the topic on how to handle others bad commits: trunk: 1. If possible, fix it. 2. If not, nag the person who committed the problematic code. 3a. If no response in 24 hours and it's blocking other things (including test farm builds) then revert the commit with a notice to squid-dev. 3b. If still broken after 7 days of discussion and attempts to fix then revert the change. 4. Let it mature in a development branch, and submit for merge again when the issues have been resolved. numbered branches: 1. If possible, fix it. 2. If not, nag the person who committed the problematic code. 3. Let the release manager decide on reverting The above policy places places a 24 hour wait time that may not be appropriate in emergencies. On the other hand, it may be interpreted as a permission to revert any change that a person considers broken after 7 days of squid-dev discussions, which may result in painful and unnecessary reversals. Let's step back a little and look at what we are trying to achieve here. A good reversal: (a) Balances the reversal effects with the effects of keeping the broken code. For example, reversing a high-demand but optional feature that breaks a default farm build on an obscure platform may not be such a good idea. On the other hand, reversing a commit that breaks build on on all platforms should not be frowned upon, especially if error reports are not followed by immediate fixes (indicating that the responsible party is unavailable to fix things promptly). (b) Details the reason for reversal so that the responsible party has a reasonable path forward to fix the problem or object to the reversal. For example, does not seem to work for me is a bad reversal commit message, but breaks 'make check' on Fedora, with the following error is a good one. Since balance and broken are relative terms requiring judgment calls, most reversal decisions should be driven by squid-dev consensus except in emergencies. Also, a quick reversal of the last commit is usually a far less painless action for most parties than reversing a 7-day old code with other changes piled on top of it (in trunk and in developers' branches). The policy should reflect that as well, perhaps by focusing on fresh commits and emergency reversals while allowing the usual squid-dev consensus process to handle other/older issues. With trunk focus, I would suggest the following: 1. The party responsible for the committed code should monitor farm build errors, squid-dev, and bugzilla for signs of trouble after the commit. If you are responsible and notice error reports, either promptly fix the bugs or inform squid-dev that you will not (with some justification, of course). [ Thus, it is usually a bad idea to commit changes when the responsible party cannot monitor for signs of trouble and promptly address problems. ] If you are responsible, expect people to pummel you and demand corrective actions -- *you* are at fault if your code breaks things (but you may disagree that something is broken and/or refuse to make prompt corrections for various reasons, of course). Problem reports do not imply folks do not appreciate your contributions. Others should report problems to squid-dev or bugzilla. For fresh trunk problems, squid-dev is the preferred route. Fixes may be helpful as well, but keep in mind that the responsible party may be working on the same kind of fix already so some coordination may be desirable. The committer is not necessarily the responsible party. 2. If you think it is an emergency (which is rather unlikely for trunk!), you may reverse any recent commit. If you do, make sure you document why the commit was reversed: detail the bug/problem in the commit message and provide emergency justification (and notification) on squid-dev. You become responsible for the reversal. You will need a committer assistance if you lack commit privileges, of course. 3. If you do not think this is an emergency, discuss the problem on squid-dev and act based on the consensus there. In your first post, CC the responsible party. Thank you, Alex.
Native FTP proxying
Hello, What do you think about adding native or pure FTP proxying support to Squid? That is, is it a good idea for Squid to start accepting pure FTP requests (not wrapped in HTTP). Here are the pros and cons I could come up with: Pros: + There is definitely a persistent demand for native FTP support in Squid, especially in environments where Squid is used as a hub for content blocking and adaptation (various URL, ICAP, and eCAP filters). + FTP is expected to be around for the foreseeable future. Most diverse environments have at least one essential FTP client, possibly due to the ease of FTP client deployment on various platforms (not to mention legacy issues). + We already have server-side native FTP support so we do not have to start from scratch. + Popular proprietary proxies (e.g., BlueCoat and McAfee WebGateway) support native FTP proxying. + Existing native FTP-only proxies (e.g., FROX and ftp-proxy) have very limited content blocking and adaptation functionality and, judging by their projects activity, one should not expect those and other modern deployment features to be supported in the foreseeable future. Cons: - FTP is a dying protocol, responsible for just a few percent of traffic in most environments. - There are existing native FTP-only proxies (e.g., FROX and ftp-proxy). - This will further complicate client-side Squid code that is currently in a pretty bad shape. What do you think? Should a quality implementation of native FTP support be accepted by the Squid Project (with specific major design choices to be discussed before the development, as usual)? Thank you, Alex.
Re: Policy for reverting changes
fre 2012-08-17 klockan 16:03 -0600 skrev Alex Rousskov: The above policy places places a 24 hour wait time that may not be appropriate in emergencies. On the other hand, it may be interpreted as a permission to revert any change that a person considers broken after 7 days of squid-dev discussions, which may result in painful and unnecessary reversals. If 7 days of discussions and attempted fixing can not fix the code to an acceptable level then it's not yet ready for trunk and should go back to live in a developer feature branch and resubmitted for merge when in better shape. (a) Balances the reversal effects with the effects of keeping the broken code. For example, reversing a high-demand but optional feature that breaks a default farm build on an obscure platform may not be such a good idea. On the other hand, reversing a commit that breaks build on on all platforms should not be frowned upon, especially if error reports are not followed by immediate fixes (indicating that the responsible party is unavailable to fix things promptly). It's very hard to say exactly where the limit is. For as long as that code breaks that obscure platform the build tests for other code on that platform is offline. Repeat this a number of times and we soon have no platforms that builds. (b) Details the reason for reversal so that the responsible party has a reasonable path forward to fix the problem or object to the reversal. For example, does not seem to work for me is a bad reversal commit message, but breaks 'make check' on Fedora, with the following error is a good one. Agreed. Since balance and broken are relative terms requiring judgment calls, most reversal decisions should be driven by squid-dev consensus except in emergencies. I do not see reversals of trunk changes as such as that intrusive to the development process. Not even when it's a bit older commit. But we need to get the hang of how to handle this proper in bzr so that the reversal propagates only to the development branches where it should propagate. Regards Henrik
Re: Squid 3.2.1 is dumping core
Those TODO are for future planed code changes, and unrelated to the
asser().
The object is empty here unless
a) There is memory reference error / corruption
b) Something attempts to stuff two replies into the same object.
Regards
HEnrik
fre 2012-08-17 klockan 13:55 -0700 skrev Linda W:
Alex?:
Did I catch some unfinished code?
Looks like I am failing after the TODO's with asserts to check
that structures are coming back empty'ed...and they aren't.
*hmmm*
(store.cc failed code section below..)
void
StoreEntry::startWriting()
{
Packer p;
/* TODO: when we store headers serparately remove the header portion */
/* TODO: mark the length of the headers ? */
/* We ONLY want the headers */
packerToStoreInit(p, this);
assert (isEmpty());###--assertion
failed...#1854; TODO?
assert(mem_obj);
const HttpReply *rep = getReply();
assert(rep);
rep-packHeadersInto(p);
mem_obj-markEndOfReplyHeaders();
rep-body.packInto(p);
packerClean(p);
}
Linda W wrote:
Amos Jeffries wrote:
The Squid HTTP Proxy team is very pleased to announce the availability
of the Squid-3.2.1 release!
---
BTW ---
I finally managed to get a good (I think) stack traceback.
(N.B. - earlier versions were dumping core, but never got the
debug symbols in correctly for a good stack trackback). This time
I did...
This look familiar for someone who's doing something wrong or should
squid be more tolerant? ;-)
Core was generated by `(squid-6)'.
Program terminated with signal 6, Aborted.
#0 0x003000434d95 in __GI_raise (sig=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
(gdb) where
#0 0x003000434d95 in __GI_raise (sig=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x0030004362ab in __GI_abort () at abort.c:93
#2 0x005042e5 in xassert (msg=0x698648 isEmpty(),
file=optimized out, line=1854) at debug.cc:567
#3 0x005938b4 in StoreEntry::startWriting (this=0x27942d0)
at store.cc:1854
#4 0x005a380c in ServerStateData::setFinalReply
(this=0x27ae6c8, rep=
0x27b9890) at Server.cc:173
#5 0x0054b0b0 in HttpStateData::processReply (this=0x27ae6c8)
at http.cc:1168
#6 0x0054b28a in HttpStateData::readReply (this=0x27ae6c8,
io=optimized out) at http.cc:1145
#7 0x0054d0eb in JobDialerHttpStateData::dial
(this=0x2814240, call=
...) at base/AsyncJobCalls.h:175
#8 0x005f9b74 in AsyncCall::make (this=0x2814210) at
AsyncCall.cc:36
#9 0x005fcbb7 in AsyncCallQueue::fireNext (this=optimized
out)
at AsyncCallQueue.cc:54
#10 0x005fcd10 in AsyncCallQueue::fire (this=0x2340480)
at AsyncCallQueue.cc:40
#11 0x0051168c in EventLoop::runOnce (this=0x7fff6fb3d720)
at EventLoop.cc:131
#12 0x00511758 in EventLoop::run (this=0x7fff6fb3d720)
at EventLoop.cc:95
#13 0x0056a29f in SquidMain (argc=optimized out,
argv=optimized out) at main.cc:1500
#14 0x004beea6 in SquidMainSafe (argv=optimized out,
argc=optimized out) at main.cc:1215
#15 main (argc=optimized out, argv=optimized out) at main.cc:1207
Re: Native FTP proxying
fre 2012-08-17 klockan 18:03 -0600 skrev Alex Rousskov: What do you think about adding native or pure FTP proxying support to Squid? That is, is it a good idea for Squid to start accepting pure FTP requests (not wrapped in HTTP). Not sure. It shares very little in common with HTTP proxying. Here are the pros and cons I could come up with: Pros: + There is definitely a persistent demand for native FTP support in Squid, especially in environments where Squid is used as a hub for content blocking and adaptation (various URL, ICAP, and eCAP filters). Yes + FTP is expected to be around for the foreseeable future. Most diverse environments have at least one essential FTP client, possibly due to the ease of FTP client deployment on various platforms (not to mention legacy issues). Yes + We already have server-side native FTP support so we do not have to start from scratch. There is not much of our server-side native FTP support that can be reused for actual FTP proxying. It's tailored for HTTP-FTP gatewaying. + Popular proprietary proxies (e.g., BlueCoat and McAfee WebGateway) support native FTP proxying. Yes + Existing native FTP-only proxies (e.g., FROX and ftp-proxy) have very limited content blocking and adaptation functionality and, judging by their projects activity, one should not expect those and other modern deployment features to be supported in the foreseeable future. Yes. Not sure how much of of FTP that can be reasonably mapped to Squid concepts. FTP is a quite different protocol compared to HTTP, with a lot of state and legacy. Cons: - FTP is a dying protocol, responsible for just a few percent of traffic in most environments. And most of that traffic is using FTP as if it was HTTP, i.e. browser generated traffic. - There are existing native FTP-only proxies (e.g., FROX and ftp-proxy). Yes, and several multi-protocol proxies. - This will further complicate client-side Squid code that is currently in a pretty bad shape. It will complicate the whole forwarding path, not only client-side. Proxyign of FTP has quite different requirements than HTTP. What do you think? Should a quality implementation of native FTP support be accepted by the Squid Project (with specific major design choices to be discussed before the development, as usual)? A FTP client-side might be accepted, allowing Squid to act as an ftp-ftp gateway using HTTP semantics internally, even accepting ftp-http gatewaying if you want. But not sure it makes sense to add native FTP proxy support. Regards Henrik
