From f92265b7492e7b62e3efad8b7e216c2d58d4dd7f Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Thu, 14 Mar 2024 15:20:27 -0400 Subject: [PATCH] [7537] Add --ignore-symlinks alias for --link option. The --link option does not provide enough information to help the user understand its purpose. This commit addresses that by adding an alias that is more explicit and direct. This commit updates the option list of iput and irsync. --- .../src/irods_parse_command_line_options.cpp | 67 ++++++++++--------- scripts/irods/test/test_iput.py | 20 +++++- scripts/irods/test/test_irsync.py | 19 ++++++ 3 files changed, 73 insertions(+), 33 deletions(-) diff --git a/lib/core/src/irods_parse_command_line_options.cpp b/lib/core/src/irods_parse_command_line_options.cpp index 53ff77a8ac..050a5ca9dc 100644 --- a/lib/core/src/irods_parse_command_line_options.cpp +++ b/lib/core/src/irods_parse_command_line_options.cpp @@ -26,38 +26,41 @@ static int parse_program_options( path_list_t& _paths ) { namespace po = boost::program_options; - po::options_description opt_desc( "options" ); + po::options_description opt_desc("options"); + // clang-format off opt_desc.add_options() - ( "help,h", "show command usage" ) - ( "all,a", "all - update all existing copies" ) - ( "bulk,b", "bulk upload to reduce overhead" ) - ( "force,f", "force - write data-object even it exists already; overwrite it" ) - ( "redirect,I", "redirect connection - redirect the connection to connect directly to the resource server." ) - ( "checksum,k", "checksum - calculate a checksum on the data server-side, and store it in the catalog" ) - ( "verify_checksum,K", "verify checksum - calculate and verify the checksum on the data, both client-side and server-side, without storing in the catalog." ) - ( "repl_num,n", po::value(), "replNum - the replica to be replaced, typically not needed" ) - ( "num_threads,N", po::value(), "numThreads - the number of threads to use for the transfer. A value of 0 means no threading. By default (-N option not used) the server decides the number of threads to use." ) - ( "physical_path,p", po::value(), "physicalPath - the absolute physical path of the uploaded file on the server" ) - ( "progress,P", "output the progress of the upload." ) - ( "rbudp,Q", "use RBUDP (datagram) protocol for the data transfer" ) - ( "recursive,r", "recursive - store the whole subdirectory" ) - ( "dest_resc,R", po::value(), "resource - specifies the resource to store to. This can also be specified in your environment or via a rule set up by the administrator" ) - ( "ticket,t", po::value(), "ticket - ticket (string) to use for ticket-based access" ) - ( "renew_socket,T", "renew socket connection after 10 minutes" ) - ( "verbose,v", "verbose" ) - ( "very_verbose,V", "very verbose" ) - ( "data_type,D", po::value(), "dataType - the data type string" ) - ( "restart_file,X", po::value(), "restartFile - specifies that the restart option is on and the restartFile input specifies a local file that contains the restart information." ) - ( "link", "ignore symlink." ) - ( "lfrestart", po::value(), "lfRestartFile - specifies that the large file restart option is on and the lfRestartFile input specifies a local file that contains the restart information." ) - ( "retries", po::value(), "count - Retry the iput in case of error. The 'count' input specifies the number of times to retry. It must be used with the -X option" ) - ( "wlock", "use advisory write (exclusive) lock for the upload" ) - ( "rlock", "use advisory read lock for the download" ) - ( "purgec", "Purge the staged cache copy after uploading an object to a" ) - ( "kv_pass", po::value(), "pass key-value strings through to the plugin infrastructure" ) - ( "metadata", po::value(), "atomically assign metadata after a data object is put" ) - ( "acl", po::value(), "atomically apply an access control list after a data object is put" ) - ( "path_args", po::value( &_paths )->composing(), "some files and stuffs" ); + ("help,h", "show command usage") + ("all,a", "all - update all existing copies") + ("bulk,b", "bulk upload to reduce overhead") + ("force,f", "force - write data-object even it exists already; overwrite it") + ("redirect,I", "redirect connection - redirect the connection to connect directly to the resource server.") + ("checksum,k", "checksum - calculate a checksum on the data server-side, and store it in the catalog") + ("verify_checksum,K", "verify checksum - calculate and verify the checksum on the data, both client-side and server-side, without storing in the catalog.") + ("repl_num,n", po::value(), "replNum - the replica to be replaced, typically not needed") + ("num_threads,N", po::value(), "numThreads - the number of threads to use for the transfer. A value of 0 means no threading. By default (-N option not used) the server decides the number of threads to use.") + ("physical_path,p", po::value(), "physicalPath - the absolute physical path of the uploaded file on the server") + ("progress,P", "output the progress of the upload.") + ("rbudp,Q", "use RBUDP (datagram) protocol for the data transfer") + ("recursive,r", "recursive - store the whole subdirectory") + ("dest_resc,R", po::value(), "resource - specifies the resource to store to. This can also be specified in your environment or via a rule set up by the administrator") + ("ticket,t", po::value(), "ticket - ticket (string) to use for ticket-based access") + ("renew_socket,T", "renew socket connection after 10 minutes") + ("verbose,v", "verbose") + ("very_verbose,V", "very verbose") + ("data_type,D", po::value(), "dataType - the data type string") + ("restart_file,X", po::value(), "restartFile - specifies that the restart option is on and the restartFile input specifies a local file that contains the restart information.") + ("link", "ignore symlink.") + ("ignore-symlinks", "ignore symlinks.") + ("lfrestart", po::value(), "lfRestartFile - specifies that the large file restart option is on and the lfRestartFile input specifies a local file that contains the restart information.") + ("retries", po::value(), "count - Retry the iput in case of error. The 'count' input specifies the number of times to retry. It must be used with the -X option") + ("wlock", "use advisory write (exclusive) lock for the upload") + ("rlock", "use advisory read lock for the download") + ("purgec", "Purge the staged cache copy after uploading an object to a") + ("kv_pass", po::value(), "pass key-value strings through to the plugin infrastructure") + ("metadata", po::value(), "atomically assign metadata after a data object is put") + ("acl", po::value(), "atomically apply an access control list after a data object is put") + ("path_args", po::value(&_paths)->composing(), "some files and stuffs"); + // clang-format on po::positional_options_description pos_desc; pos_desc.add( "path_args", -1 ); @@ -200,7 +203,7 @@ static int parse_program_options( return INVALID_ANY_CAST; } } - if ( global_prog_ops_var_map.count( "link" ) ) { + if (global_prog_ops_var_map.count("ignore-symlinks") || global_prog_ops_var_map.count("link")) { _rods_args.link = 1; } if ( global_prog_ops_var_map.count( "lfrestart" ) ) { diff --git a/scripts/irods/test/test_iput.py b/scripts/irods/test/test_iput.py index cfe7f7233a..5d45599ad3 100644 --- a/scripts/irods/test/test_iput.py +++ b/scripts/irods/test/test_iput.py @@ -821,6 +821,25 @@ def test_iput_does_not_ignore_symlinks_by_default__issue_5359(self): finally: self.user.assert_icommand(['ils', '-lr'], 'STDOUT', self.user.session_collection) # debugging + def test_iput_ignore_symlinks_option_is_an_alias_of_the_link_option__issue_7537(self): + # Create a file. + test_file_basename = 'iput_issue_7537.txt' + test_file = f'{self.user.local_session_dir}/{test_file_basename}' + with open(test_file, 'w') as f: + f.write('This file will be ignored by iput due to the symlink.') + + # Create a symlink to the file. + symlinked_file = f'{self.user.local_session_dir}/{test_file_basename}.symlinked' + os.symlink(test_file, symlinked_file) + self.assertTrue(os.path.exists(symlinked_file)) + + # Try to upload the symlinked file into iRODS. + self.user.assert_icommand(['iput', '--ignore-symlinks', symlinked_file]) + + # Show no data object exists, proving the symlinked file was ignored. + logical_path = f'{self.user.session_collection}/{test_file_basename}' + self.assertFalse(lib.replica_exists(self.user, logical_path, 0)) + class test_iput_with_checksums(session.make_sessions_mixin(rodsadmins, rodsusers), unittest.TestCase): def setUp(self): @@ -985,4 +1004,3 @@ def overwrite_verify_stale_no_checksum(self, filename, expected_checksum): def overwrite_verify_stale_with_checksum(self, filename, expected_checksum): pass - diff --git a/scripts/irods/test/test_irsync.py b/scripts/irods/test/test_irsync.py index ae657c0202..d4730bf4a5 100644 --- a/scripts/irods/test/test_irsync.py +++ b/scripts/irods/test/test_irsync.py @@ -646,3 +646,22 @@ def test_irsync_does_not_ignore_symlinks_by_default__issue_5359(self): finally: self.user0.assert_icommand(['ils', '-lr'], 'STDOUT', self.user0.session_collection) # debugging + + def test_irsync_ignore_symlinks_option_is_an_alias_of_the_link_option__issue_7537(self): + # Create a file. + test_file_basename = 'irsync_issue_7537.txt' + test_file = f'{self.user0.local_session_dir}/{test_file_basename}' + with open(test_file, 'w') as f: + f.write('This file will be ignored by iput due to the symlink.') + + # Create a symlink to the file. + symlinked_file = f'{self.user0.local_session_dir}/{test_file_basename}.symlinked' + os.symlink(test_file, symlinked_file) + self.assertTrue(os.path.exists(symlinked_file)) + + # Try to sync the symlinked file into iRODS. + logical_path = f'{self.user0.session_collection}/{test_file_basename}' + self.user0.assert_icommand(['irsync', '--ignore-symlinks', symlinked_file, f'i:{logical_path}']) + + # Show no data object exists, proving the symlinked file was ignored. + self.assertFalse(lib.replica_exists(self.user0, logical_path, 0))