Allow user to update an existing release

- Added "--final" option to mark the changed release as final
- Allow user to update an existing release
- Do not allow user to modify a released release
- Use dictionaries for visibility scopes
- Merge visibility scopes with the same name when parsing

Fixes #65, #62, #60

README.rst

@@ -183,7 +183,7 @@ Running ``smap update -h`` will give::
   usage: smap update [-h] [-o OUT] [-i INPUT] [-d]
                      [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                      [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE]
-                     [--no_guess] [--allow-abi-break] [-a | --remove]
+                     [--no_guess] [--allow-abi-break] [-f] [-a | --remove]
                      file
   
   positional arguments:
@@ -208,6 +208,8 @@ Running ``smap update -h`` will give::
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
     --allow-abi-break     Allow removing symbols, and to break ABI
+    -f, --final           Mark the modified release as final, preventing later
+                          changes.
     -a, --add             Adds the symbols to the map file.
     --remove              Remove the symbols from the map file. This breaks the
                           ABI.
@@ -221,6 +223,7 @@ Running ``smap new -h`` will give::
   usage: smap new [-h] [-o OUT] [-i INPUT] [-d]
                   [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                   [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE] [--no_guess]
+                  [-f]
   
   optional arguments:
     -h, --help            show this help message and exit
@@ -240,6 +243,8 @@ Running ``smap new -h`` will give::
                           The full name of the release to be used (e.g.
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
+    -f, --final           Mark the new release as final, preventing later
+                          changes.
   
   A list of symbols is expected as the input. If a file is provided with '-i',
   the symbols are read from the given file. Otherwise the symbols are read from

docs/readme.rst

@@ -183,7 +183,7 @@ Running ``smap update -h`` will give::
   usage: smap update [-h] [-o OUT] [-i INPUT] [-d]
                      [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                      [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE]
-                     [--no_guess] [--allow-abi-break] [-a | --remove]
+                     [--no_guess] [--allow-abi-break] [-f] [-a | --remove]
                      file
   
   positional arguments:
@@ -208,6 +208,8 @@ Running ``smap update -h`` will give::
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
     --allow-abi-break     Allow removing symbols, and to break ABI
+    -f, --final           Mark the modified release as final, preventing later
+                          changes.
     -a, --add             Adds the symbols to the map file.
     --remove              Remove the symbols from the map file. This breaks the
                           ABI.
@@ -221,6 +223,7 @@ Running ``smap new -h`` will give::
   usage: smap new [-h] [-o OUT] [-i INPUT] [-d]
                   [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                   [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE] [--no_guess]
+                  [-f]
   
   optional arguments:
     -h, --help            show this help message and exit
@@ -240,6 +243,8 @@ Running ``smap new -h`` will give::
                           The full name of the release to be used (e.g.
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
+    -f, --final           Mark the new release as final, preventing later
+                          changes.
   
   A list of symbols is expected as the input. If a file is provided with '-i',
   the symbols are read from the given file. Otherwise the symbols are read from

docs/usage.rst

@@ -69,7 +69,7 @@ Running ``smap update -h`` will give::
   usage: smap update [-h] [-o OUT] [-i INPUT] [-d]
                      [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                      [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE]
-                     [--no_guess] [--allow-abi-break] [-a | --remove]
+                     [--no_guess] [--allow-abi-break] [-f] [-a | --remove]
                      file
   
   positional arguments:
@@ -94,6 +94,8 @@ Running ``smap update -h`` will give::
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
     --allow-abi-break     Allow removing symbols, and to break ABI
+    -f, --final           Mark the modified release as final, preventing later
+                          changes.
     -a, --add             Adds the symbols to the map file.
     --remove              Remove the symbols from the map file. This breaks the
                           ABI.
@@ -107,6 +109,7 @@ Running ``smap new -h`` will give::
   usage: smap new [-h] [-o OUT] [-i INPUT] [-d]
                   [--verbosity {quiet,error,warning,info,debug} | --quiet | --debug]
                   [-l LOGFILE] [-n NAME] [-v VERSION] [-r RELEASE] [--no_guess]
+                  [-f]
   
   optional arguments:
     -h, --help            show this help message and exit
@@ -126,6 +129,8 @@ Running ``smap new -h`` will give::
                           The full name of the release to be used (e.g.
                           LIBX_1_0_0)
     --no_guess            Disable next release name guessing
+    -f, --final           Mark the new release as final, preventing later
+                          changes.
   
   A list of symbols is expected as the input. If a file is provided with '-i',
   the symbols are read from the given file. Otherwise the symbols are read from

src/smap/symver.py

@@ -298,17 +298,20 @@ def parse(self, lines):
                                                   msg)
                             else:
                                 # New visibility found
-                                v = (identifier, [])
-                                r.symbols.append(v)
+                                if identifier in r.symbols:
+                                    v = r.symbols[identifier]
+                                else:
+                                    v = []
+                                    r.symbols[identifier] = v
                                 column += m.end()
                                 last = (index, column)
                                 state = 2
                                 continue
                         else:
                             if v is None:
                                 # There was no open visibility scope
-                                v = ('global', [])
-                                r.symbols.append(v)
+                                v = []
+                                r.symbols['global'] = v
                                 msg = "Missing visibility scope before"\
                                       " \'{0}\'. Symbols considered in"\
                                       " 'global:\'".format(identifier)
@@ -319,7 +322,7 @@ def parse(self, lines):
                                                              msg))
                             else:
                                 # Symbol found
-                                v[1].append(identifier)
+                                v.append(identifier)
                                 column += m.end()
                                 last = (index, column)
                                 # Move back the state to find elements
@@ -402,9 +405,8 @@ def all_global_symbols(self):
 
         symbols = []
         for release in self.releases:
-            for scope, scope_symbols in release.symbols:
-                if scope.lower() == 'global':
-                    symbols.extend(scope_symbols)
+            if 'global' in release.symbols:
+                symbols.extend(release.symbols['global'])
         return set(symbols)
 
     def duplicates(self):
@@ -507,7 +509,7 @@ def check(self):
 
         # Check '*' wildcard usage
         for release in self.releases:
-            for scope, symbols in release.symbols:
+            for scope, symbols in release.symbols.items():
                 if scope == 'local':
                     if symbols:
                         if "*" in symbols:
@@ -762,22 +764,27 @@ def __init__(self):
         self.name = ''
         self.previous = ''
         self.released = False
-        self.symbols = []
+        self.symbols = dict()
 
     def __str__(self):
+        released = ""
         vs = []
-        for visibility, symbols in self.symbols:
-            symbols.sort()
-            vs.extend([" " * 4, visibility, ":\n",
+        visibilities = sorted(self.symbols.keys())
+        if self.released:
+            released = "    # Released"
+        for v in visibilities:
+            symbols = sorted(self.symbols[v])
+            vs.extend([" " * 4, v, ":\n",
                        "".join((" " * 8 + symbol + ";\n"
                                 for symbol in symbols))])
-        content = "".join(chain(self.name, "\n{\n", vs, "} ",
-                          self.previous, ";\n"))
+        content = "".join(chain(self.name, released, "\n",
+                                "{\n", vs, "} ",
+                                self.previous, ";\n"))
         return content
 
     def duplicates(self):
         duplicates = []
-        for scope, symbols in self.symbols:
+        for scope, symbols in (self.symbols.items()):
             seen = set()
             release_dups = set()
             if symbols:
@@ -938,7 +945,6 @@ def clean_symbols(symbols):
 
     # Report duplicated symbols
     if clean:
-        clean.sort()
         previous = None
         duplicates = set()
         for i in clean:
@@ -1181,22 +1187,39 @@ def update(args):
         print("No symbols added or removed. Nothing done.")
         return
 
+    r = None
+
     if added:
-        r = Release()
-        # Guess the name for the new release
-        r.name = cur_map.guess_name(release_info, guess=args.guess)
-        r.name.upper()
+        if release_info:
+            for to_up in (rs for rs in cur_map.releases if rs and rs.name ==
+                          release_info[0]):
+                # If the release to be modified is released
+                if to_up.released:
+                    msg = "Released releases cannot be modified. Abort."
+                    logger.error(msg)
+                    raise Exception(msg)
+
+                r = to_up
+        else:
+            r = Release()
+            # Guess the name for the new release
+            r.name = cur_map.guess_name(release_info, guess=args.guess)
+            r.name.upper()
+            r.symbols['global'] = []
 
-        # Add the symbols added to global scope
-        r.symbols.append(("global", added))
+            if not removed:
+                # Add the name for the previous release
+                r.previous = latest[0]
 
-        if not removed:
-            # Add the name for the previous release
-            r.previous = latest[0]
+                # Put the release on the map
+                cur_map.releases.append(r)
 
-            # Put the release on the map
-            cur_map.releases.append(r)
+        # If this is the final change to the release, mark as released
+        if args.final:
+            r.released = True
 
+        # Add the symbols added to global scope
+        r.symbols['global'].extend(added)
     if removed:
         if not args.allow_abi_break:
             msg = "ABI break detected: symbols would be removed"
@@ -1226,13 +1249,15 @@ def update(args):
         all_symbols_list = [symbol for symbol in all_symbols if
                             symbol not in removed_set]
 
-        # Sort the list
-        all_symbols_list.sort()
-
-        r.symbols.append(('global', all_symbols_list))
+        # Update the global symbols
+        r.symbols.update({'global': all_symbols_list})
 
         # Add the wildcard to the local symbols
-        r.symbols.append(('local', ['*']))
+        r.symbols.update({'local': ['*']})
+
+        # If this is the final change to the release, mark as released
+        if args.final:
+            r.released = True
 
         # Put the release on the map
         new_map.releases.append(r)
@@ -1335,10 +1360,13 @@ def new(args):
         r.name = name.upper()
 
         # Add the symbols to global scope
-        r.symbols.append(('global', sorted(new_symbols_set)))
+        r.symbols['global'] = list(new_symbols_set)
 
         # Add the wildcard to the local symbols
-        r.symbols.append(('local', ['*']))
+        r.symbols['local'] = ['*']
+
+        if args.final:
+            r.released = True
 
         # Put the release on the map
         new_map.releases.append(r)
@@ -1469,6 +1497,10 @@ def get_arg_parser():
     parser_up.add_argument("--allow-abi-break",
                            help="Allow removing symbols, and to break ABI",
                            action='store_true')
+    parser_up.add_argument("-f", "--final",
+                           help="Mark the modified release as final,"
+                           " preventing later changes.",
+                           action='store_true')
     group = parser_up.add_mutually_exclusive_group()
     group.add_argument("-a", "--add", help="Adds the symbols to the map file.",
                        action='store_true')
@@ -1487,6 +1519,10 @@ def get_arg_parser():
                                        " with \'-i\', the symbols are read"
                                        " from the given file. Otherwise the"
                                        " symbols are read from stdin.")
+    parser_new.add_argument("-f", "--final",
+                            help="Mark the new release as final,"
+                                 " preventing later changes.",
+                            action='store_true')
     parser_new.set_defaults(func=new)
 
     # Check subcommand parser

tests/data/test_as_lib/print_released.stdout

@@ -0,0 +1,8 @@
+BASE_1_0_0    # Released
+{
+    global:
+        one_symbol;
+    local:
+        *;
+} ;
+

tests/data/test_check/base.map

@@ -0,0 +1,9 @@
+# Simple base map
+
+BASE_1_0_0
+{
+    global:
+        one_symbol;
+    local:
+        *;
+} ;

tests/data/test_check/broken_maps.yaml

@@ -72,6 +72,19 @@
       - "Missing visibility scope before"
       - "Symbols considered in 'global:'"
     exceptions:
+-
+  input:
+    args:
+      - "check"
+      - "missing_global.map"
+    stdin:
+  output:
+    file:
+    stdout:
+    warnings:
+      - "Missing visibility scope before"
+      - "Symbols considered in 'global:'"
+    exceptions:
 -
   input:
     args:

tests/data/test_check/missing_global.map

@@ -0,0 +1,10 @@
+# Map missing global declaration before any scope, but added later
+
+MISSING_GLOBAL_1_0_0
+{
+    one_symbol;
+    local:
+        *;
+    global:
+        two_symbol;
+} ;

tests/data/test_check/ok_maps.yaml

@@ -0,0 +1,22 @@
+-
+  input:
+    args:
+      - "check"
+      - "base.map"
+    stdin:
+  output:
+    file:
+    stdout:
+    warnings:
+    exceptions:
+-
+  input:
+    args:
+      - "check"
+      - "split.map"
+    stdin:
+  output:
+    file:
+    stdout:
+    warnings:
+    exceptions:

tests/data/test_check/split.map

@@ -0,0 +1,11 @@
+# Map with global symbols split
+
+SPLIT_1_0_0
+{
+    global:
+        one_symbol;
+    local:
+        *;
+    global:
+        two_symbol;
+} ;