Page MenuHomeSoftware Heritage

Add missing `content_git_object`
ClosedPublic

Authored by Ericson2314 on Apr 26 2022, 7:42 AM.

Details

Summary

Add missing content_git_object

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Diff Detail

Repository
rDMOD Data model
Branch
arcpatch-D7653 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28858
Build 45100: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 45099: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D7653 (id=27689)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit 3bc6f9f4c1303e7e12d45e8fb5c95054d541e0fc
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: plt-amy, zack, anlambert, vlorentz

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/472/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/472/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:43 AM
Harbormaster failed remote builds in B28746: Diff 27689!

Build has FAILED

Patch application report for D7653 (id=27690)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit fd7c5e10e5587b304cec2f9f2a19bfd55b7d0894
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/473/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/473/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:48 AM
Harbormaster failed remote builds in B28747: Diff 27690!

self -> content, model.MissingData

Build has FAILED

Patch application report for D7653 (id=27691)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit efd5a9e521bfc267b5af4a93e57b9337bd5cfb50
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/474/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/474/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:50 AM
Harbormaster failed remote builds in B28748: Diff 27691!

Build has FAILED

Patch application report for D7653 (id=27692)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit c1a50cc7bcdccfb3dca6d497b996c66e67331e1b
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/475/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/475/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:53 AM
Harbormaster failed remote builds in B28749: Diff 27692!

HashableObject is for objects with an id, which is their only hash. This makes sense for most objects, because they only have one hash algorithm we care about
For Content, we do care about the other hash, and sha1_git is not more important than the others.

You should skip testing content.test(), and only test content_git_object directly

Test content_git_object directly

Build has FAILED

Patch application report for D7653 (id=27798)

Rebasing onto 2c0701cb77...

First, rewinding head to replay your work on top of it...
Applying: Add missing `content_git_object`
Changes applied before test
commit 277764247ea3e1f3af3be4be8d9f999dcf8b615b
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/477/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/477/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:36 PM
Harbormaster failed remote builds in B28858: Diff 27798!

Build has FAILED

Patch application report for D7653 (id=27799)

Rebasing onto 2c0701cb77...

First, rewinding head to replay your work on top of it...
Applying: Add missing `content_git_object`
Changes applied before test
commit b039e2d750b724368aa812b319755ff2344f6d7e
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/478/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/478/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:37 PM
Harbormaster failed remote builds in B28859: Diff 27799!

Build has FAILED

Patch application report for D7653 (id=27800)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 57c66993b6e7587b9f92b7007aab44c6719cd9b2
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/479/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/479/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:42 PM
Harbormaster failed remote builds in B28860: Diff 27800!

Need to hash raw object to compare!

Build is green

Patch application report for D7653 (id=27801)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 524f2324690a9459d842bcffa3991c52572cd1d0
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/480/ for more details.

Thanks @vlorentz for all your advice and help. I think this is good now!

Thanks.

One last thing for this diff: could you clean up the commit message? It should be this:

Add missing `content_git_object`

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

@vlorentz do you mean repeat the title in the summary field?

(I suppose https://docs.softwareheritage.org/devel/contributing/phabricator.html#starting-a-new-feature-and-submit-it-for-review did say that but I was bit confused as it seemed like the title might appear twice in the final commit.)

No, remove the fields. ie. replace all of this:

Add missing `content_git_object`

Summary:
This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Reviewers: #reviewers

Subscribers: vlorentz, anlambert, zack, plt-amy

Differential Revision: https://forge.softwareheritage.org/D7653

with just this:

Add missing `content_git_object`

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Summary/Test Plan/Reviewers/Subscribers/... is only the diff's message, not the commit's

Use git-style not phab-style commit message

Build is green

Patch application report for D7653 (id=27854)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 08c69e63794a6d7a839f7201ee957c223f65f898
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/481/ for more details.

Thanks! Now you can push to master

This revision is now accepted and ready to land.Apr 27 2022, 6:23 PM
This revision was automatically updated to reflect the committed changes.