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

Fix #17374 if not present install npm #17375

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sbnsec
Copy link
Contributor

@sbnsec sbnsec commented Oct 31, 2018

No description provided.

@sbnsec
Copy link
Contributor Author

sbnsec commented Oct 31, 2018

see : #17374

@andresriancho
Copy link
Owner

Thanks for reporting the issue and sending the PR.

I prefer to see this implemented by adding the packages to CORE_SYSTEM_PACKAGES, like we see here:

https://github.com/andresriancho/w3af/blob/master/w3af/core/controllers/dependency_check/platforms/ubuntu1204.py#L34

Would that make sense to you?

@andresriancho andresriancho self-assigned this Nov 2, 2018
@sbnsec
Copy link
Contributor Author

sbnsec commented Nov 3, 2018

Thanks for your answer @andresriancho ,
Well that makes sense and at the beginning it seems cleaner but :

  • The npm pakage is not present on the system default repository,
    So the devs of npm are maintaining a script to ensure a clean install of the repos. this script is agnostic of the distrib/version.

As retriveJS is installed that way (in base_plateform)
I would guess was that it would be more intuitive to install npm the same way ( as npm is the only dependency of retriveJS )

If you are not agree with my point of view, What would you suggest to handle the repository problem ?

Thanks :).
Have a nice day.

@andresriancho
Copy link
Owner

At least in Ubuntu the npm package is there:

Or maybe I'm missing something? Are the versions in the ubuntu repositories too old?

@sbnsec
Copy link
Contributor Author

sbnsec commented Nov 14, 2018

Okay, true, I made a new PR. #17407

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.

3 participants