Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev endreport and app info #204

Closed
wants to merge 4 commits into from

Conversation

madeddy
Copy link
Contributor

@madeddy madeddy commented Mar 15, 2024

See #189

Add/update app infos

  • add dunders for central info access
  • add version to argparse
  • change exception in py-version check

Refactoring of result system and endreport

  • adds mp.manager for data syncing
  • result counting in single dict
  • more result types
  • adds custom exception for wrong rpyc header

Outstanding:

  • Mock upgrade (Fun-fact: multiprocessing is since last spring in renpy included, but not in v8.0 - such luck!)
  • Results of deobfucate.py falls still through the counter system.(same as before this pull req)

Would be nice if you could take also a look at the outstanding issues. Could be a bit finicky where to put the result counters because of deobfuscate.py.

madeddy added 2 commits March 17, 2024 11:53
- add dunders for central info access
- add version to argparse
- change exception in py-version check
- adds mp.manager for data syncing
- result counting in single dict
- more result types
- adds custom exception for wrong rpyc header
@madeddy madeddy force-pushed the dev_report_and_info branch from 8f59e32 to 8b3948e Compare March 17, 2024 10:57
@madeddy
Copy link
Contributor Author

madeddy commented Mar 26, 2024

Did you let this uncommented because of the open issue about garbled print outputs which could make this draft a moot point? Or do i interpret this wrongly and its just no time or something?

@CensoredUsername
Copy link
Owner

Just didn't have time to look at it yet.

@CensoredUsername
Copy link
Owner

I've thought about this one some more. I think I'm just not a fan of the multiprocessing.Manager approach personally, as it makes the code very annoying to use as a module. I still would prefer going to a direction where we pass a Context object to each worker process, which has:

  • A list of warning messages printed out
  • a return status (either an exception, or a result).
  • possibly a dict of extra attributes that could be set.

Which can then be printed out in the main thread and evaluated.

This would then replace printlock being passed around all the decompilers, which has always been quite the kludge to begin with.

@madeddy
Copy link
Contributor Author

madeddy commented Apr 4, 2024

multiprocessing.Manager approach ... makes the code very annoying to use as a module

I wasn't aware it complicates things and i still don't see how. But ok.

where we pass a Context object to each worker

Um... do you mean a "custom context manager"? Where would this be hooked in? Or some use of "contextvars"? I've never seen anything done with them so I'm blank on this.

As i read your issue about the interleaved output i tried a few things with logging, but it doesn't work.

  • I tried it with Queuehandler/-listener and it worked but the output is also garbled.
  • I tried a second logger inside MP with a Memory-/BufferedHandler but this one wouldn't add his logs to the main logger after MP
  • The basic mp.logger works but is a bit restricted and its also garbled.
  • Some other variant collided with the shared state of the Manager...

Frustrating.
Context sounds interesting after my other fails. I hope i understand how this works. Could you please elaborate on the theme and your idea? Or a good source?

@CensoredUsername
Copy link
Owner

Um... do you mean a "custom context manager"? Where would this be hooked in? Or some use of "contextvars"? I've never seen anything done with them so I'm blank on this.

Nah, just an object that represents the Context that the file is being decompiled in. It's just a very common programming term / design pattern.

But by doing that all multiprocessing related logic can stay within unrpyc.py, and the rest of the decompiler won't have to worry about it (not even about anything like printlock). They all get their own context object, and then unrpyc.py will collect them again after the decompilation is done. Multiprocessing is very annoying and has a lot of platform-dependent edge cases, so usually the best way to deal with it is just minimizing how much you need it.

@madeddy
Copy link
Contributor Author

madeddy commented Apr 18, 2024

I close this PR in favour of Context in branch 49f1de0. Some of the report code here will there be integrated.

@madeddy madeddy closed this Apr 18, 2024
@madeddy madeddy deleted the dev_report_and_info branch April 27, 2024 20:17
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.

2 participants