Adjusted to build package in own directory

[Nilsonfsilva]

Hi,
I'm a maintainer for sherlock on debian/kali/Ubuntu.

A bug was recently reported in Sherlock:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1071007

Where sherlok was installing his modules in the packages rais directory,
which is not a good practice and was still causing conflict with other people
package because of the ini.py file

After changing the pyprojetc.toml file, Sherlock began to be installed directly in its own directory.

Hope this helps!

[ppfeister]

Hey @Nilsonfsilva!
Good to see ya over here. I left a note on your bug maybe a week or so ago, just a slow roll out.

Deb 1071007 will be fixed by #2127, which I personally hope to have merged sometime over the next couple of days. Worst case scenario, if something comes up in CR, we also have #2133 on standby as an interim fix which can be merged more readily (just not ideal).

Your PR makes fairly different changes to L44-45/46 compared to those in the standby proposed by @mjsir911. It also lacks the L6 change. I'm not saying that either is correct or incorrect, but I'd like to hear if you have any feedback as to why one solution may be preferable over the other?

(As it stands now, I believe that we had some issues with the linked PR, so your backport is the most likely interim fix if you have no additional comments)

[mjsir911]

Hey, It's mostly a matter of prefetence, but the way i've done it with packages= is in my opinion more expkicit than hooking into setuptools' autofind feature. I'm not exactly sure what you're doing with including the resources subdirectory explicitly. How does that change the resulting install / how can i test for it to integrate a fix into my PR? Thanks for your work, -- M May 31, 2024 10:22:32 PM Paul Pfeister ***@***.***>:

…

Hey @Nilsonfsilva[https://github.com/Nilsonfsilva]! Good to see ya over here. I left a note on your bug maybe a week or so ago, just a slow roll out. Deb 1071007 will be fixed by #2127[#2127], which I personally /*hope*/ to have merged sometime over the next couple of days. Worst case scenario, if something comes up in CR, we also have #2133[#2133] on standby as an interim fix which can be merged more readily (just not ideal). Your PR makes fairly different changes to L44-45/46 compared to those in the standby proposed by @mjsir911[https://github.com/mjsir911]. It also lacks the L6 change. I'm not saying that either is correct or incorrect, but I ask, do you have any feedback as to why, from your testing, that one may be the better solution over the other? — Reply to this email directly, view it on GitHub[#2147 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AEDDZQSK6WM45T2JUQDLXDLZFFLBFAVCNFSM6AAAAABITZORPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGMYDCOBRGM]. You are receiving this because you were mentioned. [Tracking image][https://github.com/notifications/beacon/AEDDZQX5SSWMNYQ4OGP4B3TZFFLBFA5CNFSM6AAAAABITZORPWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT7YAYLK.gif]

[ppfeister]

Appreciate the rationale. Makes sense to me.

/resources/ isn't required for ~normal~ use, but it's required for use of the --local flag. When this flag is used, it'll use the data.json file from that directory rather than pulling from remote. More useful for testing/devel, or if there's some sort of upstream issue.

If /resources/ isn't packaged, --local is expected to fail.

[Nilsonfsilva]

Hi guys!
Firstly, thank you everyone for the discussion.

I'm not an expert, but let me clarify some points that I think are pertinent:

1. The purpose of this patch is not to make it an integral part of Sherlock, but rather to resolve an RC Bug in the project's repositories.

2. Feel free to accept it or not. My goal is to close the RC Bug, because if not, Sherlock will leave the "Testing version" of the distribution.

3. I only made a pull request because due to Debian's packaging policies, situations like this, it is recommended to communicate it upstream, either through "issues" or through PR. I chose PR so that DD Samuel wouldn't question it, since the Bug has already been reopened twice by him.

Let's wait for Samuel to review it before uploading the update.
In the same way, PR that will change the way of construction with Poetry.

Anything and just contact me.

Strong hug.

[ppfeister]

pip install .
sherlock --version

seems to have issues on my end, similar to the other PR.

Traceback (most recent call last):
  File "/home/{user}/.local/bin/sherlock", line 5, in <module>
    from sherlock import main
ImportError: cannot import name 'main' from 'sherlock' (/home/{user}/.local/lib/python3.12/site-packages/sherlock/__init__.py)

Did you encounter anything on your end while testing?

Could be a local issue, but it's with freshly built eggs and all.

---

Also...

I only made a pull request because due to Debian's packaging policies, situations like this, it is recommended to communicate it upstream

Upstreaming discovered bugs and bug fixes are always appreciated!

[Nilsonfsilva]

I solved this problem with this wrapper here:
When I build the package, point to where the module was installed.

https://salsa.debian.org/pkg-security-team/sherlock/-/blob/master/debian/sherlock.py?ref_type=heads

Here are the construction rules:

https://salsa.debian.org/pkg-security-team/sherlock/-/blob/master/debian/rules?ref_type=heads#L11-14

But this is for the debian environment.

[ppfeister]

Hm. If it's not platform agnostic, don't think we can merge it here -- that would break everyone who's not running deb & derivs. Lil bit of a problem.

If I have time later, I'll poke around your rules & patches and see if I can get it working more uniformly. With everyone being busy I'm not 100% on if it'll make it in before Poetry or not, but it very well may

Regarding your comments on 1071007, feel free to shoot me a message if you'd like to work on some sandboxing for poetry+tox. Should port pretty well from what (little) testing I've done on my end with deb

[Nilsonfsilva]

I feel very flattered by the invitation. But as I maintain many packages in Debian. I don't know if I could help effectively.

But I agree that I can help when it comes to testing in the Debian environment before the merge.

count on me.

[samueloph]

Discussions around the issue on the Debian side are happening at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1071007

I do not believe this PR should have been submitted to upstream, it's a tentative workaround on the Debian side while you already have a proper fix pending as pointed out by #2147 (comment).

Sorry about the noise. Feel free to review the linked PRs at you own pace (don't feel pressured due to the discussions on the Debian side).

[ppfeister]

@Nilsonfsilva more of an offer to help with your package rather than requesting your help with ours -- if you need a hand with portability or anything, feel free to let me know!

@samueloph Don't worry about it! We're appreciative of the PR, actually. We were in discussions about merging it as an interim fix while we got to more properly testing #2127. Only reason why we haven't merged this one is because in local testing it seems to introduce import errors, and we were unable to run it in the current state. Likely due to new relative paths caused by the new pyproject, which would be a pretty easy fix if so. I'll probably have more info for y'all shortly.