cea-hpc / modules

Environment Modules: provides dynamic modification of a user's environment
http://modules.sourceforge.net/
GNU General Public License v2.0
695 stars 109 forks source link

Incorrect module modification time #458

Closed baruchgu closed 2 years ago

baruchgu commented 2 years ago

Describe the bug

The Last modification data of the "module avail -l" command is reported incorrectly when the modulefile is a symbolic link.

To Reproduce

Steps to reproduce the behavior:

Bug: The modification time belongs to the .stratus file.

~/projects [260]> ls -ltA modules/stratus/
-rw-r--r--   1 xx 775 Apr  5 11:03 .stratus
lrwxrwxrwx 1 xx   8 Mar  9 17:21 21.23.100 -> .stratus
lrwxrwxrwx 1 xx   8 Dec 27 18:39 21.20.100 -> .stratus
lrwxrwxrwx 1 xx   8 Sep 30  2021 21.13.100 -> .stratus

$ module use modules/stratus/
$ module avail -C stratus -l
- Package/Alias -----------------------.- Versions --------.- Last mod. -------
modules:
stratus/21.13.100                                           2022/04/05 11:03:34
stratus/21.20.100                                           2022/04/05 11:03:34
stratus/21.23.100                                           2022/04/05 11:03:34

Expected behavior

I expect to see the last mod. time of modulefile itself

$ module avail -C stratus -l
- Package/Alias -----------------------.- Versions --------.- Last mod. -------
modules:
stratus/21.13.100                                           2021/09/30 20:26:55
stratus/21.20.100                                           2021/12/27 18:39:19
stratus/21.23.100                                           2022/03/09 17:21:45

Modules version and configuration

$ module --version
Modules Release 5.1.0 (2022-04-30)
$ module config --dump-state

Additional context

I fixed the bug in my local copy of modulecmd.tcl file, line 9156 5.1.0 variant:

set mtime [file mtime $fpath]

My variant:

set mtime [file lstat $fpath larr; set larr(mtime)]

The fix was tested this way:

tcl/modulecmd.tcl tcsh avail -C stratus --long
xdelaruelle commented 2 years ago

The modification time reported is the one of the file behind the symbolic link as it is the content of this file which is evaluated.

If the .stratus file is updated after the symbolic link creation, it is important to show users the modification time of the .stratus file. Otherwise users will only see the creation time of the symbolic link which does not reflect that the content of the modulefile has changed.

xdelaruelle commented 2 years ago

I am closing this issue as the current behavior of Modules appears to be consistent regarding symbolic link and modification time report.

Many thanks for your report.

baruchgu commented 2 years ago

Dear. I see your concept. But from my point of view the expected behavior is as was described in the ticket. My users and I want to see the date of the new version of stratus But now we see all versions with the same date, when I have five or more stratus versions since 2015, all are seen with the same old date. Is there an option to modify the date behavior format by a flag/variable? I personally changed the TCL code in my installation.

-- תודה מראש, ברוך

On Sun, 29 May 2022 at 10:03, Xavier Delaruelle @.***> wrote:

I am closing this issue as the current behavior of Modules appears to be consistent regarding symbolic link and modification time report.

Many thanks for your report.

— Reply to this email directly, view it on GitHub https://github.com/cea-hpc/modules/issues/458#issuecomment-1140390593, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQCC2CVBKOGHQEKYBZNO4F3VMMJCTANCNFSM5W4PYPKA . You are receiving this because you authored the thread.Message ID: @.***>

xdelaruelle commented 2 years ago

You can use the siteconfig.tcl configuration file, to supersede the upstream version of the getFileMtime procedure with your own flavor.

You can set the following code in siteconfig.tcl:

rename ::getFileMtime {}
# get file modification time, cache it at first query, use cache afterward
proc getFileMtime {fpath} {
   if {[info exists ::g_fileMtime($fpath)]} {
      return $::g_fileMtime($fpath)
   } else {
      # protect 'file mtime' call in case we do not know what we are checking
      # when mcookie is not checked
      if {[catch {
         set mtime [file lstat $fpath larr; set larr(mtime)]
      } errMsg ]} {
         reportError [parseAccessIssue $fpath]
         set mtime {}
      }
      return [set ::g_fileMtime($fpath) $mtime]
   }
}