Page MenuHomePhabricator
Create Task
Maniphest T11082

Ordering of clone URIs incorrectly sorts less-preferred ID-based URIs above more-preferred callsign-based URIs
Closed, ResolvedPublic
Actions

Description

I am not sure what is unclear but here are the steps to reproduce:

  1. Create Build Plan in harbourmaster with 1 step.
  2. The Step should be of type "Make HTTP Request"
  3. Set URI https://<jenkins server>/mercurial/notifyCommit?url=${repository.uri}
  4. Check in something into repo with CALLSIGN REPO
  5. Observe Build plan reports the following: "No Mercurial jobs found using repository: https://<phabricator server>/diffusion/62/repo/"

The reason is that ${repository.uri} is returning https://<phabricator server>/diffusion/62/repo/ insteand of https://<phabricator server>/diffusion/REPO/repo/

T10366 has broken our use of Harbormaster.
We are using Jenkins as our continous integration server and mercurial as our VCS.
We have a build plan that consists of one step "Make HTTP request".
The URI of this build step is set to
https://<jenkins server>/mercurial/notifyCommit?url=${repository.uri}

Unfortunately the bug in question has changed what the variable ${repository.uri} resolves to. It no longer resolves to the correct URI but instead is resolving to some internal hidden version of the uri.

This causes jenkins to say that there are no jenkins jobs that correspond to the uri in question.
I suggest one of several possible solutions:

  1. Make ${repository.uri} go back to previous behaviour (e.g. don't return the internal hidden uri but the public visible one)
  2. Add some other variable(s) that will return the publicly visible uri so that I can restore CI functionality.

Revisions and Commits

rP Phabricator
D16008rP24acac117b9b Consider identifier types when sorting clone URIs

Related Objects

Event Timeline

nikolay.metchev created this task.Jun 2 2016, 1:14 PM
epriestley added a subscriber: epriestley.Jun 2 2016, 1:23 PM

Thanks for the report!

Unfortunately, I can't fix this problem because I'm not sure how to reproduce it. It's important that bug reports include complete reproduction steps (see Providing Reproduction Steps for help).

We attempt to make this as clear as possible in the bug report flow because when users submit partial bug reports that we can't act on it's frustrating for everyone. If there's anything we could do to make it more clear that we require reproduction steps, let us know what you found confusing.

The expectation is that you'll see these instructions when reporting a bug, which we hope make it clear what information MUST be included in bug reports:

must.png (747×1 px, 87 KB)

Next steps:

  • update this report with all the required information; or
  • we'll close this report after a few days if the required information isn't provided.
nikolay.metchev updated the task description. (Show Details)Jun 2 2016, 1:30 PM
epriestley added a comment.Jun 2 2016, 1:35 PM

Thanks, those specifics are helpful.

Although the /62/ version of the repository URI is less preferred than the /CALLSIGN/ version, it should still work fine as a clone URI. The "No Mercurial jobs: ..." error looks like it's coming from Jenkins.

  • Does the /62/ URI not actually work (i.e., does hg clone <uri> not clone)? The expectation is that it works properly and just isn't as user-friendly as the /CALLSIGN/ version.
  • Are you hard-coding some kind of repository identification on the Jenkins side based on the URI? Repository URIs are inherently somewhat mutable (for example, callsigns can now be edited, and you might want to change protocols in the future). If you're using the URI as an identifier, you might want to consider using repository.phid as an identifier instead -- that identifier is immutable and stable.
nikolay.metchev added a comment.Jun 2 2016, 1:41 PM

The /62/ version of the URI's is something new. All the jobs we created in jenkins have been created with the /CALLSIGN/ version of the URIs. The mercurial plugin for jenkins is most likely looking for the URI property of the jenkins job and matching on that.

So cloning with the hidden/new URI works but that is not what is going on here.

See https://wiki.jenkins-ci.org/display/JENKINS/Mercurial+Plugin

epriestley added a revision: D16008: Consider identifier types when sorting clone URIs.Jun 2 2016, 1:44 PM
epriestley renamed this task from Harbourmaster api non-backward compatible to Ordering of clone URIs incorrectly sorts less-preferred ID-based URIs above more-preferred callsign-based URIs.Jun 2 2016, 1:55 PM
epriestley claimed this task.
epriestley triaged this task as Normal priority.
epriestley added a project: Diffusion.
Herald added a subscriber: faulconbridge. · View Herald TranscriptJun 2 2016, 1:55 PM
epriestley closed this task as Resolved by committing rP24acac117b9b: Consider identifier types when sorting clone URIs.Jun 2 2016, 1:57 PM
epriestley added a commit: rP24acac117b9b: Consider identifier types when sorting clone URIs.
epriestley added a comment.Jun 2 2016, 2:03 PM

Thanks for the report! This should be resolved at HEAD of master, and promote to stable in about 48 hours.

Specifically, Phabricator should now properly prefer /CALLSIGN/ URIs over /123/ URIs when choosing a single clone URI for a repository.

Because repositories have multiple URIs and URIs are mutable, using URIs as a primary identifier is inherently somewhat fragile. You could improve the resilience of your build pipeline by using repository.phid instead (an immutable, stable identifier) and then calling diffusion.repository.search to lookup available URIs and select the preferred URI. Here's an example of the data you get when retrieving all URIs for a repository:

...
"uris": [
  {
    "id": "287",
    "type": "RURI",
    "phid": "PHID-RURI-ua4lktch7wfflxgrunfd",
    "fields": {
      "repositoryPHID": "PHID-REPO-ndllsh4rrlquzkryrnpz",
      "uri": {
        "raw": "git://git.rtems.org/rtems.git",
        "display": "git://git.rtems.org/rtems.git",
        "effective": "git://git.rtems.org/rtems.git",
        "normalized": "git.rtems.org/rtems"
      },
      "io": {
        "raw": "observe",
        "default": "none",
        "effective": "observe"
      },
      "display": {
        "raw": "default",
        "default": "never",
        "effective": "never"
      },
      "credentialPHID": "PHID-CDTL-qpakfvnwhh56yofyfkyq",
      "disabled": false,
      "builtin": {
        "protocol": null,
        "identifier": null
      },
      "dateCreated": "1462315042",
      "dateModified": "1462315042"
    }
  },
  {
    "id": "286",
    "type": "RURI",
    "phid": "PHID-RURI-2wc5w6aeaog7pjyowdc7",
    "fields": {
      "repositoryPHID": "PHID-REPO-ndllsh4rrlquzkryrnpz",
      "uri": {
        "raw": "http://id",
        "display": "http://local.phacility.com/diffusion/20/rtems.git",
        "effective": "http://local.phacility.com/diffusion/20/rtems.git",
        "normalized": "local.phacility.com/diffusion/20"
      },
      "io": {
        "raw": "default",
        "default": "read",
        "effective": "read"
      },
      "display": {
        "raw": "default",
        "default": "never",
        "effective": "never"
      },
      "credentialPHID": null,
      "disabled": false,
      "builtin": {
        "protocol": "http",
        "identifier": "id"
      },
      "dateCreated": "1462315042",
      "dateModified": "1462315042"
    }
  },
  {
    "id": "284",
    "type": "RURI",
    "phid": "PHID-RURI-ixjqrf5djm57quj5bako",
    "fields": {
      "repositoryPHID": "PHID-REPO-ndllsh4rrlquzkryrnpz",
      "uri": {
        "raw": "http://callsign",
        "display": "http://local.phacility.com/diffusion/RTEMSYS/rtems.git",
        "effective": "http://local.phacility.com/diffusion/RTEMSYS/rtems.git",
        "normalized": "local.phacility.com/diffusion/RTEMSYS"
      },
      "io": {
        "raw": "default",
        "default": "read",
        "effective": "read"
      },
      "display": {
        "raw": "default",
        "default": "always",
        "effective": "always"
      },
      "credentialPHID": null,
      "disabled": false,
      "builtin": {
        "protocol": "http",
        "identifier": "callsign"
      },
      "dateCreated": "1462315042",
      "dateModified": "1462315042"
    }
  },
  {
    "id": "280",
    "type": "RURI",
    "phid": "PHID-RURI-dis3yutcznaxtmuzwvrw",
    "fields": {
      "repositoryPHID": "PHID-REPO-ndllsh4rrlquzkryrnpz",
      "uri": {
        "raw": "ssh://id",
        "display": "ssh://local@localvault.phacility.com/diffusion/20/rtems.git",
        "effective": "ssh://local@localvault.phacility.com/diffusion/20/rtems.git",
        "normalized": "localvault.phacility.com/diffusion/20"
      },
      "io": {
        "raw": "default",
        "default": "read",
        "effective": "read"
      },
      "display": {
        "raw": "default",
        "default": "never",
        "effective": "never"
      },
      "credentialPHID": null,
      "disabled": false,
      "builtin": {
        "protocol": "ssh",
        "identifier": "id"
      },
      "dateCreated": "1462315042",
      "dateModified": "1462315042"
    }
  },
  {
    "id": "278",
    "type": "RURI",
    "phid": "PHID-RURI-ighooncihzavejfayonl",
    "fields": {
      "repositoryPHID": "PHID-REPO-ndllsh4rrlquzkryrnpz",
      "uri": {
        "raw": "ssh://callsign",
        "display": "ssh://local@localvault.phacility.com/diffusion/RTEMSYS/rtems.git",
        "effective": "ssh://local@localvault.phacility.com/diffusion/RTEMSYS/rtems.git",
        "normalized": "localvault.phacility.com/diffusion/RTEMSYS"
      },
      "io": {
        "raw": "default",
        "default": "read",
        "effective": "read"
      },
      "display": {
        "raw": "default",
        "default": "always",
        "effective": "always"
      },
      "credentialPHID": null,
      "disabled": false,
      "builtin": {
        "protocol": "ssh",
        "identifier": "callsign"
      },
      "dateCreated": "1462315042",
      "dateModified": "1462315042"
    }
  }
]
...
nikolay.metchev added a comment.Jun 2 2016, 2:16 PM

Thanks for the quick turnaround. I'll see if the problem is fixed in 48 hours and let you know.

Phabricator · Built by Phacility