search

ppd / freecad-ppd

5 stars 1 forks source link

Absolute paths become inaccessible when snap revisions get garbage collected #27

Open luzpaz opened 2 years ago

luzpaz commented 2 years ago 
18:39:57  Line Group File: /snap/freecad-ppd/214/usr/share/Mod/TechDraw/LineGroup/LineGroup.csv is not readable
18:39:57  Svg Hatch File: /snap/freecad-ppd/107/usr/share/Mod/TechDraw/Patterns/simple.svg is not readable
18:39:57  Pat Hatch File: /snap/freecad-ppd/107/usr/share/Mod/TechDraw/PAT/FCPAT.pat is not readable

Demo: test(1).FCStd.zip

OS: Ubuntu Core 20 (KDE/plasma)
Word size of FreeCAD: 64-bit
Version: 0.20.28580 (Git)
Build type: Release
Branch: master
Hash: 8ccaac7264742da9e58a8238cc4d9d973bb57e98
Python 3.8.10, Qt 5.15.3, Coin 4.0.0, OCC 7.6.1
Locale: English/United States (en_US)
Installed mods: 
  * ToolbarStyle
  * Assembly4 0.11.10
  * stickframe
  * sheetmetal 0.2.49
  * pinger
  * Assembly3 0.11.1
  * Behave-Dark-Colors 0.0.1
  * Curves 0.3.0
  * Ship
  * BIM 2021.12.0
  * A2plus 0.4.55
ppd commented 2 years ago

I open your file, and there are no error messages. Everything seems to work. Your mix of revision numbers, 214 and 107, indicates that those paths might be cached somewhere. When the snap updates, old revisions are garbage collected at some point, and the files are not accessible any more in this exact path.

Thinking further about this, I'm noticing those paths in the model: image

It references files in absolute paths. Those paths are obviously only valid for some installations (not AppImage, not snap, though). To make the format self-contained, it ships the used patterns (Svg Included, Pat Included), as far as my understanding goes. It might be that it first tries to access those paths when loading the FCStd, which might fail if the original snap revision does not exist any more. In that case, this would be rather benign.

If you want to investigate, find out how TechDraw constructs the path to the patterns when you assign a new pattern. If you could get it to switch to /snap/freecad-ppd/current instead of /snap/freecad-ppd/${revision_number}, you'd have the same behaviour as in native packages.

luzpaz commented 2 years ago

Interesting since @wandererfan recently opened a thread and ticket (https://github.com/FreeCAD/FreeCAD/issues/6698) regarding this file.

WandererFan commented 2 years ago

The filenames for the "included" files come from App::PropertyFileIncluded::getExchangeTempFile().

Not sure that is related to #6698 at all. That ticket is about graphic display errors.

luzpaz commented 2 years ago

@WandererFan I was thinking (in my naivete) that there could be a relationship. I also used the test file from the ticket. But I'm probably way off... :stuck_out_tongue_winking_eye:

ppd commented 2 years ago

As this is not a snap-specific problem, or rather no real problem at all, I'm going to close this issue.

luzpaz commented 2 years ago

Pardon for being off-topic of the Snap but I wonder if @WandererFan would consider a way to indicate that the log message indicate nominal behavior to the user in a way where it's saying: "hey, found this anomaly...it's not big deal though and not abnormal, I'm working around it" ?

Or am I being too pendantic?

WandererFan commented 2 years ago

If PropertyFileIncluded is looking in the wrong place for the file that is part of the FCStd file, then that seems like a problem.

If you are sharing an FCStd file with somebody that doesn't have a "/usr/share/freecad/..." (like a Win user) to fall back on, then there will be an issue.

luzpaz commented 2 years ago

If PropertyFileIncluded is looking in the wrong place for the file that is part of the FCStd file, then that seems like a problem.

If you are sharing an FCStd file with somebody that doesn't have a "/usr/share/freecad/..." (like a Win user) to fall back on, then there will be an issue.

@WandererFan That's what I thought, but yes it's a whole separate issue, isn't it? Then do we need logic in TechDraw to deal with this?

WandererFan commented 2 years ago

Assuming TD is using PropertyFileIncluded properly (???), then the logic for flatpack/appimage/snap versions needs to be in PropertyFileIncluded. TD can't be the only place PropertyFileIncluded is used.

luzpaz commented 2 years ago

@WandererFan please advise. Should we open a forum thread moving forward?

WandererFan commented 2 years ago

Yes, I suppose so. "PropertyFileIncluded points to wrong location in snap versions"?

On Sat, Apr 2, 2022 at 1:50 PM luzpaz @.***> wrote:

@WandererFan https://github.com/WandererFan please advise. Should we open a forum thread moving forward?

— Reply to this email directly, view it on GitHub https://github.com/ppd/freecad-ppd/issues/27#issuecomment-1086689342, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3LYWIGK2ZNGKSNOT237H3VDCCEVANCNFSM5SJ6Q36A . You are receiving this because you were mentioned.Message ID: @.***>

ppd commented 2 years ago

Let's keep it open in case the upstream issue gets resolved. We can react to that change in the snap, if possible.