node: Refactor rc file handling

- Use more "proper" package-manager-specific parsing, including what
  should be a fully-compliant (albiet over-engineered) parser for npm's
  ini format used in .npmrc.
- Moves all config loading into its own set of classes. (As it turns
  out, this *does* walk recursively, contrary to what I said before,
  oops!)
- Terminology change: rename rcfile -> config, just to match more with
  how npm describes itself.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>

Author: refi64

node/flatpak_node_generator/main.py

@@ -1,5 +1,5 @@
 from pathlib import Path
-from typing import Iterator, List, Set
+from typing import Dict, Iterator, List, Set
 
 import argparse
 import asyncio
@@ -12,7 +12,7 @@
 from .node_headers import NodeHeaders
 from .package import Package
 from .progress import GeneratorProgress
-from .providers import ProviderFactory
+from .providers import Config, ProviderFactory
 from .providers.npm import NpmLockfileProvider, NpmModuleProvider, NpmProviderFactory
 from .providers.special import SpecialSourceProvider
 from .providers.yarn import YarnProviderFactory
@@ -189,20 +189,20 @@ async def _async_main() -> None:
 
     print('Reading packages from lockfiles...')
     packages: Set[Package] = set()
-    rcfile_node_headers: Set[NodeHeaders] = set()
+    config_node_headers: Set[NodeHeaders] = set()
 
-    for lockfile in lockfiles:
-        lockfile_provider = provider_factory.create_lockfile_provider()
-        rcfile_providers = provider_factory.create_rcfile_providers()
+    lockfile_provider = provider_factory.create_lockfile_provider()
+    config_provider = provider_factory.create_config_provider()
+
+    lockfile_configs: Dict[Path, Config] = {}
 
+    for lockfile in lockfiles:
         packages.update(lockfile_provider.process_lockfile(lockfile))
+        lockfile_configs[lockfile] = config = config_provider.load_config(lockfile)
 
-        for rcfile_provider in rcfile_providers:
-            rcfile = lockfile.parent / rcfile_provider.RCFILE_NAME
-            if rcfile.is_file():
-                nh = rcfile_provider.get_node_headers(rcfile)
-                if nh is not None:
-                    rcfile_node_headers.add(nh)
+        nh = config.get_node_headers()
+        if nh is not None:
+            config_node_headers.add(nh)
 
     print(f'{len(packages)} packages read.')
 
@@ -220,14 +220,18 @@ async def _async_main() -> None:
         )
         special = SpecialSourceProvider(gen, options)
 
-        with provider_factory.create_module_provider(gen, special) as module_provider:
+        with provider_factory.create_module_provider(
+            gen,
+            special,
+            lockfile_configs,
+        ) as module_provider:
             with GeneratorProgress(
                 packages,
                 module_provider,
                 args.max_parallel,
             ) as progress:
                 await progress.run()
-        for headers in rcfile_node_headers:
+        for headers in config_node_headers:
             print(f'Generating headers {headers.runtime} @ {headers.target}')
             await special.generate_node_headers(headers)
 

node/flatpak_node_generator/providers/__init__.py

@@ -1,7 +1,8 @@
+from dataclasses import dataclass
 from pathlib import Path
-from typing import ContextManager, Dict, Iterator, List, Optional
+from typing import Any, ContextManager, Dict, Iterator, List, Optional, Tuple
 
-import re
+import dataclasses
 import urllib.parse
 
 from ..manifest import ManifestGenerator
@@ -45,32 +46,48 @@ def process_lockfile(self, lockfile: Path) -> Iterator[Package]:
         raise NotImplementedError()
 
 
-class RCFileProvider:
-    RCFILE_NAME: str
+@dataclass
+class Config:
+    data: Dict[str, Any] = dataclasses.field(default_factory=lambda: {})
 
-    def parse_rcfile(self, rcfile: Path) -> Dict[str, str]:
-        with open(rcfile, 'r') as r:
-            rcfile_text = r.read()
-        parser_re = re.compile(
-            r'^(?!#|;)(\S+)(?:\s+|\s*=\s*)(?:"(.+)"|(\S+))$', re.MULTILINE
-        )
-        result: Dict[str, str] = {}
-        for key, quoted_val, val in parser_re.findall(rcfile_text):
-            result[key] = quoted_val or val
-        return result
-
-    def get_node_headers(self, rcfile: Path) -> Optional[NodeHeaders]:
-        rc_data = self.parse_rcfile(rcfile)
-        if 'target' not in rc_data:
+    def merge_new_keys_only(self, other: Dict[str, Any]) -> None:
+        for key, value in other.items():
+            if key not in self.data:
+                self.data[key] = value
+
+    def get_node_headers(self) -> Optional[NodeHeaders]:
+        if 'target' not in self.data:
             return None
