Skip to content

Commit

Permalink
[7537] Add --ignore-symlinks alias for --link option.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@korydraughn
korydraughn committed Mar 14, 2024
1 parent 084e1da commit f92265b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 33 deletions.
67 changes: 35 additions & 32 deletions lib/core/src/irods_parse_command_line_options.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>(), "replNum - the replica to be replaced, typically not needed" )
( "num_threads,N", po::value<int>(), "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<std::string>(), "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<std::string>(), "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<std::string>(), "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<std::string>(), "dataType - the data type string" )
( "restart_file,X", po::value<std::string>(), "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<std::string>(), "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<int>(), "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<std::string>(), "pass key-value strings through to the plugin infrastructure" )
( "metadata", po::value<std::string>(), "atomically assign metadata after a data object is put" )
( "acl", po::value<std::string>(), "atomically apply an access control list after a data object is put" )
( "path_args", po::value<path_list_t>( &_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<std::string>(), "replNum - the replica to be replaced, typically not needed")
("num_threads,N", po::value<int>(), "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<std::string>(), "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<std::string>(), "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<std::string>(), "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<std::string>(), "dataType - the data type string")
("restart_file,X", po::value<std::string>(), "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<std::string>(), "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<int>(), "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<std::string>(), "pass key-value strings through to the plugin infrastructure")
("metadata", po::value<std::string>(), "atomically assign metadata after a data object is put")
("acl", po::value<std::string>(), "atomically apply an access control list after a data object is put")
("path_args", po::value<path_list_t>(&_paths)->composing(), "some files and stuffs");
// clang-format on

po::positional_options_description pos_desc;
pos_desc.add( "path_args", -1 );
Expand Down Expand Up @@ -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" ) ) {
Expand Down
20 changes: 19 additions & 1 deletion scripts/irods/test/test_iput.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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

19 changes: 19 additions & 0 deletions scripts/irods/test/test_irsync.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

0 comments on commit f92265b

Please sign in to comment.