search

archivematica / Issues

Issues repository for the Archivematica project
GNU Affero General Public License v3.0
16 stars 1 forks source link

Problem: transfer names can't start with a dash #1361

Open sevein opened 6 years ago

sevein commented 6 years ago
ross-spencer commented 6 years ago

@helenst a rogue mention of this issue for me! I was trying to attach another to the PR :)

sevein commented 5 years ago

@ross-spencer do you think issue is now fixed by https://github.com/artefactual/archivematica/pull/1350?

ross-spencer commented 5 years ago

LGTM :+1:

image

But https://github.com/archivematica/Issues/issues/380 will need to be looked at separately.

@sromkey as this is merged, and looks to work, should we add a 1.9 milestone and mark it as verified so that it's picked up in release notes? Additionally get an analyst double-check?

sromkey commented 5 years ago

ok, I've labeled appropriately!

sevein commented 5 years ago

Woohoo!

currmie commented 5 years ago

Fails for me on am19xenial. I just tried a standard transfer with a dash at the start of the transfer name, and AM threw this error: --unitType "SIP" --unitIdentifier "3446d975-dcb8-40f1-8b8b-87001688af5a" --unitName "-mc-dash-in-name"

currmie commented 5 years ago

Oh wait, maybe this build hasn't included the fix yet?

sevein commented 5 years ago

Thanks @currmie, you're right, it's still broken.

I think that position arguments must use the extra double dash which is what Helen was doing in the original PR. More here: https://stackoverflow.com/questions/14693718/how-to-parse-positional-arguments-with-leading-minus-sign-negative-numbers-usi.

$ ./compress_aip.py 1 2 3 "--the-name" 5
usage: compress_aip.py [-h]
                       compression compression_level sip_directory sip_name
                       sip_uuid
compress_aip.py: error: too few arguments

$ ./compress_aip.py -- 1 2 3 "--the-name" 5
Namespace(compression='1', compression_level='2', sip_directory='3', sip_name='--the-name', sip_uuid='5')
sromkey commented 5 years ago

I took the milestone off, since this wasn't meant to be in scope for this release anyway.