Spec URL: https://github.com/LecrisUT/sqlx-rpmspec/raw/b8d1b82fdbcf7e5292f2c61076ac42c6f84cee7b/rust-finl_unicode/rust-finl_unicode.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/packit/LecrisUT-sqlx-rpmspec-main/fedora-rawhide-x86_64/07376136-rust-finl_unicode/rust-finl_unicode-1.2.0-1.fc41.src.rpm Description: Required dependency for `rust-stringprep` -> `rust-sqlx` -> `rust-atuin` Fedora Account System Username: lecris rust2rpm.toml: ```toml [scripts.prep] post = [ "# Do not depend on criterion; it is needed only for benchmarks.", "tomcli set Cargo.toml del dev-dependencies.criterion", ] ```
Copr build: https://copr.fedorainfracloud.org/coprs/build/7424050 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279514-rust-finl_unicode/fedora-rawhide-x86_64/07424050-rust-finl_unicode/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.
This package has already been submitted twice, and it's proven to be a bit problematic - see comments on the previous tickets: https://bugzilla.redhat.com/show_bug.cgi?id=2250496 https://bugzilla.redhat.com/show_bug.cgi?id=2246779
Oh, I must have missed that one when I searched bugzilla. So what action do you want for this one, it is only required by `stringprep` and if it can be decoupled there, than it is also ok. But I don't see it can be done easily [1]. As for `stringprep` it seems to be used only for `sqlx-mysql` (not really? don't see it mentioned in code) and `sqlx-postgres` [2] Otherwise should we continue from #2246779? From what I read, we need to: - Pull in the patch in: https://github.com/dahosek/finl_unicode/pull/17 - Patch out the resources for benchmark? Doesn't quite make sense since srpm would still contain the incompatible license files? I guess one option is to ask them to move the test/benchmark files outside of crate and only in the workspace? Would that be problematic for them? [1]: https://github.com/sfackler/rust-stringprep/blob/6b6bbbea8c5e35526eac008845ae33b9aea0c675/src/lib.rs#L5 [2]: https://github.com/launchbadge/sqlx/blob/5d6c33ed65cc2d4671a9f569c565ab18f1ea67aa/sqlx-postgres/src/connection/sasl.rs#L10
For now I have created the issue [1] upstream and tagged the upstream devs. Hope I described the problem accurately. [1]: https://github.com/dahosek/finl_unicode/issues/18
(In reply to Cristian Le from comment #3) > Oh, I must have missed that one when I searched bugzilla. > > So what action do you want for this one, it is only required by `stringprep` > and if it can be decoupled there, than it is also ok. But I don't see it can > be done easily [1]. Yeah, I don't think the dependency can be dropped from stringprep easily. > As for `stringprep` it seems to be used only for `sqlx-mysql` (not really? > don't see it mentioned in code) and `sqlx-postgres` [2] I agree, the dependency seems to be unused in sqlx-mysql. So it's only a dependency in sqlx-postgres. > Otherwise should we continue from #2246779? From what I read, we need to: > - Pull in the patch in: https://github.com/dahosek/finl_unicode/pull/17 I don't think that alone would be enough. The copyright statement in the README is very strange - usually (c) "All Rights Preserved" means that something is not licensed *at all*: https://github.com/dahosek/finl_unicode?tab=readme-ov-file#unicode-copyright-notice The upstream project doesn't seem interested in addressing this issue, so I'm not sure what the best approach would be here. You might want to ask on the "legal" mailing list for help. > - Patch out the resources for benchmark? Doesn't quite make sense since srpm > would still contain the incompatible license files? I guess one option is to > ask them to move the test/benchmark files outside of crate and only in the > workspace? Would that be problematic for them? Option 1: Create a "clean" source tarball without the benchmark data. This would mean not using sources from crates.io directly. see https://github.com/statrs-dev/statrs/issues/195 for a similar issue, and https://src.fedoraproject.org/rpms/rust-fiat-crypto/blob/rawhide/f/gen_clean_tarball.sh or https://src.fedoraproject.org/rpms/rust-statrs/blob/rawhide/f/gen_clean_tarball.sh for possible ways to script creation of "clean" sources. Option 2: Submit a patch to upstream to exclude the offending files from published crates (i.e. with the "package.exclude" setting in Cargo.toml). That's more of a long-term solution, since it would require upstream to merge these changes and publish a new release for them to take effect. However, both of these don't address the Unicode license issue (or lack thereof). If you don't actually need the sqlx-postgres support, I would recommend to avoid packaging finl_unicode, stringprep, and sqlx-postgres for now, and to remove the unused stringprep dependency from sqlx-mysql, and revisit packaging the postgres support when / if the finl_unicode situation is cleared up.
> If you don't actually need the sqlx-postgres support, I would recommend to avoid packaging finl_unicode, stringprep, and sqlx-postgres for now, and to remove the unused stringprep dependency from sqlx-mysql, and revisit packaging the postgres support when / if the finl_unicode situation is cleared up. Seems a bit tricky [1]. I think that's also the only database support for `atuin-server`. > "All rights Preserved" I guess you meant "All rights reserved", also meaning that it is non-free? Weren't there other packages mentioned like `unicode-ident` which have the same license . Is the license incompatible or the lack of license file? My current plan is Option2 + Option1 in the meantime + PR17 which at least patches the crate metadata. Any other steps for that? Probably patching the license header for the source files themselves, but I'm not sure where and how for that. I guess I should also write an email for the legal mailing list for more feedback? [1]: https://github.com/atuinsh/atuin/blob/eebfd048797d2faffd0a9c6633580c5e3077d688/Cargo.toml#L51-L53
(In reply to Cristian Le from comment #6) > > If you don't actually need the sqlx-postgres support, I would recommend to avoid packaging finl_unicode, stringprep, and sqlx-postgres for now, and to remove the unused stringprep dependency from sqlx-mysql, and revisit packaging the postgres support when / if the finl_unicode situation is cleared up. > > Seems a bit tricky [1]. I think that's also the only database support for > `atuin-server`. Ah, perfect. It depends on the *one* backend that is problematic ... > > "All rights Preserved" > > I guess you meant "All rights reserved" Yes. Typo :) > also meaning that it is non-free? That's what this usually means, yes. > Weren't there other packages mentioned like `unicode-ident` which have the > same license . Is the license incompatible or the lack of license file? To me, this looks like the upstream project has no idea what they're doing, but I might be wrong. Other projects that include code generated from Unicode data (but not unicode data itself!) use Unicode-DFS-2016 license, which is OK for Fedora. So maybe the difference here is that finl_unicode actually bundles the Unicode data itself and not only the code generated from it? But I'm not sure. Either way, the "All rights reserved" notice seems to be wrong. > My current plan is Option2 + Option1 in the meantime + PR17 which at least > patches the crate metadata. Any other steps for that? Probably patching the > license header for the source files themselves, but I'm not sure where and > how for that. I guess I should also write an email for the legal mailing > list for more feedback? Without more clarifications from finl_unicode upstream, I don't think it can be packaged in the current form, even if you include PR +17. I don't think patching the license headers in the source files is necessary, their license is not impacted by the data that is shipped alongside them. But yes, I think posting to the "legal" mailing list for help would be a good idea.
Small update, I dug a bit deeper into the `strinprep` package, because looking at that what `finl_unicode` is doing, it is weird that it needs that package. It turns out only 2 functions are actually used (is_mark and is_separator), which do make me think these could be replaced with some built-in functions. I am not sure what common library is there for these functions. Some feedback would be useful [1] [1]: https://github.com/sfackler/rust-stringprep/pull/9
It looks like the switch from finl_unicode to unicode-properties is acceptable for the stringprep developer. Feel free to close this review request as soon as you're sure that you no longer need it.
Interestingly, upstream mentioned they will be addressing the packaging issues: https://github.com/dahosek/finl_unicode/issues/18 Might help the few other packages that are blocked by this