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

Dnsrecon fix #3940

Closed
wants to merge 1 commit into from
Closed

Dnsrecon fix #3940

wants to merge 1 commit into from

Conversation

PardesiCat
Copy link
Contributor

dnsrecon not working throwing errors

pcattestdnsrecon

but fixed ( you can check the fixed pkg + build log here )

https://git.pardesicat.xyz/pardesicat/dnsrecon-fix

problem missing depends in PKGBUILD

depends name on arch repo python-pytest flake8

@noraj
Copy link
Contributor

noraj commented Aug 21, 2023

@PardesiCat Thanks for the PR. However, I don't think this is the right way to fix it.

pytest and flake8 are dev dependencies (for test and linting) so they are not required at runtime.

This dependency error is happening because they are set as required in the package. setup.cfg load the whole requirements.txt

https://github.com/darkoperator/dnsrecon/blob/38b705febfe37490cc9bbee10640d41553635fe6/setup.cfg#L2

requirements.txt mixes runtime and dev dependencies:

https://github.com/darkoperator/dnsrecon/blob/master/requirements.txt

So the solution is better packaging upstream. Eg. a separate requirements.txt and requirements-dev.txt (cf link). There more options like extras_require, tests_require for stup.cfg. A better way would be to drop the obsolete setup.py anyway and migrate to pyproject.toml that offers more flexibility cf. https://python-poetry.org/docs/master/managing-dependencies/

@noraj noraj closed this Aug 21, 2023
@noraj
Copy link
Contributor

noraj commented Aug 21, 2023

@PardesiCat Could you PR the changes upstream please?

@noraj noraj added external::upstream-issue For issues that were created to track upstream issues type::depends related to dependencies labels Aug 21, 2023
@PardesiCat
Copy link
Contributor Author

well here also i tested in for now adding in makedepends=('git' 'python-setuptools' 'pytest' 'flake8' )

also doing the job so for now should we in BA USe in makedepends ?

@noraj
Copy link
Contributor

noraj commented Aug 21, 2023

Long term: No, because it's not a makedepends either. As a temporary workaround, I suggest doing all the following:

  1. Add python-pytest' 'flake8 in makedepends
  2. Create an upstream issue or PR
  3. Add a comment above the makedepends line of the PKGBUILD with the link to the issue or PR
  4. (We'll be able to track this and remove the dependencies once upstream is fixed)

@D3vil0p3r
Copy link
Contributor

@noraj @PardesiCat #3941

@D3vil0p3r
Copy link
Contributor

D3vil0p3r commented Aug 23, 2023

@noraj the source files seem to be fixed upstream. Should we remove the build deps from #3941 and leave it opened?

@noraj
Copy link
Contributor

noraj commented Aug 23, 2023

@D3vil0p3r We could close #3941 as it was fixed in darkoperator/dnsrecon@d0507ef. However we need to bump pkgrel to fetch the fix.

@D3vil0p3r
Copy link
Contributor

@D3vil0p3r We could close #3941 as it was fixed in darkoperator/dnsrecon@d0507ef. However we need to bump pkgrel to fetch the fix.

At this point should I keep the PR opened and bump only pkgrel? Or should I close it and @noptrix will bump pkgrel directly?

@noraj
Copy link
Contributor

noraj commented Aug 23, 2023

Done ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external::upstream-issue For issues that were created to track upstream issues type::depends related to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants