vitasdk / vita-headers

Documentation:
https://docs.vitasdk.org
MIT License
224 stars 90 forks source link

Change NID db format #83

Closed yifanlu closed 7 years ago

yifanlu commented 7 years ago
yifanlu commented 7 years ago

We should also have some sort of indicator for the new version. Maybe a tag

// version: 2

at the start? But then it won't comply with JSON standard(s). It's kinda messy to add a new field/object though.

devnoname120 commented 7 years ago

About 2):

devnoname120 commented 7 years ago

We should also have some sort of indicator for the new version. Maybe a tag // version: 2 at the start?

JSON doesn't support comments, so a tag version: 2 at the root of the JSON would work better. It would also allow to programmatically find the version.

d3m3vilurr commented 7 years ago

@devnoname120 parse hex also easy in python, but json not support hexdeciaml.

frangarcj commented 7 years ago

I'm testing hex right now I agree with version: 2

d3m3vilurr commented 7 years ago

i mean json spec. they not support hex number. if can work, probably it's non standard lib extension.

frangarcj commented 7 years ago

https://gist.github.com/frangarcj/a1515d799b4db9216ae074458dd0e2a6

frangarcj commented 7 years ago

Confirmed working with hex values. should I change format?

devnoname120 commented 7 years ago

We need to decide whether we want to switch to YAML or not. An advantage of YAML is that it natively supports hex values. I'm not sure how well YAML libraries handle it though.

Here is how it would look: https://gist.github.com/devnoname120/ff2a08dbcfa0126d9099ce894f7fe783 It seems more readable to me.

frangarcj commented 7 years ago

I think json with hex values is the best solution:

If you like the idea I can make a pull request with latest db.json in new format Changes in vita-import-load are already done

d3m3vilurr commented 7 years ago

yeah.. i'm ok at this time.

but i want to change other format, too. hex string method looks tricky.

DaveeFTW commented 7 years ago

I'm happy to go with hex strings if we enforce that strings must be 10 characters long, beginning with "0x", followed by eight hexadecimal characters where all alphabet characters are capitalised.

Additionally, we should have a simple validator script to automatically verify this repo's json db.

devnoname120 commented 7 years ago

I agree with Davee.

The validator script should be very easy to do in Python.

yne commented 7 years ago

Easy indeed, we simply need to define it schema, then use it to validate.

d3m3vilurr commented 7 years ago

i didn't like json schema (it look too ugly format. IMO). probably, json schema can check formatting using regex. but in this case, also we can write plain python script using just json module. that module is already standard library, so not need extra dependencies for validation. just need dict iterations.. (yeah it can make validation script to be little mess, it just pros & cons)

jdek commented 7 years ago

If we're changing the JSON format, can we use something which is easily parseable in all languages, i.e. something which doesn't require the JSON parser to use objects as arrays and have a custom object for each item in the array.

{ "version" : "3.61",
  "modules" : [
    {
      "name" : "SceDeci4p",
      "nid" : "0x27",
      "libraries" : [
        {
          "name" : "SceDeci4pUserp",
          "nid" : "0x797E39C0",
          "kernel" : false,
          "functions" : [
              { "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" },
              { "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" },
              { "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" },
              { "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" },
              { "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" },
              { "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" },
              { "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" },
              { "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" },
          ],
          "variables": []
        }
      ]
    },
  ]
}

I like the look of YAML though.

frangarcj commented 7 years ago

I like that json version better

El lun., 21 nov. 2016 a las 9:29, Josh de Kock (notifications@github.com) escribió:

If we're changing the JSON format, can we use something which is easily parseable in all languages, i.e. something which doesn't require the JSON parser to use objects as arrays and have a custom object for each item in the array.

{ "version" : "3.61", "modules" : [ { "name" : "SceDeci4p", "nid" : "0x27", "libraries" : [ { "name" : "SceDeci4pUserp", "nid" : "0x797E39C0", "kernel" : false, "functions" : [ { "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" }, { "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" }, { "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" }, { "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" }, { "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" }, { "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" }, { "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" }, { "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" }, ], "variables": [] } ] }, ] }

I like the look of YAML though.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/vitasdk/vita-headers/issues/83#issuecomment-261873978, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYaFnPWEzi_7aL3Sf4jOvIuG5-WxCLhks5rAVZWgaJpZM4KwFST .

devnoname120 commented 7 years ago

@enoposix I think that @yifanlu wanted to use version for the DB specification version. Should we also have a field for the firmware version?

jdek commented 7 years ago

@yifanlu sure, well you could just add a "firmware" field below version easily. I still think the YAML is a better idea--if we are going to be doing a fairly drastic change anyway, the YAML is a lot cleaner. It can be generated extremely easy without the use of a library (just one of the many benefits).

devnoname120 commented 7 years ago

@enoposix The format has already been discussed. Most of us prefer YAML for its cleanness, but it's harder to parse because it has complicated features that we don't need (e.g. references, data-type casting, etc.). Since this file is more meant to be read by a program, JSON seemed fine.

jdek commented 7 years ago

@devnoname120 right, sure. I guess that makes sense.

Why not go all out and allow multiple firmwares in one DB then?

{
  "version" : 2,
  "firmwares" : [
    {
      "firmware": "3.61",
      "modules" : [
        {
          "name" : "SceDeci4p",
          "nid" : "0x27",
          "libraries" : [
            {
              "name" : "SceDeci4pUserp",
              "nid" : "0x797E39C0",
              "kernel" : false,
              "functions" : [
                  { "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" },
                  { "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" },
                  { "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" },
                  { "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" },
                  { "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" },
                  { "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" },
                  { "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" },
                  { "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" },
              ],
              "variables": []
            }
          ]
        },
      ]
    },
  ],
}
frangarcj commented 7 years ago

