Fix spm licenses gen #150

yosshi4486 · 2021-04-06T08:42:45Z

About

I fixed swift package license's generation issue.

What happen?

I had encountered issue that LicensePlist doesn't generate swift package licenses.

Why does the problem occur?

Xcode has its own project.xcworkspace in its subdirectory, and developer can add additional xcworkspace, in most cases it is via Cocoapods.

Reason

Either project.xcworkspace's Package.resolved or YourAdded.xcworkspace's Package.resolved is updated. Not both. Implementation of swift package integration in this repository didn't care the behavior, so old Package.resolved is referred in some situations.

How did you solve it?

Compare modification dates of these files.
(Sources/LicensePlistCore/Entity/FileReader/XcodeProjectFileReader.swift)

guard
    let xcodeprojPackageResolvedModifiedDate = try xcodeprojPackageResolvedPath
        .resourceValues(forKeys: [.attributeModificationDateKey])
        .attributeModificationDate,
    let xcworkspacePackageResolveModifiedDate = try xcworkspacePackageResolvedPath
        .resourceValues(forKeys: [.attributeModificationDateKey])
        .attributeModificationDate
else {
    return try SwiftPackageFileReader(path: defaultPath).read()
}

if xcworkspacePackageResolveModifiedDate >= xcodeprojPackageResolvedModifiedDate {
    return try SwiftPackageFileReader(path: xcworkspacePackageResolvedPath).read()
} else {
    return try SwiftPackageFileReader(path: xcodeprojPackageResolvedPath).read()
}

How did you implement it?

Add test xcode project under Tests/LicensePlistTests/XcodeProjects.
Refactor private "readXxx" methods to FileReader objects for improving testability.
Test them.
Fix the problem.

Who encounter the problem?

A developer who used swift package in early stage of product development, and added cocoapods integration later.

Review Points

🙅‍♂️Changes from Tests/LicensePlistTests/XcodeProjects are not important, They are only test resource files.
🙆‍♂️Changes from Sources/LicensePlistCore/Entity/FileReader and Tests/LicensePlistTests/Entity/FileReader are important. They are core implementations and tests about this PR.

Pass tests?

I run all tests locally by cmd +U.

Related issues?

#140

Other things?

If maintainers allow the refactoring of FileReader, I'd like to rewrite other "readXxx" functions in next pr.

mono0926 · 2021-04-07T08:26:54Z

@yosshi4486

Thank you, I'll check tomorrow.

mono0926

@yosshi4486

How do you handle this error?

And, swiftlint warnings exist. Could you fix that?

swiftlint result

Loading configuration from '.swiftlint.yml'
Linting Swift files at paths 
Linting 'main.swift' (1/55)
Linting 'Consts.swift' (2/55)
Linting 'SearchRequests.swift' (3/55)
Linting 'RepoRequests.swift' (4/55)
Linting 'Extensions.swift' (5/55)
Linting 'GitHubRequest.swift' (6/55)
Linting 'String.extension.swift' (7/55)
Linting 'CocoaPodsLicense.swift' (8/55)
Linting 'Options.swift' (9/55)
Linting 'Manual.swift' (10/55)
Linting 'Library.swift' (11/55)
Linting 'ResultOperation.swift' (12/55)
Linting 'APIKit.extension.swift' (13/55)
Linting 'HasName.swift' (14/55)
Linting 'URL.extension.swift' (15/55)
Linting 'LicensePlistHolder.swift' (16/55)
Linting 'Config.swift' (17/55)
Linting 'GitHubLibraryConfigFile.swift' (18/55)
Linting 'PlistInfo.swift' (19/55)
Linting 'ManualLicense.swift' (20/55)
Linting 'LicenseHTMLHolder.swift' (21/55)
Linting 'CocoaPods.swift' (22/55)
Linting 'VersionInfo.swift' (23/55)
Linting 'GitHubLicense.swift' (24/55)
Linting 'License.swift' (25/55)
Linting 'LicenseMarkdownHolder.swift' (26/55)
Linting 'GitHub.swift' (27/55)
Linting 'SwiftPackage.swift' (28/55)
Linting 'XcodeProjectFileReader.swift' (29/55)
Linting 'SwiftPackageFileReader.swift' (30/55)
Linting 'FileReader.swift' (31/55)
Linting 'Logger.swift' (32/55)
Linting 'Shell.swift' (33/55)
Linting 'LicensePlist.swift' (34/55)
/Users/mono/Git/LicensePlist/Sources/LicensePlistCore/Entity/FileReader/XcodeProjectFileReader.swift:10:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Linting 'ViewController.swift' (35/55)
Linting 'AppDelegate.swift' (36/55)
Linting 'SceneDelegate.swift' (37/55)
Linting 'URL.extensionTests.swift' (38/55)
Linting 'RepoRequestsTests.swift' (39/55)
Linting 'SearchRequestsTests.swift' (40/55)
Linting 'ConfigTests.swift' (41/55)
Linting 'GitHubLibraryConfigFileTests.swift' (42/55)
Linting 'LicensePlistHolderTests.swift' (43/55)
Linting 'PlistInfoTests.swift' (44/55)
Linting 'GitHubLibraryConfigFileTypeTests.swift' (45/55)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/SceneDelegate.swift:52:1: warning: Trailing Newline Violation: Files should have a single trailing newline. (trailing_newline)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/SceneDelegate.swift:19:19: warning: Unused Optional Binding Violation: Prefer `!= nil` over `let _ =` (unused_optional_binding)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/SceneDelegate.swift:14:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/SceneDelegate.swift:50:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/ViewController.swift:19:1: warning: Trailing Newline Violation: Files should have a single trailing newline. (trailing_newline)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/ViewController.swift:17:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Linting 'CocoaPodsTests.swift' (46/55)
Linting 'LicenseTests.swift' (47/55)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/AppDelegate.swift:36:1: warning: Trailing Newline Violation: Files should have a single trailing newline. (trailing_newline)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/AppDelegate.swift:14:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 3. (vertical_whitespace)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject/SwiftPackageManagerTestProject/AppDelegate.swift:34:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Linting 'SwiftPackageManagerTests.swift' (48/55)
Linting 'GitHubTests.swift' (49/55)
Linting 'GitHubLicense.collectorTests.swift' (50/55)
Linting 'VersionInfoTests.swift' (51/55)
Linting 'TestUtil.swift' (52/55)
Linting 'XcodeProjectFileReaderTests.swift' (53/55)
Linting 'SwiftPackageFileReaderTests.swift' (54/55)
Linting 'ResultOperationTests.swift' (55/55)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/TestUtil.swift:6:1: warning: Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/Entity/FileReader/XcodeProjectFileReaderTests.swift:45:5: warning: Function Body Length Violation: Function body should span 40 lines or less excluding comments and whitespace: currently spans 67 lines (function_body_length)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/Entity/FileReader/XcodeProjectFileReaderTests.swift:116:5: warning: Function Body Length Violation: Function body should span 40 lines or less excluding comments and whitespace: currently spans 76 lines (function_body_length)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/Entity/FileReader/XcodeProjectFileReaderTests.swift:196:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
/Users/mono/Git/LicensePlist/Tests/LicensePlistTests/Entity/FileReader/XcodeProjectFileReaderTests.swift:215:1: warning: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Done linting! Found 15 violations, 0 serious in 55 files.

As for SwiftPackageManagerTestProject, it can be ignored by specifying like this at swiftlint.yml.

excluded:
  - Tests/LicensePlistTests/XcodeProjects/SwiftPackageManagerTestProject

A real project is useful to test a function or object of reading xcodeproj file.

Prepare for testing.

Check whether current implementation implementation works or not.

yosshi4486 · 2021-04-08T08:53:25Z

@mono0926 Thanks for comments.
I exclude XcodeProjects directory from testTarget and lint in last two commits.
(and I rebase commits to remove unnecessary my xcuserdata directory)

yosshi4486 · 2021-04-08T08:56:25Z

Package.swift

+            name: "LicensePlistTests",
+            dependencies: ["LicensePlistCore"],
+            exclude: [
+                "XcodeProjects"


I could exclude the test xcode-projects directory by specifying it in Package.swift.

yosshi4486 · 2021-04-09T01:45:37Z

@mono0926 Thank you! I've fixed things you reviewed yesterday.

mono0926 · 2021-04-09T05:23:17Z

@yosshi4486
Thanks for your work 🙏

mono0926 requested changes Apr 8, 2021

View reviewed changes

yosshi4486 added 17 commits April 8, 2021 17:35

Add xcode project for testing.

45b3ffc

A real project is useful to test a function or object of reading xcodeproj file.

Add packages to test project.

5ede04d

Prepare for testing.

Add failed test.

78785a7

Check whether current implementation implementation works or not.

Test projectPath computed variable.

deb8421

Add document codes.

67ea3d2

Test package.resolved assertions.

8e4e16a

Add SwiftPackageFileReader and its tests.

d3db461

Refactor process method of LicensePlist type.

cf1cf90

Refactor test utils.

11b04fb

Add xcworkspace file by executing "pod install".

f161ff8

Test non updated Package.resolved

638f562

Refactor test things.

f423594

Fix spm gen problem.

ef39090

Add workaround document.

f3ce46b

Update README.

d0bb647

Exclude test projects directory from Package.swift

f7f12fa

Ignore xcodeprojects from lint.

90c2158

yosshi4486 commented Apr 8, 2021

View reviewed changes

yosshi4486 requested a review from mono0926 April 8, 2021 09:15

yosshi4486 added 2 commits April 8, 2021 19:28

Adopt lint suggestions.

7a55694

Refactor long assertion text to resource file.

0d3c023

mono0926 merged commit d9016ef into mono0926:master Apr 9, 2021

mono0926 linked an issue Apr 9, 2021 that may be closed by this pull request

Wrong Docs regarding Package.swift #140

Closed

mono0926 mentioned this pull request Apr 9, 2021

Wrong Docs regarding Package.swift #140

Closed

yosshi4486 deleted the fix-spm-licenses-gen branch April 9, 2021 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix spm licenses gen #150

Fix spm licenses gen #150

yosshi4486 commented Apr 6, 2021 •

edited

Loading

mono0926 commented Apr 7, 2021 •

edited

Loading

mono0926 left a comment

yosshi4486 commented Apr 8, 2021 •

edited

Loading

yosshi4486 Apr 8, 2021

yosshi4486 commented Apr 9, 2021

mono0926 commented Apr 9, 2021

Fix spm licenses gen #150

Fix spm licenses gen #150

Conversation

yosshi4486 commented Apr 6, 2021 • edited Loading

About

What happen?

Why does the problem occur?

Reason

How did you solve it?

How did you implement it?

Who encounter the problem?

Review Points

Pass tests?

Related issues?

Other things?

mono0926 commented Apr 7, 2021 • edited Loading

mono0926 left a comment

Choose a reason for hiding this comment

yosshi4486 commented Apr 8, 2021 • edited Loading

yosshi4486 Apr 8, 2021

Choose a reason for hiding this comment

yosshi4486 commented Apr 9, 2021

mono0926 commented Apr 9, 2021

yosshi4486 commented Apr 6, 2021 •

edited

Loading

mono0926 commented Apr 7, 2021 •

edited

Loading

yosshi4486 commented Apr 8, 2021 •

edited

Loading