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

Issue #247, fix how custom fetchers are handled #281

Merged
merged 2 commits into from
Sep 24, 2018
Merged

Conversation

zlav
Copy link
Member

@zlav zlav commented Sep 24, 2018

Expected format of locator URL is:
/api/cli/custom+6214/git@github.com:zlav/pastport.git$SHAVALUE/latest_build
Output with NormalizeGitURL:
/api/cli/custom+6214/github.com/zlav/pastport$SHAVALUE/latest_build

@zlav zlav changed the title change the way custom fetchers are handled Issue #247, fix how custom fetchers are handled Sep 24, 2018
@elldritch
Copy link
Member

elldritch commented Sep 24, 2018

This problem looks like the other way around: we are correctly normalising on test but are not correctly normalising on upload. The github.com/user/project form is canonical.

The reasoning here is that GitHub repositories are sometimes cloned via the HTTPS link and sometimes via the SSH link, and these links should point to the same FOSSA project. This mix-up is common enough that we handle GitHub as a special case (and we should probably also handle BitBucket and GitLab).

If we "fix" this correctly (by normalising on upload), we'll break existing projects in a minor way. New uploads to the projects that use the HTTPS locator will instead use the canonical locator. This might cause project-ignored issues to reappear.

A couple approaches to this issue:

  1. Fix this correctly, and migrate HTTPS custom locator projects in the backend to canonical locators. Users running old versions of the CLI will still experience confusion (their CLI will still upload to the HTTPS locator).
  2. Fix this correctly, and increment the configuration file version. On older configuration file versions, preserve the existing upload behaviour. That said, ephemeral .fossa.yml configurations are a common pattern in CI, which means configuration file version may not be a reliable indicator of the CLI version that the project was first uploaded with.
  3. Do not normalise at all (which is what this PR does), which can cause inadvertent mix-ups.
  4. Fix this correctly, then add logic for backwards compatibility with older projects. Specifically, uploading and testing should use the exact locator when that project already exists and the canonical locator otherwise.
  5. Fix this correctly, and don't do any data migration.

(1), (2), (4), and (5) are likely to cause confusion. I'm leaning towards either (2) or (3).

Copy link
Member

@elldritch elldritch left a comment

Choose a reason for hiding this comment

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

As we discussed offline, this is good to merge. The expectation is a follow-up PR that refactors Locator.String and ensures there are no locator modifications elsewhere.

@zlav zlav merged commit 4c9206d into master Sep 24, 2018
zlav added a commit that referenced this pull request Sep 24, 2018
meghfossa pushed a commit that referenced this pull request Nov 12, 2021
* config file docs

* VCS updates

* add filtering docs
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

2 participants