Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-10 Thread Samuel Henrique
Hello Thomas,

On Mon, 10 Jun 2024 at 12:03, Thomas Goirand  wrote:
>
> On 6/9/24 18:47, Samuel Henrique wrote:
> > Zigo,
> >> I just saw that sherlock (the social networks package) moved its python
> >> files to /usr/share, instead of /usr/lib/python3/dist-packages
> >> . This was
> >> the sensible thing to do, as it doesn't really need to expose itself as
> >> Python module.
> >
> > Not really, that was done by accident when Nilson was trying to remove the
> > system-wide init file (#1071007) and was reverted already.
> >
> > Upstream has mentioned (to me) that their intention is to provide a library 
> > for
> > sherlock, as we've had since the package was introduced.
>
> Well, sherlock is an app, and therefore, it's the sensible thing to do
> to push it's Python code in /usr/share. IMO, it shouldn't have been
> reverted.

The move was done as a workaround for another issue (the global __init__ file),
and upstream mentioned they would like to have the module available, so at the
time the revert was the right choice.

Now that we know there's also a clash with the sherlock package, the situation
is different.

> Normally, the one that owns the PyPi name such as:
> https://pypi.org/project/sherlock/
>
> also get to have the python module name. Clearly, sherlock (the social
> media package) didn't do that.

I agree with this.

> Now, if you know upstream, then probably you can convince them to rename
> their lib to something that doesn't clash? And also, maybe, add its
> software on PyPi?

Sherlock is there, but under https://pypi.org/project/sherlock-project.

So it does look like the right thing to do on Debian's side is to either not
let the module be importable or rename it (ideally done by upstream first).

Paul (as upstream), the sherlock module clashes with
https://pypi.org/project/sherlock/, which was published first and ships a
"sherlock" library.

Do you think it would make sense to change the name of the module provided by
your sherlock (just the module, not the executable), or alternatively for us to
not ship an importable library at all?

This is an issue that's going to happen on other distros as well, as far as I
know.

> >> Therefore, this bug can be closed, and there's IMO nothing more to do in
> >> the python-sherlock (the cluster lock package), as the conflict is now
> >> solved.
> >
> > I'll reopen 1072733 since the clash still exists.
>
> :(

We have a path forward now, at least after Paul replies, so it should be all
good (although the bug stays open until solved, right?!).

Cheers,

--
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-09 Thread Samuel Henrique
Control: reopen 1072733 =

Hello all,

Chris,
> > I see your point now, it seems like it should be just "Conflicts", do you
> > agree? None of those 2 packages can/should be renamed.
>
> Please see https://www.debian.org/doc/debian-policy/ch-files.html#binaries
>
> Conflicts is not appropriate for programs with different
> functionality.

That link is for binaries, whereas we are dealing with conflicting libraries,
the section just below that one does not say anything about libraries with
conflicting names.

It sounds like you're saying this also applies to libraries and that one of
them needs to be renamed or be dropped. Can you please be specific in what you
think it should happen (if not that)?

Zigo,
> I just saw that sherlock (the social networks package) moved its python
> files to /usr/share, instead of /usr/lib/python3/dist-packages
> . This was
> the sensible thing to do, as it doesn't really need to expose itself as
> Python module.

Not really, that was done by accident when Nilson was trying to remove the
system-wide init file (#1071007) and was reverted already.

Upstream has mentioned (to me) that their intention is to provide a library for
sherlock, as we've had since the package was introduced.

> Therefore, this bug can be closed, and there's IMO nothing more to do in
> the python-sherlock (the cluster lock package), as the conflict is now
> solved.

I'll reopen 1072733 since the clash still exists.


Cheers,


--
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-09 Thread Chris Hofstaedtler
* Samuel Henrique  [240608 20:37]:
> > If they share a name but none of the API / features, then it is not
> > a correct solution.
> 
> They do not share the same API.
> 
> > These descriptions do not sound related at all. In this case,
> > Conflicts/Replaces is not appropriate.
> 
> I see your point now, it seems like it should be just "Conflicts", do you
> agree? None of those 2 packages can/should be renamed.

Please see https://www.debian.org/doc/debian-policy/ch-files.html#binaries

Conflicts is not appropriate for programs with different
functionality.

Chris



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-08 Thread Samuel Henrique
Hello Chris,

> > > Per Debian policy this is not the correct solution for naming conflicts. 
> > > Both
> > > maintainer (teams), please find a policy compliant solution together.
> >
> > The solution for this one seems correct, it's a Conflict + Replaces because
> > both packages provide a "sherlock" library. Am I missing something?
>
> Do both packages provide the same API? IOW: do they provide the same
> "type" of library?
> If so, then Conficts/Replaces may be appropriate.
>
> If they share a name but none of the API / features, then it is not
> a correct solution.

They do not share the same API.

> These descriptions do not sound related at all. In this case,
> Conflicts/Replaces is not appropriate.

I see your point now, it seems like it should be just "Conflicts", do you
agree? None of those 2 packages can/should be renamed.

Cheers,

--
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-08 Thread Chris Hofstaedtler
Hi Samuel,

only replying to your question below, with new questions.

* Samuel Henrique  [240608 14:04]:
> For #1072733:
> 
> Chris
> > Per Debian policy this is not the correct solution for naming conflicts. 
> > Both
> > maintainer (teams), please find a policy compliant solution together.
> 
> The solution for this one seems correct, it's a Conflict + Replaces because
> both packages provide a "sherlock" library. Am I missing something?

Do both packages provide the same API? IOW: do they provide the same
"type" of library?
If so, then Conficts/Replaces may be appropriate.

If they share a name but none of the API / features, then it is not
a correct solution. 

Looking at the package descriptions on tracker.d.o:

python-sherlock:
  distributed inter-process locks with a choice of backend

sherlock:
  Find usernames across social networks

These descriptions do not sound related at all. In this case,
Conflicts/Replaces is not appropriate.

Chris



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-06-08 Thread Samuel Henrique
Control: tags 1072733 moreinfo
Control: reopen 1071007 =

Hello all,

I wasn't subscribed to both bugs so I missed some of the replies, I'm
subscribed now.

I'm sending this reply to both #1072733 and #1071007 because they are related
and I'm trying to clarify the situation on both.

Summary:
#1072733 is a bug caused by a tentative fix of #1071007, I believe Francisco
made the right approach there, but maybe I'm missing something, Chris?

#1071007 is still not fully fixed as the approach is problematic.

We should not submit workarounds to upstream when they already pointed out a PR
which should address the issue.

If we need to perform a workaround on our side in the meantime, we need to make
sure it works and we should not submit it upstream (they already have the
proper fix in progress).

Plan:
Let's try to fix this issue using upstream's PR.

If we spot issues with the PR, we can contribute back (but let's make sure it
makes sense to upstream).

If we end up performing a workaround, let's make sure it works and doesn't
cause other issues. It's ok to let the package be removed from testing until
this gets fixed.

If we really want to avoid the package being removed from testing, we can
package a previous release where the issue wasn't present (using the "+really"
versioning mechanism).

The end goal is to get this fixed before the freeze, so we have time.

Now the lengthy replies below:

For #1072733:

Chris
> Per Debian policy this is not the correct solution for naming conflicts. Both
> maintainer (teams), please find a policy compliant solution together.

The solution for this one seems correct, it's a Conflict + Replaces because
both packages provide a "sherlock" library. Am I missing something?

Note that this bug is different from #1071007, they look very similar and
initially I thought they were a dupe.

Nilson
> Your package should not be called in d/rules:
> export PYBUILD_NAME=python-sherlock
> or something similar that wasn't exactly sherlock?

We should keep the same name as changing it will cause issues for users, this
is a real case of a conflicting library name.

For #1071007:

Nilson,
> I just pushed a new version of shelock with a fix for the problem in the
> "master" branch. If that's ok, I'll do a merge.
> https://salsa.debian.org/pkg-security-team/sherlock/-/tree/master?ref_type=heads

We follow the DEP14 for git branching, that means we do namespacing. When
dealing with temp/wip branches, the recommendation is to push to
$username/$branchname. If you push to master, it will lead to confusion as to
which branch is the main one (it's debian/master).

It looks like that branch has been merged into debian/master but master has not
been deleted yet. We have to remove that branch.

> This led to the bug being reopened twice as RC, leading to its removal from
> "testing" even after > pycrc had resolved its conflict issue.

We always try to avoid getting a package removed from testing, but sometimes it
happens and that's ok, once the issue is fixed the package will get back. This
is only worrying when removing the package causes a cascade effect or when we
are close to the freeze for stable.

We don't have to rush for a fix in this case, it's better to be precise.

> I made a pull request to usptream
> https://github.com/sherlock-project/sherlock/pull/2147

We should not be submitting this to upstream when they already pointed us to a
PR that solves the issue. We have to try working with that PR. If  that's not
possible, we can patch it ourselves with a different approach, but we should
not submit our workaround to upstream because they already have a proper
solution in progress.

> In accordance with the other Package Uploaders,
> Debian Developer, Francisco Vilmar.
> I will be closing the bug.
> Since the problem itself, with Sherlock installing its
modules in the root of the packages, has been fixed.

The upload done by Francisco for #1072733 can't be used to close #1071007, he
was fixing an issue caused by one of the uploads that tried to fix #1071007, it
was not fixing #1071007 itself.

> sherlock (0.14.4+git20240531.e5ad3c4-1) unstable; urgency=medium
> .
>   * New upstream version 0.14.4+git20240531.e5ad3c4
>   * debian/patches: Adjusted directory installation package (Closes: #1071007)

This upload is indeed trying to solve #1071007 by patching the source code
instead of pulling the upstream PR.
Looking at the list of files shiped [0], the egginfo files are in the wrong
place.

I also find the following commit confusing:
https://salsa.debian.org/pkg-security-team/sherlock/-/commit/fc6e9929b1018d411df599a68d5898e42bfe6560

The commit description does not mention what/why the changes are being made,
for example, why the change from dh_auto_install to dh_install.

Cheers,

[0] https://packages.debian.org/trixie/all/sherlock/filelist

--
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-05-27 Thread Samuel Henrique
Control: reopen -1 =

> I have read the other replies to the bug, which I missed previously.
> It's not upstream's intention to ship the modules in dist-packages.

Nevermind this, I misread upstream's response. Upstream contacted me to ask
about it and it's clear to me now.

Nilson:
> As Sherlock is a private module, I moved it to usr/share according to this
> policy here:

Sherlock is not a private module.

Please proceed with importing the upstream PR to fix the issue.

Cheers,

--
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-05-24 Thread Samuel Henrique
Control: reopen -1 =

The latest upload broke the package, this time by mis-placing the files in
/usr/share/:
https://salsa.debian.org/pkg-security-team/sherlock/-/commit/58dacca3117b66341a4371431d6f38a1d35b08c9
https://salsa.debian.org/pkg-security-team/sherlock/-/commit/00a20c5cc3a9c42a295e886dee1db49472338c4e

The commit "58dacca3117b66341a4371431d6f38a1d35b08c" is also doing more than
what's described in its message. I'm not sure dh_link is the right place to
remove test files, but I haven't looked into that deeply.

This breaks the ability to import sherlock into other python scripts, since
it's not under dist-packages anymore.

The original proposed solution to the issue still stands: we just need to not
ship files at the root of dist-packages.

Regards,

-- 
Samuel Henrique 



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-05-18 Thread Eriberto
Em sáb., 18 de mai. de 2024 às 21:08, Samuel Henrique
 escreveu:
>
> Control: reopen -1 =
>
> I see on git that the bug was closed with a Conflicts+Replaces stanza, but
> that's not the correct solution for this issue.
>
> As discussed on this bugreport, the fix is to not ship the file.
>
> Reopening to block the problematic package to migrate to testing.
>
> Cheers,
>
> --
> Samuel Henrique 
>

In addition, 'Conflicts' is a very extreme solution for many use cases.

Eriberto



Bug#1071007: sherlock: Must not ship /usr/lib/python3/dist-packages/__init__.py

2024-05-18 Thread Samuel Henrique
Control: reopen -1 =

I see on git that the bug was closed with a Conflicts+Replaces stanza, but
that's not the correct solution for this issue.

As discussed on this bugreport, the fix is to not ship the file.

Reopening to block the problematic package to migrate to testing.

Cheers,

--
Samuel Henrique