epics-modules / autosave

APS BCDA synApps module: autosave
https://epics-modules.github.io/autosave/
Other
8 stars 31 forks source link

set_savefile_path() value prepended when using relative path in set_pass0_restoreFile() #19

Closed gohierf closed 1 year ago

gohierf commented 6 years ago

Hello,

Today I wanted to specify a relative path in set_pass0_restoreFile(), but my path got prepended with the path I gave in set_savefile_path().

For me this issue can be solved either by treating relative path the same way as absolute path (ie without prepending set_savefile_path() path), or the documentation should be updated to specify that behaviour.

Here is the current relevant documentation section : "

  1. Specify restore files (optional, though this is the intended and most common use) Specify which save files are to be restored before record initialization (pass 0) and which are to be restored after record initialization (pass 1), using the commands set_pass_restoreFile(), as in this example:

    set_pass0_restoreFile("auto_settings.req", "P=xxx:") set_pass1_restoreFile("auto_settings.req", "P=xxx:")

(Note "macrostring" is optional, and a new feature of autosave 5.4.)

Place these commands in the startup file before iocInit. In versions earlier than 4.4, autosave would attempt to restore "auto_positions.sav" and "auto_settings.sav", if no restore files had been specified. Beginning with version 4.5, only files specified in calls to set_passn_restoreFile() are restored.

If you specify only a file name, not a path, to set_pass_restoreFile(), autosave will prepend the path specified in set_savefile_path(). This is the most common way to specify a restore file.

Beginning with autosave 5.4, if you specify a full path beginning with '/', autosave will not prepend anything to it. (Also, autosave will not write a restore-time backup of the file in this case.) "

I'm using autosave, 5.7.0.

Here is the code I used

# Specify directories in which to search for request files
set_requestfile_path("../misc/","")

# Specify where save files should be
set_savefile_path("/tmp/")

# Specify what save files should be restored
set_pass0_restoreFile("../misc/default_myIoc.sav")
set_pass0_restoreFile("myIoc.sav")

And the traces I get:

...
reboot_restore: entry for file '../misc/default_iocSkid.sav'
reboot_restore: Found filename '../misc/default_myIoc.sav' in restoreFileList.
*** restoring from '/tmp/../misc/default_myIoc.sav' at initHookState 6 (before record/device init) ***
save_restore: Can't open file '/tmp/../misc/default_myIoc.sav'.
save_restore: Trying backup file '/tmp/../misc/default_myIoc.savB'
save_restore: Can't open file '/tmp/../misc/default_myIoc.savB'.
save_restore: Can't figure out which seq file is most recent,
save_restore: so I'm just going to start with '/tmp/../misc/default_myIoc.sav0'.
save_restore: Trying backup file '/tmp/../misc/default_myIoc.sav0'
save_restore: Can't open file '/tmp/../misc/default_myIoc.sav0'.
save_restore: Trying backup file '/tmp/../misc/default_myIoc.sav1'
save_restore: Can't open file '/tmp/../misc/default_myIoc.sav1'.
save_restore: Trying backup file '/tmp/../misc/default_myIoc.sav2'
save_restore: Can't open file '/tmp/../misc/default_myIoc.sav2'.
save_restore: Can't find a file to restore from...save_restore: ...last tried '/tmp/../misc/default_myIoc.sav2'. I give up.
save_restore: **********************************
...

Regards, Francis

anjohnson commented 6 years ago

Hi Francis,

Autosave uses the rules documented because it can't be sure where the current directory will be set to at the time it uses the file-paths. Most OS's only have one "current directory" which applies to all threads in the process, thus whenever the IOC shell executes a cd command, any relative file paths stored by autosave would be pointing to a different location when it next needed to use them.

I don't think it would be safe for autosave to store the value returned from getcwd() at the time the file-name was specified and just pre-pend this to any relative paths, some versions of automount don't guarantee that will always work.

I agree that the wording you quoted could be clearer: "specify only a file name, not a path" doesn't really describe what it means very well, so I think the resolution of this issue is going to be to fix the documentation (which I hope someone in the APS BCDA group reading this will do...).

timmmooney commented 6 years ago

Thanks for commenting, Andrew. How does the following text sit with you guys?

"If you specify only a file name, not a path, to set_pass_restoreFile(), or if you specify a path that does not begin with '/', autosave will prepend the path specified in set_savefile_path()."

If you guys can think of a better way to say this, I'm open to it.

Tim Mooney (mooney@aps.anl.gov) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab


From: Andrew Johnson notifications@github.com Sent: Friday, June 1, 2018 11:40:20 AM To: epics-modules/autosave Cc: Subscribed Subject: Re: [epics-modules/autosave] set_savefile_path() value prepended when using relative path in set_pass0_restoreFile() (#19)

Hi Francis,

Autosave uses the rules documented because it can't be sure where the current directory will be set to at the time it uses the file-paths. Most OS's only have one "current directory" which applies to all threads in the process, thus whenever the IOC shell executes a cd command, any relative file paths stored by autosave would be pointing to a different location when it next needed to use them.

I don't think it would be safe for autosave to store the value returned from getcwd() at the time the file-name was specified and just pre-pend this to any relative paths, some versions of automount don't guarantee that will always work.

I agree that the wording you quoted could be clearer: "specify only a file name, not a path" doesn't really describe what it means very well, so I think the resolution of this issue is going to be to fix the documentation (which I hope someone in the APS BCDA group reading this will do...).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/autosave/issues/19#issuecomment-393938514, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGfnlwXqq6_qmUJg37m4rM10cdtU6jH-ks5t4W50gaJpZM4UWi6n.

anjohnson commented 6 years ago

I think it's the words "only a file name" that need changing, because they don't tell users what will happen if they specify something that contains a "/" in the middle of a string that doesn't start with a "/" character.

Isn't the behavior basically that any string that doesn't start with a "/" will get the savefile_path prepended to it, maybe with an intermediate "/" character? I haven't looked at the code to be sure.

gohierf commented 6 years ago

Indeed it's the "only a file name" that confused me. I'm used to say "absolute path" for path starting with a "/" and "relative path" otherwise. I would have phrased it something along : "Unless you provided an absolute path (ie. starting with an "/) to set_pass_restoreFile(), autosave will prepend the path specified in set_savefile_path()." Does that sounds right to you ? I haven't checked the code either.

timmmooney commented 6 years ago

I changed it. In the originally intended and still most common use, only a file name is specified. The documentation we're talking about was written to tell people that it was newly possible to specify either a file name or a path.

Tim Mooney (mooney@aps.anl.gov) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab


From: gohierf notifications@github.com Sent: Monday, June 4, 2018 1:35:56 PM To: epics-modules/autosave Cc: Mooney, Tim M.; Comment Subject: Re: [epics-modules/autosave] set_savefile_path() value prepended when using relative path in set_pass0_restoreFile() (#19)

Indeed it's the "only a file name" that confused me. I'm used to say "absolute path" for path starting with a "/" and "relative path" otherwise. I would have phrased it something along : "Unless you provided an absolute path (ie. starting with an "/), to set_pass_restoreFile(), autosave will prepend the path specified in set_savefile_path()." Does that sounds right to you ? I haven't checked the code either.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/autosave/issues/19#issuecomment-394455028, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGfnlyVW5Wwq4s1cOdha4DE-nZrj02A9ks5t5X4MgaJpZM4UWi6n.

gohierf commented 6 years ago

Yep sorry I hadn't check it's already perfect now.