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

shed_diff smart diff not hiding dependency changes (changeset_revision etc) #205

Closed
peterjc opened this Issue May 14, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@peterjc
Copy link
Contributor

peterjc commented May 14, 2015

The help suggests smart/magic diff is the default,

$ planemo shed_diff --help
...
  --raw                      Do not attempt smart diff of XML to filter out
                             attributes populated by the Tool Shed.
...

However, this isn't being smart enough:

$ planemo shed_diff --shed_target testtoolshed ~/repositories/pico_galaxy/tools/sample_seqs/
wget -q --recursive -O - 'https://testtoolshed.g2.bx.psu.edu/repository/download?repository_id=de64f62614881770&changeset_revision=default&file_type=gz' | tar -xzf - -C /tmp/tool_shed_diff_i5xHt_/_testtoolshed_ --strip-components 1
mkdir "/tmp/tool_shed_diff_i5xHt_/_local_"; tar -xzf "/tmp/tmpS0q0pC" -C "/tmp/tool_shed_diff_i5xHt_/_local_"; rm -rf /tmp/tmpS0q0pC
cd "/tmp/tool_shed_diff_i5xHt_"; diff -r _local_ _testtoolshed_
diff -r _local_/tools/sample_seqs/tool_dependencies.xml _testtoolshed_/tools/sample_seqs/tool_dependencies.xml
4c4
<         <repository name="package_biopython_1_65" owner="biopython" />

---
>         <repository changeset_revision="f8d72690eeae" name="package_biopython_1_65" owner="biopython" toolshed="https://testtoolshed.g2.bx.psu.edu" />

This looks like the raw diff output:

$ planemo shed_diff --shed_target testtoolshed ~/repositories/pico_galaxy/tools/sample_seqs/ --raw
wget -q --recursive -O - 'https://testtoolshed.g2.bx.psu.edu/repository/download?repository_id=de64f62614881770&changeset_revision=default&file_type=gz' | tar -xzf - -C /tmp/tool_shed_diff_VGHJk0/_testtoolshed_ --strip-components 1
mkdir "/tmp/tool_shed_diff_VGHJk0/_local_"; tar -xzf "/tmp/tmpBmc0Ck" -C "/tmp/tool_shed_diff_VGHJk0/_local_"; rm -rf /tmp/tmpBmc0Ck
cd "/tmp/tool_shed_diff_VGHJk0"; diff -r _local_ _testtoolshed_
diff -r _local_/tools/sample_seqs/tool_dependencies.xml _testtoolshed_/tools/sample_seqs/tool_dependencies.xml
4c4
<         <repository name="package_biopython_1_65" owner="biopython" />

---
>         <repository changeset_revision="f8d72690eeae" name="package_biopython_1_65" owner="biopython" toolshed="https://testtoolshed.g2.bx.psu.edu" />

I think the problem is the assumption in planemo/shed/diff.py that the special files tool_dependencies.xml and repository_dependencies.xml will only be at the tar-ball root:

def diff_and_remove(working, label_a, label_b, f):
    a_deps = os.path.join(working, label_a, "tool_dependencies.xml")
    b_deps = os.path.join(working, label_b, "tool_dependencies.xml")
    a_repos = os.path.join(working, label_a, "repository_dependencies.xml")
    b_repos = os.path.join(working, label_b, "repository_dependencies.xml")

Also, as a possible enhancement, should we consider the TS doing attribute reordering, which prompted me to make this change (testing with an older version of planemo)?: peterjc/pico_galaxy@698859a

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented May 14, 2015

Ahhh - the assumption that these files are stored in the root again. Is there any way I can convince you to move these files to the root of the tarball - like with package_s?

If I cannot we can probably modify planemo - but I really want to encourage more structured repositories.

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented May 14, 2015

In the short term it would be possible for me to "move" files about within the tar-ball and TS via the .shed.yml include settings... but that seems nasty. I'm inclined to offer a patch to planemo to fix the diff. We could also make planemo lint nag if tool_dependencies.xml and repository_dependencies.xml are not at the tar-ball root.

Longer term, things like continuous integration and auto-pushing to the (Test) Tool Shed might encourage me to split out my tools into one per git repository - and which point adopting the https://github.com/galaxy-iuc/standards/blob/master/docs/best_practices/repositories.rst like structure with each repository on the TS as a self-contained directory is natural.

peterjc added a commit to peterjc/planemo that referenced this issue May 14, 2015

Do not assume special XML files will be at root.
Previously assumed tool_dependencies.xml and repository_dependencies.xml
would be at the root of the Tool Shed tarball when giving them special
treatment in planemo shed_diff.

Currently the Tool Shed does not require this, and existing tools may
have these XML files in a subfolder.

Fixes GitHub issue galaxyproject#205.

peterjc added a commit to peterjc/planemo that referenced this issue May 14, 2015

Do not assume special XML files will be at root.
Previously assumed tool_dependencies.xml and repository_dependencies.xml
would be at the root of the Tool Shed tarball when giving them special
treatment in planemo shed_diff.

Currently the Tool Shed does not require this, and existing tools may
have these XML files in a subfolder.

Fixes GitHub issue galaxyproject#205.
@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented May 18, 2015

Fixed with #207.

@jmchilton jmchilton closed this May 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment