Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repo: Repository.targets.add_target unexpected behavior implied in walkthrough #319

Closed
awwad opened this issue Feb 22, 2016 · 3 comments
Closed

Comments

@awwad
Copy link
Contributor

awwad commented Feb 22, 2016

The repository creation walkthrough leads you to believe that adding an existing target again replaces it, when it appears to do nothing.

In particular, midway through the add target files section:

>>> repository.targets.add_target(target3_filepath, custom_file_permissions)
# Individual target files may also be added to roles, including custom data about the target.
# In the example below, file permissions of the target (octal number specifying file
 # access for owner, group, others (e.g., 0755) is added alongside the default fileinfo.
 # All target objects in metadata include the target's filepath, hash, and length.
 >>> target3_filepath = "/path/to/repository/targets/file3.txt"
 >>> octal_file_permissions = oct(os.stat(target3_filepath).st_mode)[4:]
 >>> custom_file_permissions = {'file_permissions': octal_file_permissions}
 >>> repository.targets.add_target(target3_filepath, custom_file_permissions)

The walkthrough instructs you to create a custom field for a target and add that target with the custom field. This target has already been added, though, in the walkthrough, and adding it again actually fails to incorporate the custom field. The user is not given a step showing them the result, either, so it leads the user to walk away thinking that the way to add custom fields to an existing target is to simply add the already-existing target back again with the custom field.

Example of the behavior:

>>> repository.targets.target_files
{'/file1.txt': {}, '/octophobe/file50.txt': {'file_permissions': '0644'}, '/file3.txt': {}}
>>> custom_file_permissions = {'file_permissions': '0000'}
>>> repository.targets.add_target('repo/targets/octophobe/file50.txt', custom_file_permissions)
>>> repository.targets.target_files
{'/file1.txt': {}, '/octophobe/file50.txt': {'file_permissions': '0644'}, '/file3.txt': {}}

TODO, then:

  • Either it actually is the expected behavior that re-adding replaces, and the behavior needs to be corrected (to replace an existing target when it is added again), or
  • The instructions should refer to a fresh target instead of an existing target, and, further, a warning should be added when a user attempts to add a target that already exists indicating that the command had no impact.
@awwad awwad changed the title Repository.targets.add_target unexpected behavior implied in walkthrough Repo: Repository.targets.add_target unexpected behavior implied in walkthrough Feb 29, 2016
@vladimir-v-diaz
Copy link
Contributor

This issue appears to be fixed in the latest "develop" branch:

$ python
Python 2.7.6 (default, Jun 22 2015, 17:58:13) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from tuf.repository_tool import *
>>> repository = load_repository('repository')
>>> repository.dirty_roles()
Dirty roles: []
>>> repository.status()
u'root' role contains 0 / 1 signatures.
>>> repository.targets.target_files
{u'/file1.txt': {u'file_permissions': u'664'}, u'/file2.txt': {}}
>>> target3_filepath = 'repository/targets/file3.txt'
>>> octal_file_permissions = oct(os.stat(target3_filepath).st_mode)[4:]
>>> custom_file_permissions = {'file_permissions': octal_file_permissions}
>>> repository.targets.add_target(target3_filepath, custom_file_permissions)
>>> repository.targets.target_files
{'/file3.txt': {'file_permissions': '664'}, u'/file1.txt': {u'file_permissions': u'664'}, u'/file2.txt': {}}
>>> repository.dirty_roles()
Dirty roles: [u'targets']
>>> 

@vladimir-v-diaz
Copy link
Contributor

Reopening. I need to verify the case for a replaced target path!

@vladimir-v-diaz
Copy link
Contributor

This issue should now be fixed in #403. Note how the file2.txt entry is replaced.

>>> from tuf.repository_tool import *
>>> repository = load_repository('repository')
>>> repository.dirty_roles()
Dirty roles: []
>>> repository.targets.target_files
{u'/file1.txt': {u'file_permissions': u'664'}, u'/file2.txt': {}}
>>> target2_filepath = 'repository/targets/file2.txt'
>>> octal_file_permissions = oct(os.stat(target2_filepath).st_mode)[4:]
>>> custom_file_permissions = {'file_permissions': octal_file_permissions}
>>> repository.targets.add_target(target2_filepath, custom_file_permissions)
>>> repository.targets.target_files
{u'/file1.txt': {u'file_permissions': u'664'}, u'/file2.txt': {'file_permissions': '664'}}
>>> repository.dirty_roles()
Dirty roles: [u'targets']
>>> 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants