Opened 13 months ago

Closed 3 months ago

#7101 closed change (rejected)

Replace metadata.* files with plain .json files.

Reported by: tlucas Assignee:
Priority: Unknown Milestone:
Module: Automation Keywords:
Cc: sebastian, kzar, greiner, erikvold, BrentM Blocked By:
Blocking: #7120 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Background

We configure the build process of packages mainly through our metadata.* files. With our ongoing efforts to move automation-related code to NodeJS, this becomes unfeasible, mainly due to our custom implementation of a ChainedConfigParser, in order to support inheritance between those files.

As a first step, we want to have non-inheriting .json files as configuration-files for each platform.

What to change

in the buildtools repository:

  • remove chainedconfigparser.py
  • attempt to load metadata.<platform>.json files instead of metadata.<platform> files
  • replace every usage of the ChainedConfigParser interface with a standard dictionary / list interface

Integration notes

Replace the metadata.* with the corresponding metadata.<platform>.json files.
this could be achieved by the following script, prior to this change:

translate_meta.py:

from buildtools.packager import readMetadata
import json
import sys

def print_json(platform):
    metadata = readMetadata('.', platform)
    result = {}

    skipped_sections = [
        "default",
    ]

    for key in [key for key in sorted(metadata.sections())
                if key not in skipped_sections]:
        result[key] = {}
        metadata.serialize_section_if_present(key, result[key])

    print json.dumps(result, indent=2, sort_keys=True)

if __name__ == "__main__": 
    print_json(sys.argv[1])

Usage:

$ python translate_meta.py chrome > metadata.chrome.json

Change History (9)

comment:1 follow-up: Changed 13 months ago by sebastian

Perhaps we should first identify the differences between our generated manifest.json files on different platfroms, and then see if we can do some automatic transformation (e.g. stripping unsupported keys) based on the platform rather than using inheritance.

comment:2 Changed 13 months ago by greiner

  • Cc greiner added

comment:3 in reply to: ↑ 1 Changed 13 months ago by tlucas

Replying to sebastian:

Perhaps we should first identify the differences between our generated manifest.json files on different platfroms, and then see if we can do some automatic transformation (e.g. stripping unsupported keys) based on the platform rather than using inheritance.

The overall goal is to get rid of inheritance, yes - i wanted to keep this issue minimal, so that we have json configurations, which we can start with. FWIW if we only cared about the generated manifest.json files, we'd still have to keep the metadata.* for e.g. mappings - and i wanted to avoid that.

comment:4 Changed 13 months ago by tlucas

  • Summary changed from Replace metadata.* files with plain mainfest.json files. to Replace metadata.* files with plain .json files.

comment:5 Changed 12 months ago by tlucas

  • Blocking 7120 added

comment:6 Changed 12 months ago by erikvold

  • Cc erikvold added

comment:7 Changed 12 months ago by kzar

  • Cc BrentM added

Please also consider AdBlock as well, metadata inheritance might be necessary for them. See metadata.adblock.

comment:8 Changed 12 months ago by sebastian

My idea is to have a standard (single) manifest.json file in adblockpluschrome and then have our new (to be written) JavaScript-based packager add/strip platform specific and generated keys when building the extension. Forks like AdBlock can then just provide their own manifest.json base file which should give them sufficient flexibility.

FWIW, I'm not sure if it makes sense to tackle this before the JavaScript rewrite of build.py build. It seems to be a lot of redundant work, to implement this once in Python and then again in JavaScript, and if those two implementations won't be 100% compatible, we end up breaking the configuration mechanism twice which I'd like to avoid.

Last edited 12 months ago by sebastian (previous) (diff)

comment:9 Changed 3 months ago by sebastian

  • Resolution set to rejected
  • Status changed from new to closed

The above comment is still relevant, and as it hasn't been challenged in 9 months, I'm rejecting this issue now.

Note: See TracTickets for help on using tickets.