Page MenuHomeSoftware Heritage

WIP: add permissions on edge labels
ClosedPublic

Authored by seirl on Sep 22 2020, 1:29 PM.

Diff Detail

Repository
rDGRPH Compressed graph representation
Branch
feature/permission-edge-labels
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15446
Build 23800: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 23799: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D4006 (id=14128)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

add AbstractSwhLabelList + FixedWidthSwhLabelList

Build is green

Patch application report for D4006 (id=14136)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

Use FixedWidthLongListLabel

Build has FAILED

Patch application report for D4006 (id=14143)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

Build has FAILED

Patch application report for D4006 (id=14145)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

seirl requested changes to this revision.Sep 22 2020, 5:37 PM

I would do a bit of refactoring:

  • Rename FixedWidthSwhLabelList -> SwhLabel
  • Rename SwhLabel as DirEntry and put it in SwhLabel (SwhLabel.DirEntry)
  • Move SwhPerm to SwhLabel.DirEntry.Permission

I added more inline comments inside.

java/src/main/java/org/softwareheritage/graph/SwhPerm.java
9 ↗(On Diff #14145)

Methods names aren't descriptive, and you're not passing octal, you're just passing integers all the time.

I suggest:

  • fromIntPerm() / toIntPerm()
  • fromIntCode() / toIntCode()
java/src/main/java/org/softwareheritage/graph/maps/LabelMapBuilder.java
111

This is no longer the case.

113

This will no longer be needed if you do what I suggested below.

123

I don't think we want to sort either the 3rd or the 4th field actually, so I would remove them both.

211–214

Can't we replace all of that by the toBitStream() method in SwhLabel?

This revision now requires changes to proceed.Sep 22 2020, 5:37 PM
java/src/main/java/org/softwareheritage/graph/SwhPerm.java
11 ↗(On Diff #14145)

Use .values().length

Build has FAILED

Patch application report for D4006 (id=14161)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit 137b4e5ccc565d4f4d99c6428e3625713f2557c2
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

haltode marked an inline comment as done.

Rename FixedWidthDirEntryList to SwhLabel.

Build has FAILED

Patch application report for D4006 (id=14163)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit defb68f6b34a351efcb50ab55c145ebe04532283
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

DirEntry: use Permission.Type instead of already encoded int.

Build has FAILED

Patch application report for D4006 (id=14171)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit e0c9194ed584b13103ff8c825cc9c602c7fde522
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

seirl requested changes to this revision.Sep 23 2020, 2:17 PM
seirl added inline comments.
java/src/main/java/org/softwareheritage/graph/labels/DirEntry.java
13

I would just have a constructor with int permission, but which takes the actual permission integer. No need to require the user to use that Permission.Type.

java/src/main/java/org/softwareheritage/graph/maps/LabelMapBuilder.java
112

Make that a static DirEntry.labelWidth(long numLabels) function (including the log2() call).

java/src/main/java/org/softwareheritage/graph/utils/ReadLabelledGraph.java
35

Just use label.permission here.

35–36

According to Seba, there is now a big version of PermutedFrontCodedStringList. We probably want to use that to be able to use more than 2**32 labels.

This revision now requires changes to proceed.Sep 23 2020, 2:17 PM

Make DirEntry.Permission private.

Build has FAILED

Patch application report for D4006 (id=14180)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit 1df951296ddc7a5adffa482b416fe0d8b0e36177
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

haltode marked an inline comment as done.

Replace PermutedFrontCodedStringList with -big version

Build has FAILED

Patch application report for D4006 (id=14181)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit 616c3a283b66fdfbce3668c604de45d5cb5ceba8
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 15:34:37 2020 +0200

    WIP: replace PermutedFrontCodedStringList with -big version

commit 1df951296ddc7a5adffa482b416fe0d8b0e36177
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

Use Long.SIZE instead of hardcoded 64.

Build has FAILED

Patch application report for D4006 (id=14182)

Rebasing onto 271274d4aa...

Current branch diff-target is up to date.
Changes applied before test
commit 5f1cc653a5c9c69e1084908742108ffaa6d98db6
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 15:34:37 2020 +0200

    WIP: replace PermutedFrontCodedStringList with -big version

commit 1df951296ddc7a5adffa482b416fe0d8b0e36177
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Sep 23 11:33:09 2020 +0200

    WIP: code refactoring

commit c3f0c650589c3f76309ae66dae41e8b94862fc3f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:07:12 2020 +0200

    WIP: code formatting

commit dbfbf4066e0567921a9f1791c13ec785e635bc58
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 15:03:12 2020 +0200

    WIP: use FixedWidthLongListLabel

commit dbb705fd75d27b11b253256820ae05366b7d45b9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 14:14:53 2020 +0200

    WIP: add AbstractSwhLabelList + FixedWidthSwhLabelList

commit ebd2c86d289ead21112cde440160ab567da4a49e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    WIP: add permissions on edge labels

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

This revision is now accepted and ready to land.Sep 29 2020, 3:40 PM
  • Squash commits
  • Add files missing from webgraph-big upstream

Build has FAILED

Patch application report for D4006 (id=17097)

Rebasing onto 5a987aae6e...

First, rewinding head to replay your work on top of it...
Applying: java: graph: add permissions on edge labels
Using index info to reconstruct a base tree...
M	java/pom.xml
M	java/src/main/java/org/softwareheritage/graph/maps/LabelMapBuilder.java
M	java/src/main/java/org/softwareheritage/graph/utils/ReadLabelledGraph.java
Falling back to patching base and 3-way merge...
Auto-merging java/src/main/java/org/softwareheritage/graph/utils/ReadLabelledGraph.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/utils/ReadLabelledGraph.java
Auto-merging java/src/main/java/org/softwareheritage/graph/maps/LabelMapBuilder.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/maps/LabelMapBuilder.java
Auto-merging java/pom.xml
CONFLICT (content): Merge conflict in java/pom.xml
Patch failed at 0001 java: graph: add permissions on edge labels

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!

Could not rebase; Attempt merge onto 5a987aae6e...

Already up to date.
Changes applied before test
commit f426c15116067309a92a0c1f765b14358d14f9b3
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Jan 8 16:27:29 2021 +0100

    java: labels: add files missing from webgraph-big usptream

commit c75a4bc8e9566f27c2eb249d214db6c904c34c36
Author: Thibault Allançon <haltode@gmail.com>
Date:   Tue Sep 22 12:01:10 2020 +0200

    java: graph: add permissions on edge labels

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

seirl edited reviewers, added: haltode; removed: seirl.
This revision now requires review to proceed.Jan 8 2021, 4:38 PM

Rebase on master, include webgraph files

Build is green

Patch application report for D4006 (id=17099)

Rebasing onto 5a987aae6e...

Current branch diff-target is up to date.
Changes applied before test
commit 92f810a36bc72b84a47301cebd1ef04c94b3cd2f
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Jan 8 16:33:33 2021 +0100

    Add permissions on edge labels

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

This revision was not accepted when it landed; it landed in state Needs Review.Mar 23 2021, 6:15 PM
Closed by commit rDGRPH92f810a36bc7: Add permissions on edge labels (authored by haltode, committed by seirl). · Explain Why
This revision was automatically updated to reflect the committed changes.