FORMAT ULTIMATUM :)

Latest discuted in matrix

{
  "version" : "2",
  "firmware": "3.61",
  "modules" : 
        {
        "SceAudioIn": {
        "nid": "0xA",
        "libraries": {
            "SceAudioIn": {
                "functions": {
                    "sceAudioInGetAdopt": "0x566AC433",
                    "sceAudioInGetStatus": "0x2F940377",
                    "sceAudioInInput": "0x638ADD2D",
                    "sceAudioInOpenPort": "0x39B50DC1",
                    "sceAudioInReleasePort": "0x3A61B8C4"
                },
                "kernel": false,
                "nid": "0xF8DC61A3",
                "variables": {}
            }
        }
    }
  }
}

You have 24 hours to review it :)

devnoname120 commented 7 years ago

I know order is not supposed to matter, but I would be in favor of this:

{
  "layout-version" : "2",
  "firmware": "3.61",
  "modules" : 
        {
        "SceAudioIn": {
        "nid": "0x0000000A",
        "libraries": {
            "SceAudioIn": {
                "nid": "0xF8DC61A3",
                "kernel": false,
                "variables": {},
                "functions": {
                    "sceAudioInGetAdopt": "0x566AC433",
                    "sceAudioInGetStatus": "0x2F940377",
                    "sceAudioInInput": "0x638ADD2D",
                    "sceAudioInOpenPort": "0x39B50DC1",
                    "sceAudioInReleasePort": "0x3A61B8C4"
                }
            }
        }
    }
  }
}

Changes:

d3m3vilurr commented 7 years ago

can we ignore empty variables field?

jdek commented 7 years ago

These last two formats are ugly af. They're using data as keys.

frangarcj commented 7 years ago

@enoposix we have agreed to use that instead of arrays

@d3m3vilurr yes

jdek commented 7 years ago

@frangarcj why? there's literally no reason not to use a more parseable format, a user isn't going to be using this. And 'we' have not agreed.

frangarcj commented 7 years ago

It is a long irc log but the use of eg. libraries['SceAudioIn'] was proposed for easy searchs. You have a key - value map vs array.

@enoposix What's your opinion or problems?

Sorry for the 'we' thing

jdek commented 7 years ago

@frangarcj I didn't see the benefit of using objects over arrays, but the idea of using it as an object map makes sense, I see where you're coming from now. Acked.

"layout-version" can just be shortened to "version", there shouldn't be any ambiguity as firmware is there too.

d3m3vilurr commented 7 years ago

I saw a opinion about object of array in the IRC, and remembered it only have little benefits.

  1. make more semantic format design
  2. can support unnamed exports

but currently we not support unnamed functions in this project and channel subject changed dynamic loader instead NIDs.

so actually we didn't decided about this. but I agreed, this issue holded 2 weeks & should make something. it probably better then just holding & no decision.

03:33:49 D<davee[m]> most of the complaints are implementation
03:34:01 D<davee[m]> i find that a good set of work towards a decision talks more
03:34:19 F<frangarcj> I'll make the implementation

format version (or layout-version, and I like just using version) can help this decision.

xyzz commented 7 years ago

Ok I was away for a week so didn't get the chance to submit my opinion but here it is:

Go with yaml, remove json support and json library from vita-toolchain.

Use hex strings (all caps preferred) instead of decimal numbers

This is dumb and hard to parse.

Easy indeed, we simply need to define it schema, then use it to validate.

I don't see any point of doing any kind of validation. We don't have that many changes to json and if something is broken we'll see it on the build bot anyway.

The format has already been discussed. Most of us prefer YAML for its cleanness, but it's harder to parse because it has complicated features that we don't need (e.g. references, data-type casting, etc.). Since this file is more meant to be read by a program, JSON seemed fine.

We aren't writing our own parser so what's the problem? We aren't even using these advanced features. And we already have a yaml parser for exports written so using it for db.json makes sense.

Parsing yaml in C is a shitfest but most people will use some scripting language, like Python, and there it's just one function call: http://pyyaml.org/wiki/PyYAMLDocumentation


It seems that right now the only person who wants to go with json db w/ hex encoded strings is @devnoname120 and everybody else wants yaml or just does not care, so maybe you can explain his position in more detail?

yne commented 7 years ago

Parsing yaml in C is a shitfest

Depend on how strict you want your parser to be. For example this yaml (converted from psplibdoc.xml long time ago): https://github.com/yne/prxtool/blob/master/res/psplibdoc.yml can be parsed using only ~40 line of pure C (who want a yaml dependency anyway): https://github.com/yne/prxtool/blob/master/db_nids.c#L104

I don't see any point of doing any kind of validation

I'm just giving a solution, I'm not the one who proposed ;-)

devnoname120 commented 7 years ago

There was some discussion on IRC and @xyzz said that YAML is cleaner and we have a working parser for the export.yml files, so we should settle for it.

Here is a proposal for the layout of the imports and exports files. (The DB is an imports file.) Imports: https://gist.github.com/devnoname120/7f32f510949561a962f0e6b10e88aefd Exports: https://gist.github.com/devnoname120/da986fa40f9f1c9b0d131c6280684b8b

frangarcj commented 7 years ago

I'm currently converting vita imports to yaml

frangarcj commented 7 years ago

https://github.com/vitasdk/vita-toolchain/pull/83

frangarcj commented 7 years ago

Changed to yaml. Please check actual yml file

devnoname120 commented 7 years ago

@frangarcj It seems good to me. There is only the ForDriver --> ForKernel renaming left to do, and maybe the ForKernel --> ksce.