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

bazarr: 1.1.1 -> 1.1.2 #196801

Merged
merged 1 commit into from
Nov 4, 2022
Merged

bazarr: 1.1.1 -> 1.1.2 #196801

merged 1 commit into from
Nov 4, 2022

Conversation

andreisergiu98
Copy link
Member

@andreisergiu98 andreisergiu98 commented Oct 19, 2022

Description of changes

Updated bazarr
Closes #196226

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@NULLx76 NULLx76 self-requested a review October 31, 2022 17:01
Copy link
Member

@NULLx76 NULLx76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message for 461470b9f3a6ed65fd901549e36ef36683957a80, should probably be prefixed with something like nixos/tests/bazarr: or just squashed into the other commit

@andreisergiu98
Copy link
Member Author

@NULLx76 done

@bjornfor
Copy link
Contributor

bjornfor commented Nov 2, 2022

The test needs more fixing, it even fails on master (after applying the fix from this PR).

@06kellyjac
Copy link
Member

As an aside: I use unrar-wrapper which has the free unar stand in place for the unfree unrar and it seems to work fine

bazarr = prev.bazarr.override { unrar = prev.unrar-wrapper; };

Probably not something we should do by default until the bazarr maintainers say it's supported but since unrar being unfree was mentioned I thought I'd post this :)

@bjornfor
Copy link
Contributor

bjornfor commented Nov 2, 2022

If you add time.timeZone = "UTC"; to the nixos test configuration the test also pass. Apparently bazarr tries to use UTC itself when no timezone is given, but it fails. Explicitly setting the timezone seems to work.

@bjornfor
Copy link
Contributor

bjornfor commented Nov 2, 2022

If you add time.timeZone = "UTC"; to the nixos test configuration the test also pass. Apparently bazarr tries to use UTC itself when no timezone is given, but it fails. Explicitly setting the timezone seems to work.

I made an upstream issue: morpheus65535/bazarr#1983

@andreisergiu98
Copy link
Member Author

So should we add time.timeZone = "UTC" to the test or should we ignore the errors until it is fixed upstream?
Regarding unrar, bazarr 1.1.3 added support for unar, so I think it's safer to switch to it when it's released.

@06kellyjac
Copy link
Member

bazarr 1.1.3 added support for unar, so I think it's safer to switch to it when it's released.

yoooo, awesome

@bjornfor
Copy link
Contributor

bjornfor commented Nov 2, 2022

So should we add time.timeZone = "UTC" to the test or should we ignore the errors until it is fixed upstream?

I would integrate that fix into the/a "nixos/tests/bazarr: fix it" commit, then do the version bump.

@bjornfor
Copy link
Contributor

bjornfor commented Nov 4, 2022

I would integrate that fix into the/a "nixos/tests/bazarr: fix it" commit, then do the version bump.

I did that here: #199517

@andreisergiu98
Copy link
Member Author

Cool! After you merge it, I'll drop the commit on the test and leave only the bump.

@bjornfor
Copy link
Contributor

bjornfor commented Nov 4, 2022

Cool! After you merge it, I'll drop the commit on the test and leave only the bump.

Merged.

Copy link
Contributor

@bjornfor bjornfor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased on master and nix-build ./nixos/tests/bazarr.nix passed.

@bjornfor bjornfor merged commit 1d9b905 into NixOS:master Nov 4, 2022
@andreisergiu98 andreisergiu98 deleted the bazarr-1.1.2 branch December 7, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants