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

Give users full direct control over path mapping #56

Open
rulatir opened this issue Sep 29, 2020 · 9 comments
Open

Give users full direct control over path mapping #56

rulatir opened this issue Sep 29, 2020 · 9 comments

Comments

@rulatir
Copy link

rulatir commented Sep 29, 2020

Please admit that it is a completely unrealistic fantasy to believe that path mapping between vastly different environments (development environment with checked-out monorepo source tree vs. docker container with highly optimized directory layout, container-only .d -> .avail style symlinks etc. etc.) can be solved entirely automatically.

Users must be allowed to take full control in this area when necessary.

Please provide user configurable multi path mapping as a first-class feature in ZBS/MobDebug. Please do it instead of forcing users to report issues and corner cases time and again and again and again so you can continue to tweak your autoresolver further and further and further and further.

I just came back to working on our Lua codebase after two months, and I understand NOTHING of the hacks I was forced to employ back in July to work around the limitations in ZBS/MobDebug's automatic path resolution. The whole odyssey is logged in the latter comments in this issue. I did things like changing entries in LUA_PATH from absolute to relative, something that I shouldn't have to do. I arrived at an elusive understanding of what needs to be done and why after inspecting the debugger/debugge communication, inspecting the debuggee module source code, and going through several rounds of communication with you, none of which should be a prerequisite to being able to stop on breakpoints.

Give us control!

@rulatir rulatir changed the title Give users full control over path mapping Give users full direct control over path mapping Sep 29, 2020
@pkulchenko
Copy link
Owner

@rulatir, to make this more productive, is there a specific patch for MobDebug you are thinking about or want to contribute? The patch you referenced from the ZBS repository does partially solve the problem, but it's not a MobDebug patch, so it doesn't help other MobDebug users.

Also, I'm not sure what exactly you want me to do. You already have access to the patch and can easily apply it to your code if it solves the problem. As I already said, I don't think it's the right way to solve the issue, so I haven't merged it yet, but since you didn't provide details about your case, I can't describe a possible solution and evaluate it against other options.

The whole odyssey is logged in the latter comments in this issue. I did things like changing entries in LUA_PATH from absolute to relative, something that I shouldn't have to do.

I don't see why this is a hack any more than the direct mapping you are asking about, as you'd have to do the same thing in a different context.

I asked in one of the last messages in that thread "Does this cover all your use cases?" to which you didn't respond with any follow-up issues. I also asked to show the result of running debug.getinfo(2, "S").source? several times, but don't see it being provided. I'd still be interested in that, especially from those source files that are not being mapped correctly.

All the changes we discussed were applied and merged promptly and I thought I got all your questions answered.

I'd rather focus on identifying the use case that is not being handled and discussing options to handle it. One of the scenarios I can think about is the case of two different codebases that have different paths in the container (the app being debugged) that are represented inside one source tree in the IDE. Still, there may still be options to deal with it in the current implementation.

@rulatir
Copy link
Author

rulatir commented Sep 30, 2020

@rulatir, to make this more productive, is there a specific patch for MobDebug you are thinking about or want to contribute? The patch you referenced from the ZBS repository does partially solve the problem, but it's not a MobDebug patch, so it doesn't help other MobDebug users.

I keep getting confused as to which part of the path mapping is handled by which bit. Ceterum censeo it should be handled exclusively on the IDE end, and splitting the responsibility was an architectural mistake that will keep snarling and biting everyone who approaches.

I don't see why this is a hack any more than the direct mapping you are asking about, as you'd have to do the same thing in a different context.

With explicit mapping I wouldn't have to use relative paths.

I asked in one of the last messages in that thread "Does this cover all your use cases?"

This is precisely the wrong question. I assert that direct control over path mapping will cover all of my use cases. With automatic mapping, I can never be sure of that. I am not a Turing oracle, I cannot know that any kind of smart logic written in a Turing-complete language will cover all my current and future use cases. In stark contrast to "identifying the use case that is not being handled and discussing options to handle it" in a never ending and SLOW cycle, a simple explicit ordered prefix-to-prefix map can provably express any path mapping, can be implemented in a way that is provably correct, and does the job once and for good.

I understand your attachment to the automatic mapping logic you've put countless hours into over many years, but please at least consider that you might be falling oaf to the sunk cost fallacy. The time you've spent exchanging debug info dumps, patches, questions, answers and hypotheses with users whose edge cases aren't handled, the time you've spent coding and testing fixes, the time you've spent pondering whether a particular fix should be merged or left for users to patch their installations if they want, the time you've spent debugging the same edge case and developing the same patch again for/with a different user (because you probably don't keep a knowledge base of edge cases and patches that fix them) - you aren't getting any of that time back and neither are those users. But if you extrapolate, you will see that you have a choice: you can keep incurring those losses in the future, or you can magically turn all that future lost time into free time for you and for those users. All you need to do is yield control to the users, and empower them to tell your software what to do.

@britzl
Copy link

britzl commented Aug 27, 2022

I admit I have not followed along fully, especially within that ZBS rabbit hole of an issue which is linked above, but control over the matching of breakpoints to file and chunknames might actually be helpful.

Example: We use mobdebug on phones, consoles and pcs running either Lua 5.1 or LuaJIT, sometimes running code from source files and other times from bytecode. Our editor with its integrated debugger on the other hand is running on macOS, Windows or Linux and it's a hassle to make sure the paths when setting breakpoints (SETB) match with what is passed to has_breakpoint() while mobdebug is running its debug hook. Additional complexity arises when filenames are longer than 60 characters.

We're currently patching mobdebug so that names are standardized in set_breakpoint(), remove_breakpoint() and has_breakpoint(). We can keep maintaining a patch, but perhaps mobdebug could let us override a function to "massage" the filename before use by above mentioned functions?

Let me know if I should open a new issue or submit a PR with what I have in mind instead of piggybacking on this issue!

@pkulchenko
Copy link
Owner

@britzl, thank you for the note. Do you mind providing additional details about the exact paths and the "massaging" you are applying?

@britzl
Copy link

britzl commented Aug 27, 2022

@pkulchenko we have a couple of different builders for the Lua code that ends up in the runtime and this results in file/chunknames like this =game.foo and at other times like this @game/foo.lua.

We're in control of this process and we can (and probably should) solve this in our build pipeline to guarantee standardized names at runtime rather than trying to fix names at runtime.

@rulatir
Copy link
Author

rulatir commented Aug 29, 2022

@britzl, I am the original reporter of this issue. I have since switched to the EmmyLua debuggee module and it was a life changer. It is a native module that works with the EmmyLua debugger, which has versions for major IDEs.

Tutorial. Caveat: the "IDE connect debugger" mode didn't work for me, but the "Debugger connect IDE" mode did, which is a minor change from the tutorial.

Needless to say, the Emmy module implements this feature request: it does give you full manual control of the path mapping in the most general way possible, via a callback that you implement. And this is how it should be done.

@pkulchenko
Copy link
Owner

@rulatir, thank you for sharing your experience. EmmyLua does this by using emmy.fixPath method that maps one path to another.

@britzl, there is a patch with a similar intent that applies this logic on the controller side (in ZeroBrane Studio): pkulchenko/ZeroBraneStudio#849. Do you think adding debugger.mappath function to map the paths to the debugger itself is sufficient for your needs?

@rulatir
Copy link
Author

rulatir commented Aug 29, 2022

@pkulchenko, as you might remember, in principle I am in favor of handling path mapping in the controller aka debugging client aka IDE, and keeping the debuggee module absolutely minimal. However if I can put the path mapping implementation in a separate module and link that module into the container, it's as good as configuring the mapping on the debugger side. With Emmy, I implemented fixPath so that it reads a JSON config file with rewrite rules, and I link that file from the host.

@britzl
Copy link

britzl commented Aug 30, 2022

@pkulchenko and @rulatir sorry for necroing this old issue, but perhaps some good will come out of it.

In my case I would be more helped by a fixPath(path) function as mentioned by @rulatir and also seen in the blog post shared above (https://dev.to/omervk/debugging-lua-inside-openresty-inside-docker-with-intellij-idea-2h95).

The path remapping would not help as I'm looking to fix inconsistencies in the way we set the file/chunkname. Think along the lines of =game/foo.lua vs @game.foo and that sort of thing (ie / vs . and with and without file extension). We've bit the bullet and cleaned up our build pipeline, runtime and editor/debugger to get consistent filenames:

defold/defold#6920

(The PR currently contains fixes for our builds and runtime and we'll add the fixes for the editor debugger soon)

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

No branches or pull requests

3 participants