-        target = rc_data['target']
-        runtime = rc_data.get('runtime')
-        disturl = rc_data.get('disturl')
+        target = self.data['target']
+        runtime = self.data.get('runtime')
+        disturl = self.data.get('disturl')
 
         assert isinstance(runtime, str) and isinstance(disturl, str)
 
         return NodeHeaders.with_defaults(target, runtime, disturl)
 
+    def get_registry_for_scope(self, scope: str) -> Optional[str]:
+        return self.data.get(f'{scope}:registry')
+
+
+class ConfigProvider:
+    @property
+    def _filename(self) -> str:
+        raise NotImplementedError()
+
+    def parse_config(self, path: Path) -> Dict[str, Any]:
+        raise NotImplementedError()
+
+    def load_config(self, lockfile: Path) -> Config:
+        config = Config()
+
+        for parent in lockfile.parents:
+            path = parent / self._filename
+            if path.exists():
+                config.merge_new_keys_only(self.parse_config(path))
+
+        return config
+
 
 class ModuleProvider(ContextManager['ModuleProvider']):
     async def generate_package(self, package: Package) -> None:
@@ -81,10 +98,13 @@ class ProviderFactory:
     def create_lockfile_provider(self) -> LockfileProvider:
         raise NotImplementedError()
 
-    def create_rcfile_providers(self) -> List[RCFileProvider]:
+    def create_config_provider(self) -> ConfigProvider:
         raise NotImplementedError()
 
     def create_module_provider(
-        self, gen: ManifestGenerator, special: SpecialSourceProvider
+        self,
+        gen: ManifestGenerator,
+        special: SpecialSourceProvider,
+        lockfile_configs: Dict[Path, Config],
     ) -> ModuleProvider:
         raise NotImplementedError()

node/flatpak_node_generator/providers/npm.py

@@ -34,7 +34,7 @@
 )
 from ..requests import Requests
 from ..url_metadata import RemoteUrlMetadata
