API review
Proposer: Tully Foote
Present at review:
- Tully
- Wim
- Stu
- Jeremy
- Tony
Changes to be reviewed
- New additive behavior (developed for hackathon use case)
- If given duplicate local_name, override the previous version.
- When installing if the URL is different, mv directory to backup path(ROSINSTALL_DIRECTORY/backup/. This allows changes in URLs. Before it just called svn up or equivilent.
- This allows easy adding, and modifying, but not deletion. (Deletion requires hand editing of .rosinstall file)
Question / concerns / comments
Stu
- Suppose I re-rosinstall the entire rosinstall file. That's a lot of things to replace, and will result in a large backup directory.
Tully It will only backup things which have changed URLs, so if the file is the same it won't backup anything.
Tony
- What happens if the git branch tag changes? Does it consider it part of the "URL" and back it up? What about other tags that may have similar effects?
Tully Yes a changing branch should be considered a changed url. Ticketed <<Ticket(ros 2907)>>
- What happens if an item is deleted from the .rosinstall file, and you want it to go away in the directory?
Tully It will no longer be in your path when generated. It will still be on disk. I do not feel comfortable deleting anything. A manual deletion is required. A ticket is open to warn these directories exist <<Ticket(ros 2572)>>
- What happens if an item in the .rosinstall file is called "backup"?
- What happens if the URL for an item changes multiple times?
- Under this scheme, we have to be careful with the ros package path. If we set it to the ROSINSTALL_DIRECTORY, then packages in the backup might end up in the package path. If this happens, code in those packages could run instead of the updated code.
Jeremy
- I generally agree that the backup mechanism makes me nervous
- What gets backed up? The whole rosinstall? Just the portion below the localname which had the url-change?
Tully Just the portion below local-name. The goal is to never lose uncommitted changes
- Tony makes a good point about being sure to hide these from rospack reliably.
- What gets backed up? The whole rosinstall? Just the portion below the localname which had the url-change?
- Not really addressed here, but I still think we need a better mechanism for incremental updates and deletion.
- It's a pain to always have to retype the full path to your rosinstall in order to do an update rather than rosinstall caching the url for the rosinstall file.
Tully Ticket as feature enhancement <<Ticket(ros 2837)>>
- It's a pain to always have to retype the full path to your rosinstall in order to do an update rather than rosinstall caching the url for the rosinstall file.
Meeting agenda
To be filled out by proposer based on comments gathered during API review period
Proposed change by Wim:
- Change default behavior to prompt, [Delete, Abort, Backup]
- Delete deletes the file
- Abort stops processing
- backup prompts for directory into which to backup
- New Command line options: --delete-changed-urls --abort-changed-urls --backup-changed-urls=PATH_TO_BACKUP_INTO
Stu: there may be more general names for these. Something more like -f
- This seems to deal with Tony's comments about avoiding conflicts, and Jeremy's worries about automatic backups
- There was also an issue of possible ballooning disk space made by multiple backups, which is now a lot clearer to the user.
- Change default behavior to prompt, [Delete, Abort, Backup]
- Proposed change to extra directory handling:
- Instead of warning about extra directories at end prompt user to [Delete, Ignore] extra directories
- Command line options --ignore-extra-directories or --delete-extra-directories.
- Instead of warning about extra directories at end prompt user to [Delete, Ignore] extra directories
Conclusion
Package status change mark change manifest)
Action items that need to be taken.
<<Ticket(ros 2572)>>
<<Ticket(ros 2907)>>
Major issues that need to be resolved
Change to prompt for backup <<Ticket(ros 2921)>>