Bug 2261953 - Review Request: python-python-ulid - Universally unique lexicographically sortable identifier
Summary: Review Request: python-python-ulid - Universally unique lexicographically sor...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Maxwell G
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2261943
TreeView+ depends on / blocked
 
Reported: 2024-01-30 15:03 UTC by Ben Beasley
Modified: 2024-02-05 16:15 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2024-02-05 16:15:24 UTC
Type: Bug
Embargoed:
maxwell: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6973682 to 6976028 (2.84 KB, patch)
2024-01-31 14:48 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6976028 to 6980420 (2.88 KB, patch)
2024-02-01 19:32 UTC, Fedora Review Service
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Github mdomke python-ulid issues 14 0 None open Multiple conflicting command-line tools called ulid exist 2024-01-31 14:44:59 UTC
Github oklog ulid issues 111 0 None open Multiple conflicting command-line tools called ulid exist 2024-01-31 14:44:59 UTC

Description Ben Beasley 2024-01-30 15:03:45 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/music/pydantic/fedora-rawhide-x86_64/06973584-python-python-ulid/python-python-ulid.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/music/pydantic/fedora-rawhide-x86_64/06973584-python-python-ulid/python-python-ulid-2.2.0-1.fc40.src.rpm

Description:

A ULID is a universally unique lexicographically sortable identifier. It is

  • 128-bit compatible with UUID
  • 1.21e+24 unique ULIDs per millisecond
  • Lexicographically sortable!
  • Canonically encoded as a 26 character string, as opposed to the 36
    character UUID
  • Uses Crockford’s base32 for better efficiency and readability (5 bits per
    character)
  • Case insensitive
  • No special characters (URL safe)

For more information have a look at the original specification,
https://github.com/alizain/ulid#specification.

Fedora Account System Username: music

This is a dependency for python-pydantic-extra-types, beginning with version 2.2.0.

Comment 1 Fedora Review Service 2024-01-30 15:10:41 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6973682
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2261953-python-python-ulid/fedora-rawhide-x86_64/06973682-python-python-ulid/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Maxwell G 2024-01-30 23:16:30 UTC
Hmm, this naming situation is a bit of mess, but you did handle it as best as possible and in compliance with the guidelines.

Maxwell's Python Package Review Template
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


- [x] The License tag reflects the package contents and uses the correct identifiers.
- [x] The license text is included and marked with %license.
- [x] The package builds successfully in mock.
- [x] The package is installable (checked by fedora-review).
- [x] There are no relevant rpmlint errors.
- [x] The package runs tests in %check.
- [x] The latest version is packaged or packaging an earlier version is justified.
- [x] The packager considers avoiding confusing `%foo_name` macros. (Not a blocker)
- [x] Libraries: The package name has a `python3-` prefix and uses the canonical project name
- [x] The pyproject macros are used.
- [x] There are no bundled libraries.
- [x] The package complies with the Python and general Packaging Guidelines.

Other problems:

- The URL is wrong. It should be https://github.com/mdomke/python-ulid as far as I can tell.
- There are unicode quotes in the comment at the top of the specfile and unicode bullet points in the common_description. I noticed because the Copr specfile page apparently does not handle unicode encoding properly. I would personally use standard ASCII quotes and * or - for bullet points, but that's up to you.
- I would keep the ulid binary and add `Conflicts: golang-github-oklog-ulid` or maybe rename it to /usr/bin/python-ulid. (I prefer the first solution but would accept either.) I don't think removing it completely is the right solution.

Thanks!

Comment 3 Ben Beasley 2024-01-31 00:59:51 UTC
(In reply to Maxwell G from comment #2)
> - The URL is wrong. It should be https://github.com/mdomke/python-ulid as
> far as I can tell.

You’re right. I got sloppy editing the spec file I based this on. Thanks for noticing. I’ll fix it.

> - There are unicode quotes in the comment at the top of the specfile and
> unicode bullet points in the common_description. I noticed because the Copr
> specfile page apparently does not handle unicode encoding properly. I would
> personally use standard ASCII quotes and * or - for bullet points, but
> that's up to you.

I like nice Unicode glyphs, and I think we’re far enough into the 21st century that we shouldn’t have to stick with ASCII, but I don’t feel really strongly about it in this particular case. Since I plan to ask you to co-maintain this package, I don’t mind hewing to your preference here.

> - I would keep the ulid binary and add `Conflicts: golang-github-oklog-ulid`
> or maybe rename it to /usr/bin/python-ulid. (I prefer the first solution but
> would accept either.) I don't think removing it completely is the right
> solution.

The packaging guidelines are kind of bureaucratic here: they would like me to first approach both upstreams about renaming[1], then once I have convinced them both to respond and neither wants to change, either go try to build consensus with all the other distros[1] or add the Conflicts, which I *have to do in both packages*[2]. Obviously, I don’t want to block this review and the python-pydantic-extra-types update on an extended discussion process. I’ll start by filing the issues and seeing if there is a prompt response while I consider what to do.

If I go ahead and add the Conflicts, I need to send a PR to https://src.fedoraproject.org/rpms/golang-github-oklog-ulid to add it there, too. I also need to consider whether the conflicting binary should be split into its own subpackage separate from the library in order to reduce the scope of the conflict, and if so, what I should call it? python-ulid-tool? I shouldn’t use python-ulid for the subpackage name, because that is the same as the base package name for a hypothetical future package of https://pypi.org/project/ulid/, should it somehow spring back to life…

If I go ahead and rename the binary, things are a lot tidier, but I also feel like I am straying farther from the guidelines by not trying to build a cross-distro consensus first – but is this even practical when nobody else has this problem right now? Here’s a quick survey of a few other distros:

- Arch has only the Python package, and it provides /usr/bin/ulid: https://archlinux.org/packages/extra/any/python-ulid/
- openSUSE has neither package
- Debian and Ubuntu have only the golang package, and it includes only the library, not /usr/bin/ulid: https://packages.ubuntu.com/noble/all/golang-github-oklog-ulid-dev/filelist
- Gentoo has neither package

At the moment, I’m leaning toward the Conflicts, golang-github-oklog-ulid PR, and tool subpackage approach.

[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/#_approaching_upstream
[2] https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/#_incompatible_binary_files_with_conflicting_naming_and_stubborn_upstreams

Comment 4 Ben Beasley 2024-01-31 14:45:00 UTC
(In reply to Ben Beasley from comment #3)
> At the moment, I’m leaning toward the Conflicts, golang-github-oklog-ulid
> PR, and tool subpackage approach.

I’m still open to changes to the approach, and it’s possible that upstream will step up quickly, but that approach could look like this:

New Spec URL: https://download.copr.fedorainfracloud.org/results/music/pydantic/fedora-rawhide-x86_64/06976002-python-python-ulid/python-python-ulid.spec
New SRPM URL: https://download.copr.fedorainfracloud.org/results/music/pydantic/fedora-rawhide-x86_64/06976002-python-python-ulid/python-python-ulid-2.2.0-1.fc40.src.rpm

(Sometimes I hand-write man pages for binaries where help2man isn’t very fruitful and the interface doesn’t change rapidly, and I would consider doing that here, but I don’t really want to do it until I know if upstream will do anything with the issues I filed.)

Comment 5 Fedora Review Service 2024-01-31 14:48:40 UTC
Created attachment 2014199 [details]
The .spec file difference from Copr build 6973682 to 6976028

Comment 6 Fedora Review Service 2024-01-31 14:48:42 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6976028
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2261953-python-python-ulid/fedora-rawhide-x86_64/06976028-python-python-ulid/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 7 Ben Beasley 2024-01-31 18:37:15 UTC
In https://github.com/oklog/ulid/issues/111, upstream noted that the names of go binaries aren’t fully under upstream control, suggested we build that package’s binary as ulid-go instead. I’ve proposed this in https://src.fedoraproject.org/rpms/golang-github-oklog-ulid/pull-request/1. Please feel free to give feedback on that PR.

Comment 8 Ben Beasley 2024-02-01 19:26:41 UTC
The conflict is resolved in F40+ by renaming the existing /usr/bin/ulid to /usr/bin/ulid-go, so I moved the /usr/bin/ulid binary back into python3-python-ulid and added man pages.

New Spec URL: https://music.fedorapeople.org/python-python-ulid.spec
New SRPM URL: https://music.fedorapeople.org/python-python-ulid-2.2.0-1.fc39.src.rpm

Comment 9 Fedora Review Service 2024-02-01 19:32:09 UTC
Created attachment 2014448 [details]
The .spec file difference from Copr build 6976028 to 6980420

Comment 10 Fedora Review Service 2024-02-01 19:32:13 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6980420
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2261953-python-python-ulid/fedora-rawhide-x86_64/06980420-python-python-ulid/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 11 Maxwell G 2024-02-05 15:39:23 UTC
This looks good to me, although do note that the Conflicts is in the wrong place; it needs to be under the python3-python-ulid subpackage definition. Please fix that on import. Other than that, package approved! On import, don't forget to add the package to release-monitoring.org. You can give @python-packagers-sig `commit` if you wish. I am also happy to co-maintain.

Thanks!

Comment 12 Ben Beasley 2024-02-05 16:04:51 UTC
(In reply to Maxwell G from comment #11)
> This looks good to me,

Thanks!

> although do note that the Conflicts is in the wrong
> place; it needs to be under the python3-python-ulid subpackage definition.

Sigh… a classic mistake. Thanks for spotting it.

> Please fix that on import.

Will do.

> Other than that, package approved! On import,
> don't forget to add the package to release-monitoring.org. You can give
> @python-packagers-sig `commit` if you wish. I am also happy to co-maintain.
> 
> Thanks!

Ok!

Comment 13 Fedora Admin user for bugzilla script actions 2024-02-05 16:06:01 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-python-ulid

Comment 14 Ben Beasley 2024-02-05 16:08:00 UTC
https://release-monitoring.org/project/98094/

Comment 15 Fedora Update System 2024-02-05 16:14:55 UTC
FEDORA-2024-43dea391a0 has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-43dea391a0

Comment 16 Fedora Update System 2024-02-05 16:15:24 UTC
FEDORA-2024-43dea391a0 has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.