-from . import LockfileProvider, ModuleProvider, ProviderFactory, RCFileProvider
+from . import Config, ConfigProvider, LockfileProvider, ModuleProvider, ProviderFactory
 from .special import SpecialSourceProvider
 
 _NPM_CORGIDOC = (
@@ -103,8 +103,103 @@ def process_lockfile(self, lockfile: Path) -> Iterator[Package]:
         yield from self.process_dependencies(lockfile, data.get('dependencies', {}))
 
 
-class NpmRCFileProvider(RCFileProvider):
-    RCFILE_NAME = '.npmrc'
+class NpmConfigProvider(ConfigProvider):
+    _COMMENT = ('#', ';')
+
+    @property
+    def _filename(self) -> str:
+        return '.npmrc'
+
+    def _parse_value_as_json(self, string: str) -> Any:
+        try:
+            return json.loads(string)
+        except json.JSONDecodeError:
+            return string
+
+    def _parse_value_literal(self, string: str) -> str:
+        result = ''
+        escaped = False
+        for c in string:
+            if escaped:
+                if c not in self._COMMENT and c != '\\':
+                    result += '\\'
+                result += c
+                escaped = False
+            elif c == '\\':
+                escaped = True
+            elif c in self._COMMENT:
+                break
+            else:
+                result += c
+
+        if escaped:
+            result += '\\'
+        return result.strip()
+
+    def _parse_value(self, string: str) -> Any:
+        SINGLE_QUOTE = "'"
+        DOUBLE_QUOTE = '"'
+
+        string = string.strip()
+
+        if string.startswith(SINGLE_QUOTE) and string.endswith(SINGLE_QUOTE):
+            return self._parse_value_as_json(string[1:-1])
+        elif string.startswith(DOUBLE_QUOTE) and string.endswith(DOUBLE_QUOTE):
+            return self._parse_value_as_json(string)
+        else:
+            return self._parse_value_literal(string)
+
+    def _coalesce_to_string(self, value: Any) -> Any:
+        if isinstance(value, list):
+            return ','.join(map(self._coalesce_to_string, value))
+        elif isinstance(value, dict):
+            return '[object Object]'
+        else:
+            return str(value)
+
+    def parse_config(self, path: Path) -> Dict[str, Any]:
+        LITERALS = {
+            'true': True,
+            'false': False,
+            'null': None,
+        }
+
+        result: Dict[str, Any] = {}
+
+        with path.open() as fp:
+            for line in fp:
+                line = line.strip()
+                if not line or line.startswith(self._COMMENT):
+                    continue
+
+                try:
+                    key_s, value_s = line.split('=', 1)
+                except ValueError:
+                    key_s = line
+                    value_s = 'true'
+
+                key = self._coalesce_to_string(self._parse_value(key_s))
+                is_array = False
+                if key.endswith('[]'):
+                    is_array = True
+                    key = key[:-2]
+
+                value = self._parse_value(value_s)
+                if isinstance(value, str):
+                    value = LITERALS.get(value, value)
+
+                if is_array and key not in result:
+                    result[key] = []
+                elif is_array and not isinstance(result[key], list):
+                    result[key] = [result[key]]
+
+                previous_value = result.get(key)
+                if isinstance(previous_value, list):
+                    previous_value.append(value)
+                else:
+                    result[key] = value
+
+        return result
 
 
 class NpmModuleProvider(ModuleProvider):
@@ -124,13 +219,15 @@ def __init__(
         special: SpecialSourceProvider,
         lockfile_root: Path,
         options: Options,
+        lockfile_configs: Dict[Path, Config],
     ) -> None:
         self.gen = gen
         self.special_source_provider = special
         self.lockfile_root = lockfile_root
         self.registry = options.registry
         self.no_autopatch = options.no_autopatch
         self.no_trim_index = options.no_trim_index
+        self.lockfile_configs = lockfile_configs
         self.npm_cache_dir = self.gen.data_root / 'npm-cache'
         self.cacache_dir = self.npm_cache_dir / '_cacache'
         # Awaitable so multiple tasks can be waiting on the same package info.
@@ -143,8 +240,6 @@ def __init__(
         self.git_sources: DefaultDict[
             Path, Dict[Path, GitSource]
         ] = collections.defaultdict(lambda: {})
-        # FIXME better pass the same provider object we created in main
-        self.rcfile_provider = NpmRCFileProvider()
 
     def __exit__(
         self,
@@ -324,21 +419,16 @@ async def generate_package(self, package: Package) -> None:
     def relative_lockfile_dir(self, lockfile: Path) -> Path:
         return lockfile.parent.relative_to(self.lockfile_root)
 
-    @functools.lru_cache(typed=True)
-    def get_lockfile_rc(self, lockfile: Path) -> Dict[str, str]:
-        rc = {}
-        rcfile_path = lockfile.parent / self.rcfile_provider.RCFILE_NAME
-        if rcfile_path.is_file():
-            rc.update(self.rcfile_provider.parse_rcfile(rcfile_path))
-        return rc
-
     def get_package_registry(self, package: Package) -> str:
         assert isinstance(package.source, RegistrySource)
-        rc = self.get_lockfile_rc(package.lockfile)
-        if rc and '/' in package.name:
-            scope, _ = package.name.split('/', maxsplit=1)
-            if f'{scope}:registry' in rc:
-                return rc[f'{scope}:registry']
+        if '/' in package.name:
+            config = self.lockfile_configs.get(package.lockfile)
+            if config is not None:
+                scope, _ = package.name.split('/', maxsplit=1)
+                registry = config.get_registry_for_scope(scope)
+                if registry is not None:
+                    return registry
+
         return self.registry
 
     def _finalize(self) -> None:
@@ -468,10 +558,19 @@ def __init__(self, lockfile_root: Path, options: Options) -> None:
     def create_lockfile_provider(self) -> NpmLockfileProvider:
         return NpmLockfileProvider(self.options.lockfile)
 
-    def create_rcfile_providers(self) -> List[RCFileProvider]:
-        return [NpmRCFileProvider()]
+    def create_config_provider(self) -> NpmConfigProvider:
+        return NpmConfigProvider()
 
     def create_module_provider(
-        self, gen: ManifestGenerator, special: SpecialSourceProvider
+        self,
+        gen: ManifestGenerator,
+        special: SpecialSourceProvider,
+        lockfile_configs: Dict[Path, Config],
     ) -> NpmModuleProvider:
-        return NpmModuleProvider(gen, special, self.lockfile_root, self.options.module)
+        return NpmModuleProvider(
+            gen,
+            special,
+            self.lockfile_root,
+            self.options.module,
+            lockfile_configs,
+        )