-
Notifications
You must be signed in to change notification settings - Fork 91
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 Properties VCPKG #855
Add Properties VCPKG #855
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
namespace Microsoft.ComponentDetection.Contracts.TypedComponent; | ||
namespace Microsoft.ComponentDetection.Contracts.TypedComponent; | ||
|
||
using System.Linq; | ||
using PackageUrl; | ||
|
||
public class VcpkgComponent : TypedComponent | ||
|
@@ -20,6 +21,11 @@ public VcpkgComponent(string spdxid, string name, string version, string triplet | |
this.Triplet = triplet; | ||
this.Description = description; | ||
this.DownloadLocation = downloadLocation; | ||
|
||
if (!string.IsNullOrEmpty(downloadLocation) && downloadLocation.ToLower().Contains("https://github.com/")) | ||
{ | ||
this.SetGitRepoProperties(); | ||
} | ||
} | ||
|
||
public string SPDXID { get; set; } | ||
|
@@ -36,6 +42,10 @@ public VcpkgComponent(string spdxid, string name, string version, string triplet | |
|
||
public int PortVersion { get; set; } | ||
|
||
public string GitRepositoryOwner { get; set; } | ||
|
||
public string GitRepositoryName { get; set; } | ||
|
||
public override ComponentType Type => ComponentType.Vcpkg; | ||
|
||
public override string Id | ||
|
@@ -71,4 +81,22 @@ public override PackageURL PackageUrl | |
} | ||
} | ||
} | ||
|
||
private void SetGitRepoProperties() | ||
{ | ||
/* example download locations | ||
* "git+https://github.com/leethomason/tinyxml2@9.0.0" | ||
* "git+https://github.com/Microsoft/vcpkg#ports/nlohmann-json" | ||
*/ | ||
var locationArr = this.DownloadLocation.Split('/'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. qq, if downloadlocation is null will the split work? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a null or empty check before this method gets called |
||
if (!string.IsNullOrEmpty(locationArr[2])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's do an array length check, in which we expect at least 4 fragments, if less than that this should be noop. Otherwise we can hit index out of range exceptions here. |
||
{ | ||
this.GitRepositoryOwner = locationArr[2]; | ||
} | ||
|
||
if (!string.IsNullOrEmpty(locationArr[3])) | ||
{ | ||
this.GitRepositoryName = locationArr[3].TakeWhile(ch => char.IsLetterOrDigit(ch)).ToString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this check may fail for repositories with valid names like |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect to see some tests to validate these new changes.