NCrusher74 / SwiftTaggerID3

A Swift library for reading and editing ID3 tags
Apache License 2.0
5 stars 2 forks source link

Migrating to package second attempt #9

Closed NCrusher74 closed 4 years ago

NCrusher74 commented 4 years ago

I don't think I did it right...

To tired to figure it out tonight. Will work on it tomorrow.

NCrusher74 commented 4 years ago

I don't know why I always have such a hard time figuring out packages, but I do. No matter how many times I read the documentation, or look at other packages to use them as an example, it seems like whatever I'm trying to do is just different enough that I can't bridge the gap between what I'm seeing and what I need to do.

It doesn't seem to want to find Mp3File, which is the starting point for everything. In examples I've seen, it seems like the purpose Mp3File serves is usually handled in a file that's named after the package name (so it'd be SwiftTaggerID3.) So, should I rename Mp3File (and then fix everything that breaks?)

Do I still need SwiftTaggerID3.h and SwiftTaggerID3.plist? Do I still need the Products folder and the framework file therein? I thought I started out with a manifest file for the tests, but I think I accidentally overwrote it when moving files to their new homes? Do I still need the plist file for the tests folder?

NCrusher74 commented 4 years ago

Okay, I'm reverting a few commits because I think it might be easier to start troubleshooting from the point at which I added your Workspace package.

NCrusher74 commented 4 years ago

Okay, so at this point what I have is a package that doesn't appear to be seeing Data as an identifier, and for which I clearly need to declare a swift version because it's telling me some of the stuff I'm using might not be available in earlier versions. I don't have any dependencies declared in the manifest.

SDGGiesbrecht commented 4 years ago

I’ll check the branch out and take a closer look.

P.S. Thank you for the donation. (At least I assume that was you from the message that accompanied it. The signed name has nothing in common with your username here, so I could be out to lunch.)

NCrusher74 commented 4 years ago

You're very welcome. That was me. My paypal is leftover from a time when I was using a pen name and I've not yet figured out how to migrate it all over.

SDGGiesbrecht commented 4 years ago

That didn’t turn out to be as complicated as I expected.

Xcode has been providing implicit imports of Foundation, but you have to make them explicit outside Xcode.

The @availability stuff restricts certain things to the platforms where the needed pieces are available. Xcode was just restricting the entire thing automatically (which typically makes sense for the application at the top), but you usually want to restrict as little as possible with SwiftPM, to reduce the incompatibilities you cause for your clients.

That got it building again.

You’ll still have to move the manifest to the root of the repository if you want it to be reachable by anything else.

NCrusher74 commented 4 years ago

So it needs to go up one level, so that it's in the same directory as the project file is in? (and then I'll probably have to change my pathing accordingly?)

Do I need the project file any longer? Is it better to have that, or not? And what about the other artifacts of the project, like the header file and plist?

SDGGiesbrecht commented 4 years ago

Do I need the project file any longer? Is it better to have that, or not? And what about the other artifacts of the project, like the header file and plist?

You don’t need any of those.

NCrusher74 commented 4 years ago

Okay, I moved everything up one level and deleted the (now-empty) second-level SwiftTaggerID3 directory. Everything is in the top-level SwiftTaggerID3 directory now, the project file, plist files, and header file have all been deleted.

SDGGiesbrecht commented 4 years ago

I assume it builds and tests pass? Then it’s ready.

NCrusher74 commented 4 years ago

Oh, yes, it does.

SDGGiesbrecht commented 4 years ago

Then merge and I’ll set up the file header stuff in a separate PR.

NCrusher74 commented 4 years ago

Though, actually, I hadn't tried to run the tests. They're not passing, but I assume it's mostly due to the fact that they're heavily dependent upon the Bundle stuff that isn't there anymore? Do we want to handle that later?

SDGGiesbrecht commented 4 years ago

That should be done now.

You should just need to make sure the paths/URLs are pointing in the new locations, using #file to get the path of the current file and adjusting where you point relative to it. It should be fairly straightforward.

NCrusher74 commented 4 years ago

Okay I will work on that.

NCrusher74 commented 4 years ago

Is this what I should have for the variation on your testSpecificationDirectory we talked about yesterday?

@usableFromInline internal var testMediaDirectory: URL?

public func testSpecificationDirectory(_ callerLocation: StaticString = #file) -> URL {
        let repositoryRoot: URL
        if let overridden = ProcessInfo.processInfo
            .environment["SWIFTPM_PACKAGE_ROOT"]
        {
            // @exempt(from: tests)
            repositoryRoot = URL(fileURLWithPath: overridden)
        } else {
            repositoryRoot = URL(fileURLWithPath: String(describing: callerLocation))
                .deletingLastPathComponent().deletingLastPathComponent().deletingLastPathComponent()
        }
        return repositoryRoot.appendingPathComponent("Tests/SwiftTaggerID3Tests/TestMedia")
}

I removed the part referencing a cache because I don't have that?

SDGGiesbrecht commented 4 years ago

In your case, you can probably reduce the whole thing to just this:

let testMediaDirectory = URL(fileURLWithPath: #file)
  .deletingLastPathComponent()
  // deleting however many...
  .deletingLastPathComponent()
  .appendingPathComponent("however/much/path/is/needed/TestMedia")

The rest of the complexity was only needed to support reuse by other packages, and to make it work in the Android emulator.

NCrusher74 commented 4 years ago

Okay, working on that now.

Just to be clear, if my directory structure is this:

Screen Shot 2020-05-19 at 3 57 31 PM

Where the media files are in the TestMedia subdirectory, and I'm working in the TestMedia.swift file, then all I need is this?

let testMediaDirectory = URL(fileURLWithPath: #file)
    .deletingLastPathComponent()
    .appendingPathComponent("/TestMedia")
SDGGiesbrecht commented 4 years ago

Basically.

let testMediaDirectory = URL(fileURLWithPath: #file) // .../SwiftTaggerID3Tests/TestMedia.swift
    .deletingLastPathComponent() // .../SwiftTaggerID3Tests
    .appendingPathComponent("/TestMedia")  // .../SwiftTaggerID3Tests//TestMedia

But I’m not sure what will happen with that extra slash on the last line.

NCrusher74 commented 4 years ago

lol those slashes are the bane of my existence. I can never quite be sure whether or not I'm supposed to have them. But I got rid of it for now.

NCrusher74 commented 4 years ago

Okay, I've got all the tests fixed and passing, except for one bit of weirdness where suddenly the CreditsListFrame is ending up with the value written to the array twice. I assume this is a bug with the way the output array is appended, I'm pretty sure it has nothing to do with the package, except in that the package required me to restructure things in a way that made it apparent?

SDGGiesbrecht commented 4 years ago

It shouldn’t have anything to do with the switch to a package.

NCrusher74 commented 4 years ago

I know, but for some reason it was passing before and now it's not. It's like a duplicate value is being appended to the value(array) for the CreditsListFrame, and that wasn't happening before.

It would make sense if I were writing to the file that's already got fully-loaded tag on it, because I know I have some work to do there with regard to wiping out values when they should be wiped out, versus keeping the ones we want to keep, but it's being written to the the file that doesn't have any metadata.

I'm trying to track it down.

NCrusher74 commented 4 years ago

Okay figured it out. We're good to go. Will merge now.

SDGGiesbrecht commented 4 years ago

Then I’ll set the headers and stuff up now.

NCrusher74 commented 4 years ago

Awesome, thank you!