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.

Sign up for GitHub

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

Jump to bottom

OmeroPy: don't look for bin/omero in .git #5964

Closed
wants to merge 1 commit into from
Closed

OmeroPy: don't look for bin/omero in .git #5964

wants to merge 1 commit into from

Conversation

joshmoore
Copy link
Member

The travers method walks up from the current directory looking
for a subdirectory that contains bin/omero. Unfortunately, that
holds true for .git as well. A better fix would likely be to not
check more deeply than 1 or 2 subdirectories.

see: manics@040878c#commitcomment-32511555

@joshmoore
OmeroPy: don't look for bin/omero in .git
ff54e76 
The `travers` method walks up from the current directory looking
for a subdirectory that contains `bin/omero`. Unfortunately, that
holds true for .git as well. A better fix would likely be to not
check more deeply than 1 or 2 subdirectories.

see: manics@040878c#commitcomment-32511555
joshmoore referenced this pull request in manics/openmicroscopy Feb 28, 2019
@manics
omero.web.django_additional_settings
040878c 
JSON mapping of additional Django settings. Use this to set or override Django settings that aren't managed by OMERO.web.
@will-moore
Copy link
Member

I found this didn't work when I created my own branch json•additional•settings because when the path t is e.g. /Users/wmoore/Desktop/OMERO/openmicroscopy/.git/logs/refs/heads/json•additional•settings
then t.dirname().dirname().basename() is refs so it fails the check for .git.
Same will be true for
/Users/wmoore/Desktop/OMERO/openmicroscopy/.git/logs/refs/remotes/simon/json•additional•settings

It needs something like:

             dist = None
             for root, dirs, files in os.walk(p):
+                if '.git' in root:
+                    continue
                 for f in files:
                     t = path(os.path.join(root, f))

@will-moore
Copy link
Member

NB: This took a long time to debug because trying to print anything e.g. print root seemed to make the exception vanish (probably changing some str() behaviour) so I instead had to add a try/except to print out various values when it failed.

@joshmoore
Copy link
Member Author

It needs something like:

Even if the top-level ends with .git? I'll need to double-check.

@will-moore
Copy link
Member

This bug has got me again, having checked out @manics omero•web•client•settings branch to test #6053

rm -rf /Users/wmoore/Desktop/OMERO/openmicroscopy/.git/logs/refs/remotes/simon/omero•web•client•settings
git branch -d json•additional•settings

Still didn't work, so I used my fix above #5964 (comment) so I can run tests...

@joshmoore
Copy link
Member Author

@will-moore : if you have something you can test, can you try two further things:

  • rename your openmicroscopy to openmicroscopy.git and see what happens
  • try changing your fix to test for /.git/ (cross-platform) as opposed to just ".git" in

@joshmoore joshmoore added exclude transfer Migrate to another repository labels Aug 20, 2019
@joshmoore
Copy link
Member Author

Taken care of by #6200

@joshmoore joshmoore closed this Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
exclude transfer Migrate to another repository
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@joshmoore @will-moore