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

Mono: fix DSM 7 compatibility #5604

Merged
merged 2 commits into from
Feb 20, 2023
Merged

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Feb 12, 2023

Description

This PR is to update the patch files to use /var/packages/mono/var instead of /var/packages/mono/target/var for DSM 7 compatibility.

Follow up of #5522
Supersedes #5070
Fixes #5051, #5354

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix

@mreid-tt mreid-tt requested a review from hgy59 February 12, 2023 22:08
@mreid-tt
Copy link
Contributor Author

mreid-tt commented Feb 12, 2023

@hgy59, I did a clean install as well as an upgrade install and tested Sonarr v3 in each case. All the logs looked clean as follows:

For the testing with Sonarr v3 both scenarios were successful in that the certificate errors were not encountered.

@publicarray
Copy link
Member

publicarray commented Feb 16, 2023

Good work!

Are you sure that is the correct followup PR you mention?

I'll test it out over the weekend

@mreid-tt
Copy link
Contributor Author

Thanks. This PR follows on the last merge into the repository. It does necessarily change something there but it was in that PR that I realised that the original issues (#5051, #5354) were in fact fixed. Based on discussion with @hgy59 in #5522 (comment) and #5522 (comment) it seems that something was previously broken in the framework and the last published version did not correctly apply the patch to relocate special folders.

In that conversation he also pointed out that the var location also needed to be updated for DSM7 and I confirmed that this was also needed for people who were upgrading from the existing mono version in #5522 (comment). So you are correct that it's not a follow-up in the sense that it fixes something that was broken in #5522 but that was where the discoveries originated from so I thought it should be mentioned.

@mreid-tt mreid-tt mentioned this pull request Feb 18, 2023
6 tasks
@mreid-tt
Copy link
Contributor Author

@hgy59, is this good to merge?

Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

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

LGTM.
we only have to keep in mind, that aarch64-6.1 must not be published due to the known error with ECC.

@mreid-tt mreid-tt merged commit ae916b4 into SynoCommunity:master Feb 20, 2023
@mreid-tt mreid-tt deleted the mono-patch branch February 20, 2023 00:26
@mreid-tt
Copy link
Contributor Author

we only have to keep in mind, that aarch64-6.1 must not be published due to the known error with ECC.

No need to remember, I've included the exclusion in the Makefile with a reference to the originating issue.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Feb 20, 2023

@hgy59, when this is published I would suggest removing v5.20.1.34-18 as that version was known to have the issue applying the patch to relocate special folders (I've already confirmed that v5.20.1.34-17 was good).

@hgy59 hgy59 added status/published Published and activated (may take up to 48h until visible in DSM package manager) and removed status/to-publish labels Feb 20, 2023
@mreid-tt mreid-tt mentioned this pull request Feb 22, 2023
10 tasks
@hgy59
Copy link
Contributor

hgy59 commented Feb 23, 2023

@hgy59, when this is published I would suggest removing v5.20.1.34-18 as that version was known to have the issue applying the patch to relocate special folders (I've already confirmed that v5.20.1.34-17 was good).

I have deleted all earlier 5.20 versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/published Published and activated (may take up to 48h until visible in DSM package manager)
Projects
None yet
3 participants