Skip to content

feat: add gen-deploy cli to generate deploy.json#295

Closed
yoyoismee wants to merge 2 commits into
latticexyz:mainfrom
yoyoismee:main
Closed

feat: add gen-deploy cli to generate deploy.json#295
yoyoismee wants to merge 2 commits into
latticexyz:mainfrom
yoyoismee:main

Conversation

@yoyoismee

Copy link
Copy Markdown
Contributor

added mud gen-deploy cli

generate deploy.json from files

param

  • components, path to components. [default to src/components]
  • systems, path to systems. [default to src/systems]
  • outdir, path for output. [default to deploy.json]

specification

  • list all files in "components" as components
  • list all files in "systems" as systems and assign write access to any imported components

files change

  • added packages/cli/src/commands/gen-deploy.ts

@yoyoismee yoyoismee requested a review from alvrs as a code owner December 13, 2022 17:21
const route = path.resolve(systems, e);
const out = spawnSync("forge",["flatten" ,route]);
const flatCode = out.stdout.toString()
const used = comps.filter(c => flatCode.includes(c))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw this would include components that the system doesn't actually write to, not that I have a better solution in mind

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regex the set function?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regex the set function?

That was my 1st thought actually, but set is called on the variable, which can have any name. And even if you account for that, a library may have some func which uses set, but the system doesnt use that func.
I've been trying some stuff with solidity-parser, but it's not rly gonna help here (imports make things very complicated)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can think of 2 ways.

  • try find calls in systems that hit any non-view/pure function on components. (might be hard tho)
  • add warning that this function might add more write access than actually required. user might wanna remove some manually

@dk1a dk1a Dec 14, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • For 1 you'd need to write some complicated parsing, imo not worth it for this little utility
  • I like 2, would prefer it for my project compared to filling writeAccess manually from scratch

Another option I could suggest: parse e.g. @custom:ecsc TestComponent; but neither forge inspect, nor solidity-parser can do that. So I guess you'd need solc's devdoc json output. Seems complicated, but still way easier than writing a function parser.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added warning msg :)

@yoyoismee yoyoismee requested a review from a user December 23, 2022 07:00
@dk1a

dk1a commented Jan 2, 2023

Copy link
Copy Markdown
Contributor

Just fyi #309 kinda breaks this. Not asking to change anything yet, #309 is a draft and its particular features are optional anyways

@alvrs

alvrs commented Jan 10, 2023

Copy link
Copy Markdown
Member

As discussed above this solution generates a deploy.json that gives more write access than necessary (eg. reading from a component will cause it to be added to the system's write access list). This script might still be useful for some, but to be included in MUD's default CLI we have to find a solution that generates a precise deploy.json. (Note: if you don't care about giving too many permissions, you can already put "*" into the deploy.json's write access list. Not a recommended practice of course.) Adding some conventions might help, adding "subsystems" as recommended way of splitting logic instead of libraries might help (see discussion in #327). I'll close this PR for now until we have a better foundation to autogenerate deploy.json.

@alvrs alvrs closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants