Merge lp://staging/~ken-vandine/ubuntu-system-settings/lp1524424 into lp://staging/ubuntu-system-settings

Proposed by Ken VanDine
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 1577
Merged at revision: 1600
Proposed branch: lp://staging/~ken-vandine/ubuntu-system-settings/lp1524424
Merge into: lp://staging/ubuntu-system-settings
Diff against target: 11 lines (+1/-0)
1 file modified
plugins/about/Storage.qml (+1/-0)
To merge this branch: bzr merge lp://staging/~ken-vandine/ubuntu-system-settings/lp1524424
Reviewer Review Type Date Requested Status
Iain Lane (community) Needs Information
Sebastien Bacher (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+280196@code.staging.launchpad.net

Commit message

Subtract amount used by apps when computing otherSize

Description of the change

Subtract amount used by apps when computing otherSize

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

thanks, that's similar to the change I tried yesterday and it seems to do the job

review: Approve
Revision history for this message
Iain Lane (laney) wrote :

Are you sure that these calculations are right now?

https://bazaar.launchpad.net/~ken-vandine/ubuntu-system-settings/lp1524424/view/head:/plugins/about/Storage.qml#L67

otherSize subtracts things which are already in usedByUbuntu, so you are counting them twice.

(I guess you also double count movies / pictures / audio if they are in home too.)

review: Needs Information
Revision history for this message
Sebastien Bacher (seb128) wrote :

@Laney, I don't think you count twice

ubu = ds - fs - hs -cs

so the change does

os = ds - fs - (ds - fs - hs - cs) - cs - ms - ps - as

so the ds/fs/cs compensate each others and are not duplicated

Revision history for this message
Iain Lane (laney) wrote :

On Fri, Dec 11, 2015 at 12:42:13PM -0000, Sebastien Bacher wrote:
> @Laney, I don't think you count twice
>
> ubu = ds - fs - hs -cs
>
> so the change does
>
> os = ds - fs - (ds - fs - hs - cs) - cs - ms - ps - as
>
> so the ds/fs/cs compensate each others and are not duplicated

If you want "other" to be "homesize - movies - pictures - audio" then
IMO it would be better to say that in the code and not rely on
cancelling out of things which are counted elsewhere.

I think it probably wants to be "homesize - movies - pictures - audio -
totalClickSize" though, right? Because the clicks are counted as "Used
by apps" in their own right, and currently (with this branch) because
totalClickSize is included in the calculation for both "Used by Ubuntu"
and "Other files" it is classed as consumed space in both "Other files"
and "Used by apps".

BTW don't we need to add the sizes of all of the click packages' data
directories to "Used by apps" too (and subtract it from "Other files)?
Or does 'installed-size' count that? I thought it was just the size of
the data in the click itself.

Cheers,

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Revision history for this message
Sebastien Bacher (seb128) wrote :

> I think it probably wants to be "homesize - movies - pictures - audio - totalClickSize" though, right?

Unsure, I though that click were stored out of the userdir, so it would be wrong to remove them from that one?

> BTW don't we need to add the sizes of all of the click packages' data
> directories to "Used by apps" too (and subtract it from "Other files)?

no, it's a known bug/not-done-yet to count datas, I'm unsure how to do that with the xdg dirs, count .config|local|cache/<clickid>? what about logs, etc? but that's getting out of topic for this specific change

Revision history for this message
Iain Lane (laney) wrote :

On Mon, Dec 14, 2015 at 12:23:13PM -0000, Sebastien Bacher wrote:
> > I think it probably wants to be "homesize - movies - pictures - audio - totalClickSize" though, right?
>
> Unsure, I though that click were stored out of the userdir, so it would be wrong to remove them from that one?

I thought they were in there. But if they are not (/opt/click.ubuntu.com
rings a bell), then this change is probably actually okay as long as you
count user clicks only (otherwise you count system stuff again).

In any event I think it would be better to be more explicit about the
calculations instead of cancelling out parts of "Used by Ubuntu".

> > BTW don't we need to add the sizes of all of the click packages' data
> > directories to "Used by apps" too (and subtract it from "Other files)?
>
> no, it's a known bug/not-done-yet to count datas, I'm unsure how to do that with the xdg dirs, count .config|local|cache/<clickid>? what about logs, etc? but that's getting out of topic for this specific change

How do applications know where to save data to? They write to a well
known directory under XDG_{CACHE,CONFIG,DATA}_HOME, so use that. Count
the logs and other stuff (if that actually does happen) as in use by the
system if it's too hard to do it any other way.

From https://wiki.ubuntu.com/SecurityTeam/Specifications/ApplicationConfinement:

  Applications will always have read access to their install directory
  and have write access to directories they own as determined by the XDG
  base directory specification. Specifically:
  XDG_CACHE_HOME/<APP_PKGNAME>, XDG_RUNTIME_DIR/<APP_PKGNAME>,
  XDG_RUNTIME_DIR/confined/<APP_PKGNAME> (for TMPDIR),
  XDG_DATA_HOME/<APP_PKGNAME> and XDG_CONFIG_HOME/<APP_PKGNAME>. Apps
  can discover these paths using standard APIs and appending the package
  name as defined by the 'name' field of the manifest.

Yes it is out of scope, that's why it was "BTW". I'm just suggesting
things to make the software better.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches