Bug 4271 - testparm should not print includes
Summary: testparm should not print includes
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.0.23d
Hardware: All Windows XP
: P3 minor
Target Milestone: none
Assignee: Michael Adam
QA Contact: Samba QA Contact
URL:
Keywords:
: 6152 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-11-30 08:18 UTC by Robert Dahlem
Modified: 2009-11-20 18:52 UTC (History)
3 users (show)

See Also:


Attachments
fix for v3-3-test (2.87 KB, patch)
2009-05-17 17:44 UTC, Michael Adam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Dahlem 2006-11-30 08:18:25 UTC
http://www.samba.org/samba/docs/man/Samba-HOWTO-Collection/install.html
states that it is good practice to keep smb.conf as small as possible and recommends keeping documentation in smb.conf.master, then running
    testparm -s smb.conf.master > smb.conf

In this regard it is counterproductive to print include statements like
    include = /etc/smb.conf.global
because this will lead to re-inclusion (and double evaluation) of smb.conf.global

There is no gain in mentioning included files other than documentation purposes. Consequently I propose to prefix include statements with '#'.
Comment 1 Michael Adam 2009-05-04 08:04:13 UTC
I agree that testparm should present a flattened view of the config, not putting include statements generally.

This applies to version 3.2, 3.3 and following, too...
Comment 2 Michael Adam 2009-05-15 19:12:35 UTC
I have applied a fix to master and v3-4-test.
It not only excludes "include" but also the other
meta directives "config file" and "config backend"
from being printed.

Hmm, is this important enough to go into 3.3?
Comment 3 Michael Adam 2009-05-17 17:44:24 UTC
Created attachment 4166 [details]
fix for v3-3-test

This is the patch with the bugfix for v3-3-test.

Could we take it for 3.3?

Michael
Comment 4 Volker Lendecke 2009-05-18 02:20:52 UTC
I'm not sure I necessarily agree with the "flattened" view thing. There is no such thing as a flattened view, as includes depend on the runtime-macros that testparm can never catch. I would much rather like a testparm that does not execute the includes at all but just print them as is, without having done the includes.

Volker
Comment 5 Michael Adam 2009-05-18 03:09:38 UTC
(In reply to comment #4)
> I'm not sure I necessarily agree with the "flattened" view thing. There is no
> such thing as a flattened view, as includes depend on the runtime-macros that
> testparm can never catch.

This is right, but testparm currently _does_ provide flattened view of the config, at least the config that it can see with the current values of macros: as it is, testparm runs lp_load(), which loads the current configuration expamding all includes, thus flattening the include tree, and afterwards dumps out the config from the globals struct and services array.

Therefore imho, as long as this is the modus operandi, includes and other meta directives ("config file" and "config backend") should not be printed.

At least as long as it is a recommended procedure to run 
"testparm -s smb.conf> smb.conf.checked"
to get a working smb.conf file, testparm is currently wrong,
and my fix attacks exactly this.

> I would much rather like a testparm that does not
> execute the includes at all but just print them as is, without having done the
> includes.

This of course also a valid way of dealing with things.
But it would mean a change in paradigm, so I would rather
have it as a second mode of testparm that can be triggered
with a command line switch, or even as a separate binary.

Michael
Comment 6 Volker Lendecke 2009-05-18 03:13:23 UTC
Well, testparm together with includes just does not work. So we need to fix the documentation or provide a hint that "testparm -s smb.conf.master > smb.conf" should *not* be used if meta statements are around.

Volker
Comment 7 Michael Adam 2009-05-18 03:27:16 UTC
(In reply to comment #6)
> Well, testparm together with includes just does not work.

It does now in master and v3-4-test. :-)

> So we need to fix the documentation or provide a hint that 
> "testparm -s smb.conf.master > smb.conf" should *not* be
> used if meta statements are around.

It is not just that:
It is really that printing includes from testparm is deceiving
since it will for one section (share ir global) only ever print
the last include file encountered in that section.

So it is not just a matter of fixing documentation but broken-ness
of testparm printing include statements.

Sorry for insisting, but I think with this change it is really
better than without, even though a completely rewritten testparm
might be better.

Michael
Comment 8 Michael Adam 2009-09-22 15:52:17 UTC
Closing this bug report: It is fixed in 3.4 and won't be fixed in 3.3 any more.
Michael
Comment 9 Michael Adam 2009-11-20 18:52:50 UTC
*** Bug 6152 has been marked as a duplicate of this bug. ***