Skip to content

Commit

Permalink
[irods#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 authored and alanking committed Mar 18, 2024
1 parent 075ce0c commit 9f61224
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 34 deletions.
67 changes: 35 additions & 32 deletions lib/core/src/irods_parse_command_line_options.cpp
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", "[Deprecated] ignore symlinks. Use --ignore-symlinks.")
("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
3 changes: 2 additions & 1 deletion lib/core/src/parseCommandLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ CommandLineOptions document so we can keep it all consistent.
#include "irodsntutil.hpp"
#endif

#include <cstring>

/*
Input:
Expand Down Expand Up @@ -51,7 +52,7 @@ parseCmdLineOpt( int argc, char **argv, const char *optString, int includeLong,
/* handle the long options first */
if ( includeLong ) {
for ( int i = 0; i < argc; i++ ) {
if ( strcmp( "--link", argv[i] ) == 0 ) {
if (std::strcmp("--ignore-symlinks", argv[i]) == 0 || std::strcmp("--link", argv[i]) == 0) {
rodsArgs->link = True;
argv[i] = "-Z"; /* ignore symlink */
}
Expand Down
36 changes: 35 additions & 1 deletion scripts/irods/test/test_iput.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,41 @@ 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 directory for the symlink.
# The symlink file must be placed in a separate directory due to the recursive flag for iput.
symlink_dir_basename = 'iput_issue_7537_symlink_dir'
symlink_dir = f'{self.user.local_session_dir}/{symlink_dir_basename}'
os.mkdir(symlink_dir)

# Create a symlink to the file.
symlink_file_basename = f'{test_file_basename}.symlink'
symlink_file = f'{symlink_dir}/{symlink_file_basename}'
os.symlink(test_file, symlink_file)
self.assertTrue(os.path.exists(symlink_file))

# Try to upload the symlink file into iRODS.
self.user.assert_icommand(['iput', '--ignore-symlinks', symlink_file])
self.user.assert_icommand(['ils', '-lr'], 'STDOUT') # Debugging

# Show no data object exists, proving the symlink file was ignored.
logical_path = f'{self.user.session_collection}/{symlink_file_basename}'
self.assertFalse(lib.replica_exists(self.user, logical_path, 0))

# Try to upload the symlink file again using the recursive flag and the parent directory.
self.user.assert_icommand(['iput', '--ignore-symlinks', '-r', symlink_dir])
self.user.assert_icommand(['ils', '-lr'], 'STDOUT') # Debugging

# Show no data object exists, proving the symlink file was ignored.
logical_path = f'{self.user.session_collection}/{symlink_dir_basename}/{symlink_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 +1020,3 @@ def overwrite_verify_stale_no_checksum(self, filename, expected_checksum):

def overwrite_verify_stale_with_checksum(self, filename, expected_checksum):
pass

35 changes: 35 additions & 0 deletions scripts/irods/test/test_irsync.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,3 +646,38 @@ 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 irsync due to the symlink.')

# Create a directory for the symlink.
# The symlink file must be placed in a separate directory due to the recursive flag for irsync.
symlink_dir_basename = 'irsync_issue_7537_symlink_dir'
symlink_dir = f'{self.user0.local_session_dir}/{symlink_dir_basename}'
os.mkdir(symlink_dir)

# Create a symlink to the file.
symlink_file_basename = f'{test_file_basename}.symlink'
symlink_file = f'{symlink_dir}/{symlink_file_basename}'
os.symlink(test_file, symlink_file)
self.assertTrue(os.path.exists(symlink_file))

# Try to upload the symlink file into iRODS.
self.user0.assert_icommand(['irsync', '--ignore-symlinks', symlink_file, f'i:{symlink_file_basename}'])
self.user0.assert_icommand(['ils', '-lr'], 'STDOUT') # Debugging

# Show no data object exists, proving the symlink file was ignored.
logical_path = f'{self.user0.session_collection}/{symlink_file_basename}'
self.assertFalse(lib.replica_exists(self.user0, logical_path, 0))

# Try to upload the symlink file again using the recursive flag and the parent directory.
self.user0.assert_icommand(['irsync', '--ignore-symlinks', '-r', symlink_dir, f'i:{symlink_dir_basename}'])
self.user0.assert_icommand(['ils', '-lr'], 'STDOUT') # Debugging

# Show no data object exists, proving the symlink file was ignored.
logical_path = f'{self.user0.session_collection}/{symlink_dir_basename}/{symlink_file_basename}'
self.assertFalse(lib.replica_exists(self.user0, logical_path, 0))

0 comments on commit 9f61224

Please sign in to comment.