Skip to content

HACKING.md would be nice #305

Closed
Closed

Description

@GitMensch

I suppose to add a new file HACKING.md with notes how to setup a dev environment, that also specifies how to compile, lint, package and debug the debug extension; starting from "only vscode installed". Minor notes alone and links would be enough.

As the README.md is the main part of the "user view" I'd not add the information there, but in a new file.

@brownts As you had the most recent experience: Would you mind doing that?

Activity

  1. WebFreak001 commented on Nov 26, 2021

    @WebFreak001
    Owner

    @GitMensch a little unrelated to this issue, but I have added a github actions action: you can now tag releases to make them release on vscode marketplace, openvsx and on the releases tab.

    Make sure you bump the version of the package in package.json before releasing!

  2. brownts commented on Nov 26, 2021

    @brownts
    Collaborator

    Sure, I could take a pass at this.

    I did notice that the Travis CI was no longer working for running the unit tests. I was wondering if maybe that was something that would now be setup to run on Github Actions as well. That along with a linting would be good to run against pull requests or even the main branch.

    I'd be curious if there are any plans to expand the testing. I know it is time consuming and error prone to attempt to adhoc test many different permutations of launch/attach/gdb/lldb, etc., let alone different versions of these toolchains. I'd love if there were something more automated for this, but haven't really looked to see what might exist for this. Maybe you have some ideas here.

  3. WebFreak001 commented on Nov 26, 2021

    @WebFreak001
    Owner

    you can spawn and control vscode instances from unittests if you want to simulate a full vscode UI.

    But debug extensions implement the debug adapter protocol, so you could just simulate a debug app running this extension and validate which commands are sent/received. There are probably already libraries and applications for this out there on the internet.

  4. GitMensch commented on Nov 26, 2021

    @GitMensch
    CollaboratorAuthor

    I did notice that the Travis CI was no longer working for running the unit tests. I was wondering if maybe that was something that would now be setup to run on Github Actions as well. That along with a linting would be good to run against pull requests or even the main branch.

    Actually it would already be nice to have an action that would only do the linting part, adding tests would of course be much better - but the first may be much easier to add now and would at least make sure that the code compiles and passes linting.

    @brownts to make that usable current linting issues should be fixed, that may be something you could help with.

    I have added a github actions action: you can now tag releases to make them release on vscode marketplace, openvsx and on the releases tab

    @WebFreak001 Thanks, will likely do relative soon (also after updating the ChangeLog), possibly next week, and will update that and package.json for "unreleased" afterwards.
    As this extension currently doesn't follow semver rules (as far as I see each update normally increments the second number and we does not start at 1.0.0): Are you fine with one of

    • ".0" being dev versions and published releases starting from ".1"?
    • -0dev0 pre-release identifier (which would also allow incrementing that last part, either with each commit or from time to time)?

    or should we just leave at ".0" if no patched version, the tag matches the actual release the previous versions in git have the same number but aren't actually identical?

  5. WebFreak001 commented on Nov 26, 2021

    @WebFreak001
    Owner

    we do follow semver, a version starting with major version 0 means breaking changes may be introduced with the minor version

    Bump the minor version every time, no matter if there are breaking changes until we reach 1.0.0

    In 1.0.0 we declare full stability and no longer break stuff with updates

  6. GitMensch commented on Nov 26, 2021

    @GitMensch
    CollaboratorAuthor

    Sounds good. So what about a pre-release identifier for "not released" versions?

  7. WebFreak001 commented on Nov 26, 2021

    @WebFreak001
    Owner

    yeah you can suffix them with whatever you want, I usually go with -alpha.1, -alpha.2, ..., -beta.1, -beta.2, etc.

    But the marketplaces don't support that, they will just publish the extension to the users anyway - so I don't think there is much value in doing it for extensions.

  8. GitMensch commented on Mar 3, 2022

    @GitMensch
    CollaboratorAuthor

    The file should also contain a note about using fixed and older versions of vscode and dap for backward compatibility (for now, I'd say "roughly a year", which is what #315 updates, too).

  9. brownts commented on Mar 14, 2022

    @brownts
    Collaborator

    I started working on this today. I decided to start from a scratch VSCode/Ubuntu install. At least that is what I'm currently using, so figured that would be best to start with. Anyway, I noticed that when I tried to perform an npm install in the freshly cloned repository that I was getting a compilation error. Looking at this a bit more, it seemed that because the package-lock.json was out-of-date with the package.json (i.e., ssh2 had recently been upgraded to 1.6.0 in package.json in ac16c92, but not in package-lock.json), that npm installed version 1.7.0 and for some reason this causes a compilation failure on my system.

    ...
    
    > ssh2@1.7.0 install /config/git/junk/code-debug/node_modules/ssh2
    > node install.js
    
    make: Entering directory '/config/git/junk/code-debug/node_modules/ssh2/lib/protocol/crypto/build'
      CXX(target) Release/obj.target/sshcrypto/src/binding.o
    ../src/binding.cc: In member function 'void ChaChaPolyCipher::clear()':
    ../src/binding.cc:83:7: error: 'EVP_CIPHER_CTX_cleanup' was not declared in this scope; did you mean 'EVP_CIPHER_CTX_set_num'?
       83 |       EVP_CIPHER_CTX_cleanup(ctx_pktlen_);
          |       ^~~~~~~~~~~~~~~~~~~~~~
          |       EVP_CIPHER_CTX_set_num
    
    ...

    I started over and forced ssh2 to 1.6.0 (to also force updating of the package-lock.json file with ssh2@1.6.0 instead of ssh2@1.7.0) and this worked around the problem:

    npm install ssh2@1.6.0
    

    I'm not exactly sure what the correct way is to update the lock file, but I know that the current discrepancy between the package.json and package-lock.json seems to force this update to a newer version (i.e., 1.7.0) which then fails to build...at least for me.

    Also, when I indirectly update package-lock.json via npm install, it appears to downgrade the lockfileVersion from 2 down to 1. After some investigation, it appears that npm v7 is when this version changed. With just the basic sudo apt install npm on Ubuntu 20.04, it installs 6.14.16. Is there a minimum version of these tools that we should be targeting?

    I noticed that the build action targets Node.js 14.x, which is in alignment with the Ubuntu 20.04 standard installation version of Node.js. It just seems that when the package-lock.json was last generated, that it may have been generated with a 7.x version of npm rather than a 6.x version.

  10. GitMensch commented on Mar 14, 2022

    @GitMensch
    CollaboratorAuthor

    As I have no clue about npm or node.js in general (just typescripting a bit...) I've normally did npm update and found the old node.js 12 (or npm 6.14) not working with vscode last month so I've updated to node 16.14 / npm 8.5.2 and now compile, package and other things seem to work (all but debugging, not sure if that worked before).
    I think that an older format of package-lock should work with newer npm, too, but not necessarily the other way around - Please PR a working package-lock.json and I'll merge it.

    For the ssh2 error - please create an issue at https://github.com/mscdex/ssh2/issues/to get that fixed, so we can use newer versions also on older systems.

  11. GitMensch commented on Mar 14, 2022

    @GitMensch
    CollaboratorAuthor

    Note... not necessary to create an issue, it is already there at mscdex/ssh2#1147, and is also already solved by mscdex/ssh2@5af3bc6 - the only thing missing is an updated npm release...

  12. brownts commented on Mar 18, 2022

    @brownts
    Collaborator

    Thanks for doing the legwork in investigating the issue with the ssh2 package. I've not used npm, node.js or even typescript before a few months ago, so all of this is a bit new to me as well. I'm stumbling my way through it though. 😜

    I created #333 to synchronize the two files.

    I believe we can run npm clean-install and it will error out if the two are not in sync. That might be a good job to have setup to check potential PRs and commits to make sure the two files stay synchronized going forward.

  13. GitMensch commented on Mar 18, 2022

    @GitMensch
    CollaboratorAuthor

    For now that seems like a good thing to mention in HACKING.md, for later this can be added to a check workflow which is run in PRs and in each commit.

  14. brownts commented on Apr 2, 2022

    @brownts
    Collaborator

    I setup a Windows development environment today to test out Windows functionality of this extension. I ran into an issue and wanted to know if anyone else running under Windows has run into the same problem. When I tried to launch the extension under the debugger (it first tries to compile, due to the preLaunchTask in launch.json). However, this was having trouble running npm on my Windows system.

    > Executing task: C:\Program Files\nodejs\npm run compile --loglevel silent <
    
    The terminal process failed to launch: A native exception occurred during launch (Cannot create process, error code: 193).

    After some investigation, I discovered this was due to the fact that there are two different scripts (one for POSIX systems ...npm, and one for Windows...npm.cmd):

    c:\>where npm
    C:\Program Files\nodejs\npm
    C:\Program Files\nodejs\npm.cmd
    
    c:\>

    I also discovered that the default setting in the current task.json is to run the npm command as a process rather than through the shell. If I change the task type to shell, then everything works fine (the npm.cmd is properly resolved by the shell).

    Thus, it appears the following should be added to tasks.json so this works on both POSIX and Windows systems.

    "type": "shell"

    I can create a pull request for this, but was curious if anyone else had run into this as well.

  15. GitMensch commented on Apr 2, 2022

    @GitMensch
    CollaboratorAuthor

    Yes, please create a PR, I've stumbled on this before and adjusted the process to be npm.cmd - the shell solution looks much better.

  16. added this to the Backlog milestone on Apr 27, 2022
  17. GitMensch commented on May 3, 2022

    @GitMensch
    CollaboratorAuthor

    As I had that running today and wanted to document the "debugging this extension" issue I've started HACKING.md, also gathering all information I found "somewhere".
    This new file definitely needs some love, please feel free to improve it.

  18. brownts commented on May 28, 2022

    @brownts
    Collaborator

    I've made some significant updates to this file in cdb9975 which I believe now satisfies the original intent of this issue. I'm going to close this issue out as having been addressed.

  19. GitMensch commented on May 29, 2022

    @GitMensch
    CollaboratorAuthor

    That's marvelous! Thank you very much, this helps a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions