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.

Sign up for GitHub

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

Jump to bottom

Stop matching pre-release suffixed next version #140

Merged
merged 3 commits into from
Feb 27, 2016
Merged

Stop matching pre-release suffixed next version #140

merged 3 commits into from
Feb 27, 2016

Conversation

norio-nomura
Copy link
Contributor

@norio-nomura norio-nomura mentioned this pull request Feb 22, 2016
Merged
@norio-nomura
Copy link
Contributor Author

Updated.

@mxcl
Copy link
Contributor

mxcl commented Feb 22, 2016

Thanks, though we need a test to prevent regression.

@norio-nomura
Copy link
Contributor Author

Rebased and added test.

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2016

@swift-ci Please Test

@kostiakoval
Copy link
Collaborator

@swift-ci is available for SwiftPM now? 😲 WoW

@aciidgh
Copy link
Member

aciidgh commented Feb 23, 2016

woah, can others trigger it or only admins?

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2016

Only Apple Members, the fear was:

  1. Blackhat submits PR
  2. Blackhat triggers CI on our servers
  3. Blackhat could potentially try to 0wn our servers.

@aciidgh
Copy link
Member

aciidgh commented Feb 23, 2016

I understand

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2016

But, seems broken. I have tested locally and for some reason this patch causes a failure in ManifestTests on Linux—but not Darwin.

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2016 

Test Case 'ManifestTests.testManifestLoading' started.
/home/mxcl/src/swiftpm/Tests/ManifestParser/ManifestTests.swift:40: error: ManifestTests.testManifestLoading : XCTAssertEqual failed: ("[PackageDescription.Package.Dependency]") is not equal to ("[PackageDescription.Package.Dependency]") - 
Test Case 'ManifestTests.testManifestLoading' failed (0.202 seconds).

@modocache
Copy link
Collaborator

Could it be that XCTAssertEqual itself behaves differently in swift-corelibs-xctest? 🤔

What are the types of the two inputs?

@shahmishal
Copy link
Member

@swift-ci Please Test

@shahmishal
Copy link
Member

Pull request testing system for swiftpm is working now.
Sorry for the trouble.

@norio-nomura
Copy link
Contributor Author

I hope this will be in next SNAPSHOT…

@norio-nomura
Copy link
Contributor Author

Oh, new SNAPSHOT has been released without this. 😢
What can I do for merging this?

@norio-nomura norio-nomura mentioned this pull request Feb 26, 2016
Merged
@norio-nomura
Copy link
Contributor Author

Rebased.

@mxcl
Copy link
Contributor

mxcl commented Feb 26, 2016

@norio-nomura sorry a test was failing, so I didn't merge. Let me try again.

@mxcl
Copy link
Contributor

mxcl commented Feb 26, 2016

Yes, I still get a failure on Linux, but I do not on Darwin, and I do not get this failure on Linux for master branch.

Test Case 'ManifestTests.testManifestLoading' started.
/home/mxcl/src/swiftpm/Tests/ManifestParser/ManifestTests.swift:40: error: ManifestTests.testManifestLoading : XCTAssertEqual failed: ("[PackageDescription.Package.Dependency]") is not equal to ("[PackageDescription.Package.Dependency]") - 
Test Case 'ManifestTests.testManifestLoading' failed (0.225 seconds).
Executed 1 test, with 1 failure (0 unexpected) in 0.225 (0.225) seconds

@aciidgh
Copy link
Member

aciidgh commented Feb 26, 2016

weird, it passes for me on Ubuntu 15.05

Test Case 'ManifestTests.testManifestLoading' started.
Test Case 'ManifestTests.testManifestLoading' passed (0.17 seconds).
Executed 1 test, with 0 failures (0 unexpected) in 0.17 (0.171) seconds

$ swiftc -v
Swift version 3.0-dev (LLVM a7663bb722, Clang 4ca3c7fa28, Swift 1c2f40e246)
Target: x86_64-unknown-linux-gnu

@norio-nomura unrelated but you also probably need to add PackageDescriptiontest.PackageTests() in LinuxMain.swift

@norio-nomura
Copy link
Contributor Author

Thanks all. 🙏
@aciidb0mb3r I added that!

@mxcl
Copy link
Contributor

mxcl commented Feb 26, 2016

@swift-ci Please Test

@norio-nomura
Copy link
Contributor Author

Yay! Test passed! 🙌

mxcl added a commit that referenced this pull request Feb 27, 2016
@mxcl
Merge pull request #140 from norio-nomura/fix-SR-787
e83af8b 
Stop matching pre-release suffixed next version
@mxcl mxcl merged commit e83af8b into apple:master Feb 27, 2016
@norio-nomura
Copy link
Contributor Author

Thanks! 🙏

@norio-nomura norio-nomura deleted the fix-SR-787 branch February 27, 2016 23:54
norio-nomura added a commit to norio-nomura/swift-package-manager that referenced this pull request Mar 7, 2016
@norio-nomura
Fix documentation on version specification examples of Package.swift
42750fc 
Related to apple#140
It should use `.max` on `minor` or `patch` with same `major` version for avoiding pre-release suffixed next version matches.
@norio-nomura norio-nomura mentioned this pull request Mar 7, 2016
Closed
aciidgh pushed a commit to aciidgh/swift-package-manager that referenced this pull request Jan 11, 2019
@ddunbar
Merge pull request apple#140 from ddunbar/llbuild-qos
de5d407 
[BuildSystem] Set a QoS class for build lanes.
@norio-nomura norio-nomura mentioned this pull request Feb 22, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@norio-nomura @mxcl @kostiakoval @aciidgh @modocache @shahmishal