Skip to content

ported to OSX. not tested; almost definitely needs more work.#1

Merged
mjordan merged 8 commits into
mjordan:masterfrom
axfelix:master
Oct 16, 2014
Merged

ported to OSX. not tested; almost definitely needs more work.#1
mjordan merged 8 commits into
mjordan:masterfrom
axfelix:master

Conversation

@axfelix
Copy link
Copy Markdown
Contributor

@axfelix axfelix commented Oct 15, 2014

hey Mark,

rearchitected this to make it (hopefully) work cross-platform using the Automator to add a context menu entry and Cocoadialog in place of Gtk. not yet tested, but if you want to ensure I haven't broken it on Linux/Windows, that wouldn't be a bad idea... (I'll check myself later on).

@mjordan
Copy link
Copy Markdown
Owner

mjordan commented Oct 15, 2014

Will test over lunch.

@axfelix
Copy link
Copy Markdown
Contributor Author

axfelix commented Oct 15, 2014

You can hold of on that, looks like I indeed broke some stuff :)

fixing ...

@axfelix
Copy link
Copy Markdown
Contributor Author

axfelix commented Oct 15, 2014

hey Mark,

go ahead and test now -- all should be fine, except it's throwing a strange error from the bagit lib:

ERROR:root:expected string or buffer
Traceback (most recent call last):
File "/usr/local/lib/python2.7/dist-packages/bagit.py", line 147, in make_bag
_make_tag_file('bag-info.txt', bag_info)
File "/usr/local/lib/python2.7/dist-packages/bagit.py", line 723, in _make_tag_file
txt = re.sub('\n|\r|(\r\n)', '', txt)
File "/usr/lib/python2.7/re.py", line 151, in sub
return _compile(pattern, flags).sub(repl, string, count)
TypeError: expected string or buffer

it appears to be creating the bag succesfully prior to throwing the error, too... any chance you changed the tag behaviour when you changed the tag behaviour last night when you changed the directory modification behaviour?

Comment thread createbag.py
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get as far as creating a Bag. Running it under Ubuntu results in the following error when I try::

Traceback (most recent call last): File "createbag.py", line 169, in on_folder_clicked "The Bag for folder %s has been created." % bag_dir) NameError: global name 'bag_dir' is not defined

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing line 169 to:

"The Bag for folder %s has been created." % folder_picker_dialog.get_filename()

Appears to have eliminated the error. I didn't get the tag error you reported. Did you add some custom tags?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't, no, which is weird. I also committed a different fix for the error a few minutes ago that's more consistent with the way you had it working previously, but either way.

@mjordan
Copy link
Copy Markdown
Owner

mjordan commented Oct 15, 2014

Re. the tag error, all I did last night was make the 'create_bag_in' tag enabled (it was commented out previous to that).

@axfelix
Copy link
Copy Markdown
Contributor Author

axfelix commented Oct 15, 2014

just pushed a fix for that -- oddly, that wasn't getting thrown on my system, probably because the tag error was coming up first -- which means that one might be local to my environment somehow.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty old - does it work on modern Macs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure does! good old Cocoadialog.

…tory since I didn't realize that's where bags will end up without being moved elsewhere
@axfelix
Copy link
Copy Markdown
Contributor Author

axfelix commented Oct 15, 2014

most recent commit "finishes" the OSX version (it creates a bag and provides a success message), and I think all other issues should be fixed at this point!

I commented out the cleaning of the /tmp directory I'd added earlier today; originally, I put that in there because I noticed while testing that if I kept trying to create the same bag, it would keep creating arbitrarily many /data directories in the same subfolder in /tmp. I think the script should probably finish by moving the /tmp bag to the desktop or the same directory as the original one so that /tmp can be cleaned -- what do you think?

@mjordan
Copy link
Copy Markdown
Owner

mjordan commented Oct 15, 2014

Awesome, will test tonight.

The behavior you're describing was not intended so I'd classify it as a bug. Let me get through testing your PR and we'll take a look at that (and another bug I found today).

Comment thread createbag.py
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that the script is installed in a specific location. Make relative to current script with os.path.dirname(os.path.realpath(file))?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I always forget that one -- though we're still kind of hamstrung by the need to provide straightforward automator syntax in the readme.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see that the path needs to be hard coded because of https://github.com/mjordan/createbag/pull/1/files#diff-04c6e90faac2675aa89e2176d2eec7d8R71. But is it essential that the application gets installed in the user's home directory? Is there a way to share one installation among many Macs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At that point in pretty sure we're onto thinking about app bundling (which is very easy to do on OSX, if there were a way to register automator services programmatically too)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should plan for bundling on all platforms. Can you figure out if that's feasible on OS X and if so, we can reopen this issue. For now let's go with the hard-coded installation directory and make sure it's documented.

mjordan added a commit that referenced this pull request Oct 16, 2014
ported to OSX. not tested; almost definitely needs more work.
@mjordan mjordan merged commit bf1d9b2 into mjordan:master Oct 16, 2014
@mjordan
Copy link
Copy Markdown
Owner

mjordan commented Oct 16, 2014

Just merged. Awesome. I'm going to add some comments to the main script to clarify my understanding of the logic, now that there are long-span if/else blocks to accommodate Darwin....

Then, a few bug fixes, including a weird one where BagIt tags are being added using config options....

@axfelix
Copy link
Copy Markdown
Contributor Author

axfelix commented Oct 16, 2014

Sounds good! Looking forward to working more on this. Let me know if anything I did is weird/unclear.

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

Successfully merging this pull request may close these issues.

2 participants