Issue#520 explicit user group for module hide forbid tag #521

jdechard · 2024-01-15T13:35:34Z

Open PR to review code for explicit user/ group option in module-{hide,forbid,tag} command (see issue here..

Code review needed to not use the boolean flag.

jdechard · 2024-01-15T13:57:43Z

Hi Xavier,

PR done, sorry for the delay.

Thank you for your suggestion for appIyand isnearly. In fact this was my first try 2 weeks ago and it works well only for the user/ group used in the module-{hide,forbid,tag} command.

I use the following test:

user alan belonging to groupA
user bill belonging to groupB
a .modulerc file hiding the foo application with the different option set
The table below sums up what we should get:

`module-hide` option	bool `user` for `alan`	bool `not-user` for `alan`	wanted `apply` for `alan`	bool `user` for `bill`	bool `not-user` for `bill`	wanted `apply` for `bill`
`--user alan`	1	0	1	0	0	0
`--not-user alan`	0	1	1	0	0	0

And of course the same apply to the groupoption.
With:

set apply [expr {($user || $group || (!$notuser && !$notgroup)) &&\
   ($isbefore || $isafter || (![info exists before] && ![info exists\
   after]))}]

apply=1 all the time for bill due to the ORoperator between user/groupand notuser/notgroupbooleans. I tried different unique expressions without success so far... There is maybe something that I don't see here...

xdelaruelle · 2024-01-15T14:27:43Z

There is something missing in your message I think. I do not see the content of the `.modulerc` file to correctly understand your example. Le lun. 15 janv. 2024 à 14:57, Jérémy Déchard ***@***.***> a écrit :

…

Hi Xavier, PR done, sorry for the delay. Thank you for your suggestion for appIyand isnearly. In fact this was my first try 2 weeks ago and it works well only for the user/ group used in the module-{hide,forbid,tag} command. I use the following test: - user alan belonging to groupA - user bill belonging to groupB - a .modulerc file hiding the foo application with the different option set - The table below sums up what we should get: module-hide option bool user for alan bool not-user for alan wanted apply for alan bool user for bill bool not-user for bill wanted apply for bill --user alan 1 0 1 0 0 0 --not-user alan 0 1 1 0 0 0 And of course the same apply to the groupoption. With: set apply [expr {($user || $group || (!$notuser && !$notgroup)) &&\ ($isbefore || $isafter || (![info exists before] && ![info exists\ after]))}] apply=1 all the time for bill due to the ORoperator between user/groupand notuser/notgroupbooleans. I tried different unique expressions without success so far... There is maybe something that I don't see here... — Reply to this email directly, view it on GitHub <#521 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFTKVMRIGFSKLSKJ65E5RTYOUYWJAVCNFSM6AAAAABB3JI2PWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJSGIZDKOJYGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

jdechard · 2024-01-15T14:31:22Z

The .modulerc is trivial and contains only the different options:

#%Module
# Comment/Uncomment to test the different options

module-hide --user alan foo
#module-hide --not-user alan foo
#module-hide --group groupA foo
#module-hide --not-group groupA foo

xdelaruelle · 2024-01-16T03:56:05Z

With module-hide --not-user alan, it should not apply to alan but it should apply for bill. The table example should be updated this way:

`module-hide` option	bool `user` for `alan`	bool `not-user` for `alan`	wanted `apply` for `alan`	bool `user` for `bill`	bool `not-user` for `bill`	wanted `apply` for `bill`
`--user alan`	1	0	1	0	0	0
`--not-user alan`	0	1	`0`	0	0	`1`

If we split Tcl code a bit, it may be simpler to build the accurate apply expression.

    set user_or_group_target_defined [expr {[info exists user] || [info exists\
       group]}]
    set user_or_group_targeted [expr {$user || $group}]
    set user_or_group_excluded [expr {$notuser || $notgroup}]
    set time_frame_defined [expr {[info exists before] || [info exists after]}]
    set in_time_frame [expr {!$time_frame_defined || $isbefore || $isafter}]
                                                                         
    set apply [expr {$in_time_frame && ($user_or_group_targeted ||\
       (!$user_or_group_target_defined && !$user_or_group_excluded))}]

User or group exclusion should be skipped if a user or group target is defined.

This code seems to work with the few examples I have tried.

jdechard · 2024-01-22T10:45:48Z

Hello Xavier,

Thank you for the good catch and to break down the Tcl code!

From my test user_or_group_target_defined equals 1 even when it should not (see bold in the table).

`module-hide` option`	variables for `alan`	variables for `bill`
`--user alan`	user=1 notuser=0 group=0 notgroup=0 user_or_group_target_defined=1 user_or_group_targeted=1 user_or_group_excluded=0 time_frame_defined=0 in_time_frame=1 apply=1	user=0 notuser=0 group=0 notgroup=0 user_or_group_target_defined=1 user_or_group_targeted=0 user_or_group_excluded=0 time_frame_defined=0 in_time_frame=1 apply=0
`--not-user alan`	user=0 notuser=1 group=0 notgroup=0 user_or_group_target_defined=1 user_or_group_targeted=0 user_or_group_excluded=1 time_frame_defined=0 in_time_frame=1 apply=0	user=0 notuser=0 group=0 notgroup=0 user_or_group_target_defined=1 user_or_group_targeted=0 user_or_group_excluded=0 time_frame_defined=0 in_time_frame=1 apply=0 (wrong)

If I correct the user_or_group_target_defined value, the apply expression seems to work well!

I'll try to look into that (maybe a typo that I don't see yet) and find out a valid isnearly expression.

Regards,

xdelaruelle · 2024-01-30T04:59:21Z

Hello Jeremy,

I made an error on the definition of the user_or_group_target_defined variable. It should rely on userlist and grouplist instead of user and group variable.

    set user_or_group_target_defined [expr {[info exists userlist] || [info\
       exists grouplist]}]

To help clarify isnearly definition, a in_near_time_frame intermediate variable may be defined:

    set in_near_time_frame [expr {[info exists after] && !$isafter &&\              
       [getState clock_seconds] >= ($after - $nearsec)}]

I think isnearly could then be defined this way (but I have not tested it):

    set isnearly [expr {!$apply && ($user_or_group_targeted ||\                     
       (!$user_or_group_target_defined && !$user_or_group_excluded)) &&\            
       $in_near_time_frame}]

Regards,
Xavier

xdelaruelle · 2024-02-19T08:19:11Z

@jdechard I am taking over this pull request. I will finish tests and documentation soon, so this change will be part of 5.4

jdechard · 2024-02-19T09:14:16Z

Hello Xavier,

Sorry to not have answer sooner (little bit busy) , your suggestions seem to work well on my test with alan and bill but I did not tested completely all the time option possibilities.

Many thanks for you help :)

Regards

Add --group and --user options to module-hide, module-forbid and module-tag modulefile commands. When set, restriction or tag specifically applies to the list of user and/or to the list of group defined. Fixes envmodules#520

codecov · 2024-02-19T18:53:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (80c460d) 99.60% compared to head (91b4de7) 99.60%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #521   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files          14       14           
  Lines        5123     5123           
=======================================
  Hits         5103     5103           
  Misses         20       20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

xdelaruelle and others added 7 commits February 19, 2024 17:42

Clarify application criteria code for module-{hide,forbid,tag}

04a898e

Add --group/--user option to module-{hide,forbid,tag}

ac3c3a5

Add --group and --user options to module-hide, module-forbid and module-tag modulefile commands. When set, restriction or tag specifically applies to the list of user and/or to the list of group defined. Fixes envmodules#520

doc: desc. --user/--group options to module-{hide,forbid,tag}

1c4090e

ts: test --user/--group on module-hide

a700af8

ts: test --user/--group on module-tag

6fc101a

lint: add --user/--group options to syntaxdbs

df31dc8

ts: test --user/--group on module-forbid

91b4de7

xdelaruelle force-pushed the Issue#520_Explicit_user_group_for_module-hide_forbid_tag branch from 7a3ac8a to 91b4de7 Compare February 19, 2024 18:19

xdelaruelle merged commit 5b7245e into envmodules:main Feb 19, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue#520 explicit user group for module hide forbid tag #521

Issue#520 explicit user group for module hide forbid tag #521

jdechard commented Jan 15, 2024

jdechard commented Jan 15, 2024

xdelaruelle commented Jan 15, 2024 via email

jdechard commented Jan 15, 2024

xdelaruelle commented Jan 16, 2024

jdechard commented Jan 22, 2024 •

edited

Loading

xdelaruelle commented Jan 30, 2024

xdelaruelle commented Feb 19, 2024

jdechard commented Feb 19, 2024

codecov bot commented Feb 19, 2024 •

edited

Loading

Issue#520 explicit user group for module hide forbid tag #521

Issue#520 explicit user group for module hide forbid tag #521

Conversation

jdechard commented Jan 15, 2024

jdechard commented Jan 15, 2024

xdelaruelle commented Jan 15, 2024 via email

jdechard commented Jan 15, 2024

xdelaruelle commented Jan 16, 2024

jdechard commented Jan 22, 2024 • edited Loading

xdelaruelle commented Jan 30, 2024

xdelaruelle commented Feb 19, 2024

jdechard commented Feb 19, 2024

codecov bot commented Feb 19, 2024 • edited Loading

Codecov Report

jdechard commented Jan 22, 2024 •

edited

Loading

codecov bot commented Feb 19, 2024 •

edited

Loading