Bug 314

Summary: NSC support wscript changes appear buggy
Product: ns-3 Reporter: Gustavo J. A. M. Carneiro <gjcarneiro>
Component: build systemAssignee: Gustavo J. A. M. Carneiro <gjcarneiro>
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs
Priority: P3    
Version: ns-3.2   
Hardware: All   
OS: All   

Description Gustavo J. A. M. Carneiro 2008-09-05 13:02:53 UTC
in src/internet-stack/wscript

    if arch == 'x86_64' or arch == 'i686' or arch == 'i586' or arch == 'i486' or arch == 'i386':
        conf.env['NSC_ENABLED'] = 'yes'
        conf.define('NETWORK_SIMULATION_CRADLE', 1)
        conf.write_config_header('ns3/core-config.h')
        e = conf.create_library_configurator()
        e.mandatory = True
        e.name = 'dl'
        e.define = 'HAVE_DL'
        e.uselib = 'DL'
        e.run()
        ok = True

First, what is the "conf.write_config_header('ns3/core-config.h')" doing there? It overwrites an existing configuration header.  Sounds like a copy paste error.  I'll rename this header file to internet-stack-config.h, if no objections.

Next,
        e.run()
        ok = True

Shouldn't this be:
        ok = e.run()
?

Finally I think it is more modular to move opt.add_option('--nsc', ...) from the toplevel wscript file into src/internet-stack/wscript, for better organization.

I'll fix these things if no objections.
Comment 1 Gustavo J. A. M. Carneiro 2008-09-05 13:04:24 UTC
Oh, I see now e.mandatory = True when checking for libdl.  So never mind that part (assuming this is intentional).
Comment 2 Mathieu Lacage 2008-09-05 13:53:06 UTC
while you are at it, --nsc should be renamed to --nsc-enable to match what you did for --python-disable.


Comment 3 Tom Henderson 2008-09-05 14:11:50 UTC
(In reply to comment #2)
> while you are at it, --nsc should be renamed to --nsc-enable to match what you
> did for --python-disable.
> 

While we are on the topic, is there any reason not to align the syntax with configure?

Optional Features:
  --disable-FEATURE       do not include FEATURE (same as --enable-FEATURE=no)
  --enable-FEATURE[=ARG]  include FEATURE [ARG=yes]


--python-disable instead of --disable-python has tripped me up in the past
Comment 4 Mathieu Lacage 2008-09-05 14:17:39 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > while you are at it, --nsc should be renamed to --nsc-enable to match what you
> > did for --python-disable.
> > 
> 
> While we are on the topic, is there any reason not to align the syntax with
> configure?

+1

Comment 5 Gustavo J. A. M. Carneiro 2008-09-05 14:18:53 UTC
I was just commenting about this on IRC.
In principle I dislike --python-disable, and prefer --disable-python, but:

gjc@dark-tower:ns-3-dev$ ./waf --python
Usage: waf [options] [commands ...]

* Main commands: configure build install clean dist distclean uninstall distcheck
* Example: ./waf build -j4

waf: error: ambiguous option: --python (--python-disable, --python-scan?)

I can easily discover options related to python with this :-)
And I can type incomplete options, ./waf --python-dis works, for example.

Ayway, I am +0.5 for the --python-disable form.  But I won't be much upset if we decide to go for --disable-python instead.
Comment 6 Mathieu Lacage 2008-09-08 12:34:30 UTC
all this has been fixed.