eprints / eprints3.4

EPrints 3.4 core and releases
http://www.eprints.org/uk/index.php/eprints-3-4/
GNU Lesser General Public License v3.0
31 stars 28 forks source link

epadmin warning flags with update to Ubuntu 24.04 #403

Closed blairfix closed 1 month ago

blairfix commented 1 month ago

When running epadmin update on an Ubuntu 24.04 server, I get the following warning:

Smartmatch is deprecated at /opt/eprints3/bin/epadmin line 396.

When running epadmin config_core, it seems that organization name is not defined:

Organisation Name [['organisation_name' not defined]] ?

drn05r commented 1 month ago

Organisation Name (organisation_name) is an addition phrase that is set when you run epadmin create and provide a value for it at the command line (much like archive_name). This was added in EPrints v3.4.3. If you want to add an organisation_name like would have been added through epadmin create add it to your archive's cfg/lang/n/phrases/archive_name.xml. Having this phrase makes it easy to update static pages if your organisation's name changes.

I am surprised you are getting a Smartmatch warning, as there is a line to turn of warnings for experimental::smartmatch: https://github.com/eprints/eprints3.4/blob/dfe6a1d7e1e5b84540eef136b3e5390ef2e7596e/bin/epadmin#L6 What version of Perl are you running? That said, there is no real need to use smartmatch on line 396, so I will look into fixing that.

blairfix commented 1 month ago

I'm running Perl v5.38.2

blairfix commented 1 month ago

Looks like the Smartmatch feature is deprecated and marked for removal: https://perldoc.perl.org/perldeprecation#Smartmatch

ajmetz commented 1 month ago

Copy and paste of a note from the 8th August 2023:

`---------- Smart match operator found:

3.4.4 Core: check_xapian ---> is this via ingredient? epadmin 3.4.5 Core: epadmin `----------

End copy and paste.

I see epadmin only has it in a single line: if (not $repo_type ~~ @flavours)

(Just noticed David mentioned that too: "there is no real need to use smartmatch on line 396, so I will look into fixing that.")

Off the top of my head, I think that single line could be rephased to:

my $matches_flavours = join(

                             # Join by regex OR...
                             '|',

                             # Regex safe:
                             map {quotemeta($_)}

                             # List of acceptable flavours
                             (
                                 @flavours,
                             )
                         );

unless ($repo_type =~ m/^($matches_flavours)$/)

Give me a moment, and I'll check.

Yours, Andrew.

Quoting Blair Fix @.***>:

Looks like the Smartmatch feature is deprecated and marked for
removal: https://perldoc.perl.org/perldeprecation#Smartmatch

-- Reply to this email directly or view it on GitHub: https://github.com/eprints/eprints3.4/issues/403#issuecomment-2379887616 You are receiving this because you are subscribed to this thread.

Message ID: @.***>

ajmetz commented 1 month ago

Just corrected some errors in my previous comment, and there may be more elegant ways to do it too.

ajmetz commented 1 month ago

A quick test, and the above code appears to work.

[Sat 28 Sep 2024 11:46:43 BST eprints@ip-172-31-44-66 bin]$ perl ./epadmin create edu_balls
Available repository types are: 
   zero
   pub
Usage: epadmin create [repository type]
[Sat 28 Sep 2024 11:46:55 BST eprints@ip-172-31-44-66 bin]$ perl ./epadmin create edu_balls
Available repository types are: 
   pub
   zero
Usage: epadmin create [repository type]
[Sat 28 Sep 2024 11:47:41 BST eprints@ip-172-31-44-66 bin]$ perl ./epadmin create edu_lib
Available repository types are: 
   zero
   pub
Usage: epadmin create [repository type]
[Sat 28 Sep 2024 11:47:48 BST eprints@ip-172-31-44-66 bin]$ perl ./epadmin create zero

Create a zero Repository

Please select an ID for the repository, which will be used to create a directory
and identify the repository. Lower case letters, numbers and underscores, may not start with
a number or underscore. examples: "lemurprints", "test3" or "research_archive"

Existing repositories:
initial_archive

Archive ID? ^C
[Sat 28 Sep 2024 11:48:01 BST eprints@ip-172-31-44-66 bin]$ perl ./epadmin create
Available repository types are: 
   zero
   pub
Usage: epadmin create [repository type]
[Sat 28 Sep 2024 11:48:08 BST eprints@ip-172-31-44-66 bin]$ git status
On branch branch3.4.5
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   epadmin

Untracked files:
  (use "git add <file>..." to include in what will be committed)
    data.xml
    epadmin.htm
    gviews.htm
    import.htm

no changes added to commit (use "git add" and/or "git commit -a")
[Sat 28 Sep 2024 11:48:45 BST eprints@ip-172-31-44-66 bin]$ git diff
diff --git a/bin/epadmin b/bin/epadmin
index 34e213a..0df61cc 100755
--- a/bin/epadmin
+++ b/bin/epadmin
@@ -393,7 +393,20 @@ sub create
    my $conf = $EPrints::SystemSettings::conf;
    my @flavours =  keys %{$conf->{flavours}};
    my $repo_type = $ARGV[0]? $ARGV[0]:"";
-   if (not $repo_type ~~ @flavours)
+   my  $matches_flavours   =   join(
+
+                                 # Join by regex OR...
+                                 '|',
+
+                                 # Regex safe:
+                                 map {quotemeta($_)}
+
+                                 # List of acceptable flavours
+                                 (
+                                     @flavours,
+                                 )
+                             );
+    unless ($repo_type =~ m/^($matches_flavours)$/)
    {
        print STDERR "Available repository types are: \n   ",join("\n   ",@flavours),"\n";
        print STDERR "Usage: epadmin create [repository type]\n";
[Sat 28 Sep 2024 11:48:48 BST eprints@ip-172-31-44-66 bin]$ 
ajmetz commented 1 month ago

This is probably more readable code.

    my  $available_flavours    =   join(

                                        # Join by regex OR...
                                        '|',

                                        # Regex safe:
                                        map {quotemeta($_)}

                                        # List of acceptable flavours
                                        (
                                            @flavours,
                                        )
                                    );
    my  $unavailable_flavour    =   $repo_type !~ m/^($available_flavours)$/;

    if ($unavailable_flavour)
ajmetz commented 1 month ago

Now let's see if I can figure out how to do a pull request on github.

ajmetz commented 1 month ago

Or the far shorter:

+   my @repo_matches_available_flavour = (map {$repo_type eq $_? $_:()} @flavours);
+
+   unless (@repo_matches_available_flavour)
ajmetz commented 1 month ago

Had a go at a fork and pull request, only no idea if I've forked the correct branch or not... https://github.com/eprints/eprints3.4/pull/404

ajmetz commented 1 month ago

This seems to be a diff of both commits: https://github.com/eprints/eprints3.4/pull/404/files This addresses only the smartmatch warning issue raised.

Assuming the phrase advice from David resolves the lack of defined organisation name.

ajmetz commented 1 month ago

Kudos Dave. Your commit was a far more elegant way to do it, in terms of being a minimal change from the original, =). https://github.com/eprints/eprints3.4/commit/0b8057543fceec4e97127a8da0f4173749e05172#commitcomment-147351503

ajmetz commented 1 month ago

A grep over a map, also negated the need for the ternary operator, which was a further optimisation. Kudos!