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

add taglib and imagemagick to sln #82

Closed
wants to merge 1 commit into from
Closed

add taglib and imagemagick to sln #82

wants to merge 1 commit into from

Conversation

dkanada
Copy link
Member

@dkanada dkanada commented Dec 12, 2018

Are these libraries supposed to be built at the moment? They weren't in the sln file. Also, the taglib submodule seems to throw an error that it only works with .NetFramework 4.0 and not Core on my machine.

Copy link
Member

@nvllsvm nvllsvm left a comment

Choose a reason for hiding this comment

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

This was added earlier but broke the build in both the Docker image and my development environment (Arch Linux with dotnet-sdk==2.2.0+100-1).

Try testing your change by building it with in the Docker image.

docker build .

EDIT - I should mention that I have no clue what I'm doing with .NET dev. All I know is that the current Dockerfile builds and runs well for me.

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

Without this I get seventeen build errors instead of two build errors. ImageMagick isn't found at all and taglib throws the same error because the source built here is the same source I added to the main sln file. Still looking for the cause.

@LogicalPhallacy
Copy link
Contributor

The current SLN in master works fine for me in full visual studio IF you make sure you pull submodules as well. I wasn't having the hangup with imagemagick but I was with taglib and what was happening was git wasn't getting the submodules down right. Try git submodule update --init --recursive on the latest and see if you are ok afterwards

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

Yeah I pulled the submodules, the exact error is below which made me think it might be a different issue. @nvllsvm what error did you get with the changes here?

Error NU1201: Project taglib-sharp is not compatible with netcoreapp2.1 (.NETCoreApp,Version=v2.1). Project taglib-sharp supports: net40 (.NETFramework,Version=v4.0) (NU1201) (Emby.Photos)

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

I would try installing dotnet4.0 but I was under the impression only dotnetCore was required to build this project.

@nvllsvm
Copy link
Member

nvllsvm commented Dec 12, 2018

I'll apaste it when I'm back on my workstation. For now, try building the Docker image. It's a portable environment that should replicate the problem I'm seeing.

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

Just tried the docker image, it throws the same error I posted above using this patch. Unrelated but it looks like there isn't any mention of 2-sdk in the dotnet repo, it seems to be aliased to 2.2-sdk in the manifest. Since it works with the current taglib file that would mean either the current taglib doesn't require dotnet4.0 or the 2.2-sdk docker files include dotnet4.0 in the included files.

Since I can't currently build the project in my IDE before or after this change but the docker build only works before this change I am thinking that the docker config somehow includes dotnet4.0 in the build.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018

They're in the sln file on master currenty (

Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ImageMagickSharp", "ImageMagickSharp\ImageMagickSharp\ImageMagickSharp.csproj", "{124E0710-6572-497B-B2A5-696AA08AD8BB}"
)

I submitted a PR to add them a few hours ago since I had very similar problems to you (in Rider on MacOS). Everything is now working for me - @nvllsvm's changes removed a few of the build configurations for these projects (I think), which seemed to break the Docker build.

Even after merging his changes, these projects are still part of the solution and building still works fine for me on MacOS.

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

They're in the sln but they dont seem to be included in any of the build versions below, which I think is causing my issues. I am using the latest commit from master though so I don't think it's a git pull issue.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018

Which platform are you on?

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

Arch Linux, same as @nvllsvm.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018 via email

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

According to this comment mono is also required to build taglib-sharp. I am not sure whether or not that dependency is open source, but I also don't see it in the build files, so I am wondering why the docker image works without mentioning mono.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018 via email

@JustAMan
Copy link
Contributor

Before the full Jellyfin started I think I built everything using only dotnet core 2.1, no Mono was required. I wasn't rebuilding TagLib, though.

@dkanada note that one of your errors seems to point to "netcore-app", not just "netcore". I'm not proficient with .net, but that looks strange.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018

Error NU1201: Project taglib-sharp is not compatible with netcoreapp2.1 (.NETCoreApp,Version=v2.1). Project taglib-sharp supports: net40 (.NETFramework,Version=v4.0) (NU1201) (Emby.Photos)

@dkanada can you take a look at your taglib-sharp.csproj? Mine has the following:
<TargetFrameworks>net40;netstandard2.0</TargetFrameworks> so it should work with .Net Core (netcoreapp2.1).

@Bond-009
Copy link
Member

@BnMcG publishing our own taglib-sharp nuget package doesn't seem like a good idea. The best solution (IMO) is waiting till someone at mono releases a new taglib-sharp nuget package, since the current one is from 2012.
taglib-sharp issue: mono/taglib-sharp#40

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018

@Bond-009 Agreed if that's an option, there doesn't seem to be much movement on mono/taglib-sharp#40, though.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018

@dkanada, Can you try pulling https://github.com/bnmcg/jellyfin/tree/build-thirdparty and building with this branch? I reverted this commit (so now ImageMagickSharp and TagLibSharp are both included in the sln and have build configurations), and now ImageMagickSharp and TagLibSharp build for me again.

Is this also the case for you?

@nvllsvm I've just tried docker build, and I think the 2 errors are where it's trying to build the .Net 4.5 version of taglibsharp and imagemagicksharp. I'll do some more digging when I get home, but I think at least on my workstation without docker, it builds the net standard libraries fine, and then fails on the net 4.5 versions.

I'll see if we can just remove the .Net 4.5 target from both of these projects if everything now targets .net core/standard, or failing that see if we can specify which target to build against in the build command, rather than trying to build all targets (which seems to be what's happening at the moment).

I think adding Mono to the Docker image would also fix the build issue, but that seems like a less optimal solution.

@dkanada
Copy link
Member Author

dkanada commented Dec 12, 2018

@BnMcG nope I get the same error, which is weird because MonoDevelop recognizes my Mono installation.

@joshuaboniface
Copy link
Member

We should be aiming to remove aall Mono dependencies in favour of .NET Core if at all possible.

@BnMcG
Copy link
Contributor

BnMcG commented Dec 12, 2018 via email

@nvllsvm
Copy link
Member

nvllsvm commented Dec 12, 2018

@joshuaboniface #95

@dkanada
Copy link
Member Author

dkanada commented Dec 14, 2018

The easiest solution is to just remove the net45 and net40 target frameworks from ImageMagick and taglib-sharp, but we currently include taglib-sharp as a submodule. I tried the docker recipe with this change and the didn't see any errors from the build step. This also fixed my dev environment which was nice. I previously had to manually remove net40 from taglib-sharp to build Jellyfin.

@dkanada
Copy link
Member Author

dkanada commented Dec 14, 2018

Not the same error message but this thread has a lot of people reporting issues using <TargetFrameworks> instead of <TargetFramework> and also mentions the same fix.

@dkanada
Copy link
Member Author

dkanada commented Dec 15, 2018

I am closing this to rebase on develop since pull requests go there now.

@dkanada dkanada closed this Dec 15, 2018
cvium pushed a commit that referenced this pull request Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants