Pass PTVSD folder name to the new debug adapter executable

[kimadeline]

For #7001

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Has a news entry file (remember to thank yourself!)
• Appropriate comments and documentation strings in the code
• Has sufficient logging.
• Has telemetry for enhancements.
• Unit tests & system/integration tests are added/updated
• Test plan is updated as appropriate
• package-lock.json has been regenerated by running npm install (if dependencies have changed)
• The wiki is updated with any design decisions/details.

[DonJayamanne]

How about we get the ptvsd launcher to call the ptvsd-folder script. That's it's one less process that's started for every debug session, hence faster launching the debugger, etc .. (I.e move the code into the same Python process)

[kimadeline]

So ptvsd_launcher.py would look like this?

if '--experiment' flag:
   # import get_folder_name from ptvsd_folder_name.py first
   folder_name = get_folder_name()
   ptvsd = import folder_name/ptvsd/adapter/__main__.py
   ptvsd.main(ptvsd_args) 
else:
   launch_old_ptvsd() # the old code

Since the folder name contains periods (for example ptvsd-5.0.0a3-cp37-cp37m-macosx_10_13_x86_64) I can't seem to be able to use import(folder_name) or importlib.import_module(folder_name). So far I am using imp.load_source locally, however this displays a warning (doesn't prevent the code from running though):

DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses

There's also this snippet in adapter/__main__.py that makes it sound like when executing the file there is some setup & arg parsing first (it parses sys.argv but we have our ptvsd_launcher args in it), which we should then do if we call main() ourselves in ptvsd_launcher.py:

if __name__ == "__main__":
    # ptvsd can also be invoked directly rather than via -m. In this case, the first
    # entry on sys.path is the one added automatically by Python for the directory
    # containing this file. This means that import ptvsd will not work, since we need
    # the parent directory of ptvsd/ to be in sys.path, rather than ptvsd/adapter/.
    #
    # The other issue is that many other absolute imports will break, because they
    # will be resolved relative to ptvsd/adapter/ - e.g. `import state` will then try
    # to import ptvsd/adapter/state.py.
    #
    # To fix both, we need to replace the automatically added entry such that it points
    # at parent directory of ptvsd/ instead of ptvsd/adapter, import ptvsd with that
    # in sys.path, and then remove the first entry entry altogether, so that it doesn't
    # affect any further imports we might do. For example, suppose the user did:
    #
    #   python /foo/bar/ptvsd/adapter ...
    #
    # At the beginning of this script, sys.path will contain "/foo/bar/ptvsd/adapter"
    # as the first entry. What we want is to replace it with "/foo/bar', then import
    # ptvsd with that in effect, and then remove the replaced entry before any more
    # code runs. The imported ptvsd module will remain in sys.modules, and thus all
    # future imports of it or its submodules will resolve accordingly.
    if "ptvsd" not in sys.modules:
        # Do not use dirname() to walk up - this can be a relative path, e.g. ".".
        sys.path[0] = sys.path[0] + "/../../"
        __import__("ptvsd")
        del sys.path[0]

    # Load locale settings.
    locale.setlocale(locale.LC_ALL, "")

    main(_parse_argv())

Using the imp package & cleaning up sys.argv manually (sys.argv.pop(1)) feels icky, is that the correct way to proceed?

(i could also replace the .s in the folder name with - or _ and call it a day...)

[kimadeline]

@ericsnowcurrently suggested not using ptvsd_launcher.py to call the ptvsd-folder script, but instead call it only once and cache the result.

[DonJayamanne]

My concern was two fold, we're exposing unnecessary information into another layer, and unnecessary perf cost.
Fetching the information in the python process is next to negligable. Now running a separate python process, caching, etc introduces more moving parts and more room for error.

I.e. launcher script isn't self contained.

More problems of exposing this in other layers is the fact that we now have to modify the code in launcherProvider.ts to pass this path to ptvsd. I.e. were modifying these places to pass additional arguments and do more stuff. Doesn't feel right to keep changing such code. I.e. modifying to keep same functionality.

If going down this path then we need to make more changes to get the path to ptvsd passed in other locations, etc... Feels like a lot of work (changes & leaking of concerns) when it can be done in the launcher.

[DonJayamanne]

Please modify launchProvider.ts as well to pass this info in there.
More unnecessary changes in other places that could be avoided if it was done in one python file with just tests for one python file (lot less changes).

[kimadeline]

After discussing with @ericsnowcurrently , will be addressed in #7549.

[ericsnowcurrently]

Mostly LGTM. I've left comments where I think it's worth looking at changing something, though not all comments are for things I expect to change. I didn't find anything wrong (other than missing Python tests), but did have a number of questions and observations. I'd be glad to talk over any of it. :)

Also, for the Python script I've noted shorter names that are more typical. Keep in mind that underscores are mostly used only in constants and function names, and not in variables.

Note that Python tests go under pythonFiles/tests.

[ericsnowcurrently]

FWIW, I don't see the value of all this extra stuff in the doc comment. The info is represented in the code.

[kimadeline]

What do you mean by extra stuff, the jsdoc params or the description?

[ericsnowcurrently]

jsdoc params

[DonJayamanne]

Please address changes (modify the other ts file) mentioned in #7152 (comment)

[ericsnowcurrently]

Mostly LGTM. I have a number of comments for you to consider, but nothing critical. I'd be glad to talk about any of it with you. :)

Also, thanks for adding both unit tests and functional tests!

[ericsnowcurrently]

LGTM

I've left a few comments, but feel free to do what you will with them. I'll mark this as approved.

Will address remote debugging in #7549