Skip to content

Refactor @stdlib/blas/base/dcopy #792

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jan 17, 2023
Merged

Refactor @stdlib/blas/base/dcopy #792

merged 13 commits into from
Jan 17, 2023

Conversation

Pranavchiku
Copy link
Member

@Pranavchiku Pranavchiku commented Jan 3, 2023 •
edited
Loading

Checklist

  • update readme.md
  • docs
  • include/stdlib/math/base/special/ ( No such change visible )
  • src
  • include.gypi
  • manifest.json
  • package.json
  • lib
  • examples
  • benchmark
  • test

I will keep on populating the checklist as and when necessary.

@kgryte

@Pranavchiku
Copy link
Member Author

I am facing some error while pushing docs will have a look asap.

@Pranavchiku
Copy link
Member Author

I am receiving the following error while generating native addons, using make install-node-addons NODE_ADDONS_PATTERN=blas/base/dcopy

gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
make[1]: Entering directory '/home/pranavchiku/stdlib/lib/node_modules/@stdlib/blas/base/dcopy/build'
  RULE binding_gyp_addon_target_compile_fortran_linux_0 Release/obj.target/addon/geni/dcopy.o
  CC(target) Release/obj.target/addon/src/addon.o
../src/addon.c:29:1: error: unknown type name ‘namespace’
   29 | namespace stdlib_blas_base_dcopy {
      | ^~~~~~~~~
../src/addon.c:29:34: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘{’ token
   29 | namespace stdlib_blas_base_dcopy {
      |                                  ^
make[1]: *** [addon.target.mk:171: Release/obj.target/addon/src/addon.o] Error 1
make[1]: Leaving directory '/home/pranavchiku/stdlib/lib/node_modules/@stdlib/blas/base/dcopy/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/pranavchiku/stdlib/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:314:20)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12)
gyp ERR! System Linux 5.15.0-56-generic
gyp ERR! command "/usr/bin/node" "/home/pranavchiku/stdlib/node_modules/.bin/node-gyp" "rebuild"
gyp ERR! cwd /home/pranavchiku/stdlib/lib/node_modules/@stdlib/blas/base/dcopy
gyp ERR! node -v v12.22.9
gyp ERR! node-gyp -v v6.1.0
gyp ERR! not ok 
Error: failed to build add-on: /home/pranavchiku/stdlib/lib/node_modules/@stdlib/blas/base/dcopy

@Pranavchiku
Copy link
Member Author

I am not sure why it is occurring, @kgryte if you can have a look?

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

You haven't refactored addon.c. C does not have namespaces. Namespaces are available in C++.

@Pranavchiku
Copy link
Member Author

Ohh yes, sorry for that. On it.

@Pranavchiku
Copy link
Member Author

When I try to commit changes of index.d.ts and repl.txt I receive the following log,

make[1]: Entering directory '/home/pranavchiku/stdlib'

Linting file: lib/node_modules/@stdlib/blas/base/dcopy/docs/types/index.d.ts
Error: Cannot find module '/home/pranavchiku/.dts/typescript-installs/4.2/node_modules/typescript'
Require stack:
- /home/pranavchiku/stdlib/node_modules/dtslint/bin/lint.js
- /home/pranavchiku/stdlib/node_modules/dtslint/bin/index.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at testDependencies (/home/pranavchiku/stdlib/node_modules/dtslint/bin/lint.js:64:16)
    at Object.<anonymous> (/home/pranavchiku/stdlib/node_modules/dtslint/bin/lint.js:26:28)
    at Generator.next (<anonymous>)
    at /home/pranavchiku/stdlib/node_modules/dtslint/bin/lint.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/home/pranavchiku/stdlib/node_modules/dtslint/bin/lint.js:4:12)
make[1]: *** [/home/pranavchiku/stdlib/tools/make/lib/lint/typescript/dtslint.mk:110: dtslint-files] Error 1
make[1]: Leaving directory '/home/pranavchiku/stdlib'
make: *** [/home/pranavchiku/stdlib/tools/make/lib/lint/typescript/Makefile:178: lint-typescript-declarations-files] Error 2

TypeScript declaration file lint errors.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

Try deleting your node modules folder and doing a fresh make install-node-modules && make init.

@Pranavchiku
Copy link
Member Author

Nope, I still receive same error.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

Are you able to confirm that the .dts folder exists, as indicated in the error message?

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Jan 6, 2023 •
edited
Loading

It does not exist.
Shouldn't it be there after make init ?

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

No. dtslint should automatically create it. What version of dtslint do you have installed?

@Pranavchiku
Copy link
Member Author

I guess I don't have dtslint installed.
If I do

$ dtslint --version
Command 'dtslint' not found, did you mean:
  command 'dtblint' from deb dt-utils (2021.03.0+ds-2)
Try: sudo apt install <deb name>

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

No, it would be in your node_modules folder.

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Jan 6, 2023 •
edited
Loading

I am not sure about version, dtslint is present in node_modules and I recently did npm install --save-dev dtslint by looking at readme.

Edit: dtslint@3.7.0

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

Okay, that is the same version as me, and mine currently works, so not sure why your installation is throwing the error above.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

What version of Node.js are you running?

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Jan 6, 2023 •
edited
Loading

What version of Node.js are you running?

v12.22.9

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

Try upgrading to at least v16 (e.g., nvm install 16).

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

You can also try to lint the files directly via

$ make lint-typescript-declarations TYPESCRIPT_DECLARATIONS_FILTER=.*/blas/base/dcopy/.*

@Pranavchiku
Copy link
Member Author

Try upgrading to at least v16 (e.g., nvm install 16).

I did this, now trying make install-node-modules && make init let's hope it works this time.

@Pranavchiku
Copy link
Member Author

Still facing same issue. I guess .dts. folder is not present in my device, not sure why.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

A bit odd that you wouldn't have encountered this error before.

@Pranavchiku
Copy link
Member Author

Yeah, I didn't encounter updating docs in past PRs although I did those in my early PRs which had different Ubuntu settings as I did a clean installation in month of december.

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Jan 6, 2023 •
edited
Loading

Apart from index.d.ts
I tried to make the necessary changes, and all tests and benchmarks are cleared.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

Maybe you have a permissions issue. You can try manually creating a .dts folder in your home directory with suitable permissions.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

You can also try doing some console.logging in your local node_modules folder in the package responsible for installing TypeScript versions: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/utils/src/typescript-installer.ts

It's possible that errors are being swallowed regarding permissions, etc.

@Pranavchiku
Copy link
Member Author

I have the folder installed, it does not have node_modules in the path, /home/pranavchiku/.dts/typescript-installs/4.2/node_modules/typescript.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

In short, I don't have an easy answer atm, apart from doing a bit of digging and debugging on your end.

@Pranavchiku
Copy link
Member Author

No worries, I will try to get it done asap.

@kgryte
Copy link
Member

kgryte commented Jan 6, 2023

My initial sense is to start putting console log calls in the code actually responsible for the installations. Mucking about with the .dts folder is likely not where the problem is.

@Pranavchiku Pranavchiku marked this pull request as ready for review January 17, 2023 04:26
@Pranavchiku
Copy link
Member Author

Pranavchiku commented Jan 17, 2023 •
edited
Loading

All tests and benchmarks cleared. This PR can have a review.

Apologies for taking so long to complete this PR.

@kgryte kgryte added Native Addons Issue involves or relates to Node.js native add-ons. BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). labels Jan 17, 2023
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte reviewed Jan 17, 2023
View reviewed changes
kgryte
kgryte approved these changes Jan 17, 2023
View reviewed changes
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @Pranavchiku!

@kgryte kgryte merged commit ffdc1d2 into stdlib-js:develop Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgryte kgryte kgryte approved these changes

Assignees
No one assigned
Labels
BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Native Addons Issue involves or relates to Node.js native add-ons.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Pranavchiku @kgryte