Re: [MERGE] Remove old .h file tests from unit-testing
On Mon, 2008-03-10 at 18:59 +1300, Amos Jeffries wrote: Alex Rousskov wrote: IMO, at the end of the cleanup, squid.h should be the file that all sources are guaranteed to include first simply because it is called squid.h. Config.h has, by name, a much narrower scope. I realize that this is subjective and that very few people care about things like that in Squid so if, for whatever reason, Amos wants to remove squid.h from most files and leave config.h instead, I am not going to object since I am not the person doing the cleanup leg work, and since the functional effect will be the same. The little tweaks that are not global in nature should be moved out to specific headers and wrappers, of course. The guaranteed-to-be-included-first.h file should eventually be almost empty. Its primary value is the guarantee itself, which becomes a useful tool for [usually temporary] hacks. With the testheaders.sh every .h now has guaranteed-to-be-included-at-earliest-actual-need. Yes, that is great(*), but to-be-included-first guarantee is a little different (and much easier to support/test). The hacks should be localized to the object(s) they are hacking anyway and not affect the entire Squid + sub-programs + linked-programs + helpers + unit-tests + tools. If they do you would be: - hacking a system include (which requires config.h for the wrapping!) - hacking an objects behaviour (which gets linked/included everywhere its used anyway!) The guaranteed-to-be-included-first.h file is needed for exceptional hacks that defy normal rules. For example, it is 3:00 in the morning and you must make Squid to compile on some remote box, but some internal system header is broken and needs a special _BSD_API #define to compile. Many system headers include that header conditionally so you cannot just quickly wrap a system include file to #define that constant. If only you could define it before any system header is included! Such hacks are usually temporary. Again, I am just sharing my experience here. No significant pressure to implement guaranteed-to-be-included-first.h file idea... Thank you, Alex. P.S. Can a script reliably test an included-at-earliest-actual-need guarantee? I think I can write a header that compiles fine without config.h but still needs it to enable an optional API. How would a script detect such a need?
Re: [MERGE] Remove old .h file tests from unit-testing
Alex Rousskov wrote: On Mon, 2008-03-10 at 18:59 +1300, Amos Jeffries wrote: Alex Rousskov wrote: IMO, at the end of the cleanup, squid.h should be the file that all sources are guaranteed to include first simply because it is called squid.h. Config.h has, by name, a much narrower scope. I realize that this is subjective and that very few people care about things like that in Squid so if, for whatever reason, Amos wants to remove squid.h from most files and leave config.h instead, I am not going to object since I am not the person doing the cleanup leg work, and since the functional effect will be the same. The little tweaks that are not global in nature should be moved out to specific headers and wrappers, of course. The guaranteed-to-be-included-first.h file should eventually be almost empty. Its primary value is the guarantee itself, which becomes a useful tool for [usually temporary] hacks. With the testheaders.sh every .h now has guaranteed-to-be-included-at-earliest-actual-need. Yes, that is great(*), but to-be-included-first guarantee is a little different (and much easier to support/test). The hacks should be localized to the object(s) they are hacking anyway and not affect the entire Squid + sub-programs + linked-programs + helpers + unit-tests + tools. If they do you would be: - hacking a system include (which requires config.h for the wrapping!) - hacking an objects behaviour (which gets linked/included everywhere its used anyway!) The guaranteed-to-be-included-first.h file is needed for exceptional hacks that defy normal rules. For example, it is 3:00 in the morning and you must make Squid to compile on some remote box, but some internal system header is broken and needs a special _BSD_API #define to compile. Many system headers include that header conditionally so you cannot just quickly wrap a system include file to #define that constant. If only you could define it before any system header is included! Such hacks are usually temporary. Those hacks would go in config.h around #include autoconf.h. Which in squid is guaranteed-before-system-headers. The few places where the sys headers are not wrapped need fixing. Again, I am just sharing my experience here. No significant pressure to implement guaranteed-to-be-included-first.h file idea... Thank you, Alex. P.S. Can a script reliably test an included-at-earliest-actual-need guarantee? I think I can write a header that compiles fine without config.h but still needs it to enable an optional API. How would a script detect such a need? Thats the tricky bit. It can test the guarantee for included files and custom types. Which covers most of the cases, where the optional API is represented by a class or uses new branch-specific fields of objects. What can't be tested this way is the inverse case of code which does not add object fields anywhere, reference branch-specific fields, or use a branch-specific class API. That case can only really be tested by hand. Amos -- Please use Squid 2.6STABLE17+ or 3.0STABLE1+ There are serious security advisories out on all earlier releases.
Re: [MERGE] Remove old .h file tests from unit-testing
Bundle Buggy has detected this merge request. For details, see: http://squid-cache.org/bundlebuggy//request/%3C20080309110310.889A6E9FF1%40treenet.co.nz%3E
Re: [MERGE] Remove old .h file tests from unit-testing
On Mon, 2008-03-10 at 00:03 +1300, Amos Jeffries wrote: - removes squid.h includes from many unit-tests I think this is a step backwards (which may be OK as a temporary measure, but I think there are better ways to deal with the problem at hand). IMO, virtually all Squid sources should include (directly or indirectly) squid.h as the first include file. In my experience, this design is very useful for portability and other hacks. The squid.h file itself should have little more than a guarded config.h include, of course. We have the opposite situation now where squid.h includes half of Squid headers (directly or indirectly). That should be fixed. I would rather see squid.h cleaned up (which is trivial) and SquidDirty.h file temporary introduced for those source files that need the current dirty include-everything squid.h. This way, virtually all sources, including test cases, can continue to include squid.h. Some will also include SquidDirty.h. Uses of SquidDirty.h will be slowly weeded out. Thank you, Alex.
Re: [MERGE] Remove old .h file tests from unit-testing
On Sun, 2008-03-09 at 11:23 -0600, Alex Rousskov wrote: IMO, virtually all Squid sources should include (directly or indirectly) squid.h as the first include file. In my experience, this design is very useful for portability and other hacks. We have two such files include/config.h project global tweaks src/squid.hspecific to the squid binary with src being split up a lot of the tweaking we have in squid.h either goes to config.h or into it's specific module. I would rather see squid.h cleaned up (which is trivial) and SquidDirty.h file temporary introduced for those source files that need the current dirty include-everything squid.h. This way, virtually all sources, including test cases, can continue to include squid.h. Some will also include SquidDirty.h. Uses of SquidDirty.h will be slowly weeded out. Good idea. Or we could consider squid.h the dirty one, moving out the little tweaks which is there to config.h or better places. But it's a little tricky with some of the tweaks which depends heavily on system.headers... Regards Henrik
Re: [MERGE] Remove old .h file tests from unit-testing
On Sun, 2008-03-09 at 11:23 -0600, Alex Rousskov wrote: IMO, virtually all Squid sources should include (directly or indirectly) squid.h as the first include file. In my experience, this design is very useful for portability and other hacks. We have two such files include/config.h project global tweaks src/squid.hspecific to the squid binary with src being split up a lot of the tweaking we have in squid.h either goes to config.h or into it's specific module. I would rather see squid.h cleaned up (which is trivial) and SquidDirty.h file temporary introduced for those source files that need the current dirty include-everything squid.h. This way, virtually all sources, including test cases, can continue to include squid.h. Some will also include SquidDirty.h. Uses of SquidDirty.h will be slowly weeded out. Good idea. Or we could consider squid.h the dirty one, moving out the little tweaks which is there to config.h or better places. Exactly what I'm currently doing with the cleanup patch. The patch is linking a lot of files previously unlinked to the 'clean' config.h which they need. Also moving a few 'unclean' items which sould not be directly in a global include out of squid.h so other objects no longer depend on the 'dirty' squid.h But it's a little tricky with some of the tweaks which depends heavily on system.headers... Thats fine, as long as they are limited to tweaking system headers. They can even go in their own tweak file if very messy and be included by config.h instead of inline to keep it relatively clean. Same way the current squid.h include defines.h and typedefs.h. config.h (as future squid.h) should NEVER directly include any classes squid custom defines (like: #include HttpHeader.h). At worst they might be in a libX.la specific group header _if such group .h is needed at all_ (thinking src/squid-common.h + src/libsquid-common.la etc). In most cases you will find the object has few enough depends that its easy to just include the minimal .h files directly into its .cc. HTH Amos
Re: [MERGE] Remove old .h file tests from unit-testing
On Sun, 2008-03-09 at 22:23 +0100, Henrik Nordstrom wrote: Or we could consider squid.h the dirty one, moving out the little tweaks which is there to config.h or better places. IMO, at the end of the cleanup, squid.h should be the file that all sources are guaranteed to include first simply because it is called squid.h. Config.h has, by name, a much narrower scope. I realize that this is subjective and that very few people care about things like that in Squid so if, for whatever reason, Amos wants to remove squid.h from most files and leave config.h instead, I am not going to object since I am not the person doing the cleanup leg work, and since the functional effect will be the same. The little tweaks that are not global in nature should be moved out to specific headers and wrappers, of course. The guaranteed-to-be-included-first.h file should eventually be almost empty. Its primary value is the guarantee itself, which becomes a useful tool for [usually temporary] hacks. Thank you, Alex.
Re: [MERGE] Remove old .h file tests from unit-testing
Alex Rousskov wrote: On Sun, 2008-03-09 at 22:23 +0100, Henrik Nordstrom wrote: Or we could consider squid.h the dirty one, moving out the little tweaks which is there to config.h or better places. IMO, at the end of the cleanup, squid.h should be the file that all sources are guaranteed to include first simply because it is called squid.h. Config.h has, by name, a much narrower scope. I realize that this is subjective and that very few people care about things like that in Squid so if, for whatever reason, Amos wants to remove squid.h from most files and leave config.h instead, I am not going to object since I am not the person doing the cleanup leg work, and since the functional effect will be the same. The little tweaks that are not global in nature should be moved out to specific headers and wrappers, of course. The guaranteed-to-be-included-first.h file should eventually be almost empty. Its primary value is the guarantee itself, which becomes a useful tool for [usually temporary] hacks. With the testheaders.sh every .h now has guaranteed-to-be-included-at-earliest-actual-need. This has the side-effect of giving config.h a guaranteed-to-be-included due to most .h or .cc files using #if HAVE... syntax to system includes. Even if its not included explicitly in the local .h The hacks should be localized to the object(s) they are hacking anyway and not affect the entire Squid + sub-programs + linked-programs + helpers + unit-tests + tools. If they do you would be: - hacking a system include (which requires config.h for the wrapping!) - hacking an objects behaviour (which gets linked/included everywhere its used anyway!) Amos -- Please use Squid 2.6STABLE17+ or 3.0STABLE1+ There are serious security advisories out on all earlier releases.
