Skip to content
This repository has been archived by the owner on Mar 26, 2022. It is now read-only.

Broken package on MacOS since 0.8.0 #134

Closed
abulte opened this issue Sep 4, 2018 · 10 comments
Closed

Broken package on MacOS since 0.8.0 #134

abulte opened this issue Sep 4, 2018 · 10 comments
Assignees

Comments

@abulte
Copy link

abulte commented Sep 4, 2018

Hi,

#60 #131 seems to break the package installation/import on MacOS (10.13.6, APFS volume, homebrew python 2.7.10).

$ pip install commonmark==0.7.5
$ python
Python 2.7.10 (default, Oct  6 2017, 22:29:07)
>>> import CommonMark
>>> 

$ pip install commonmark==0.8.0
$ python
>>> import commonmark
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named commonmark
>>> import CommonMark
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/alexandre/Developer/Etalab/udata/pyenv/lib/python2.7/site-packages/CommonMark/__init__.py", line 4, in <module>
    from commonmark.main import commonmark
ImportError: No module named commonmark.main

FYI the package is still installed under site-packages/CommonMark for me, but if I understand #131 correctly it should be under site-packages/commonmark?

Thanks!

@kislyuk
Copy link

kislyuk commented Sep 4, 2018

We are facing the same problem.

It seems that the package namespace was rearranged and the API has changed in a breaking way. When this happens, it's best practice to update the major version. Ideally commonmark 0.x.x should retain the old API, while commonmark 1.x.x should incorporate the breaking namespace changes.

@abulte
Copy link
Author

abulte commented Sep 5, 2018

More info after a few tests: if I clone the repo and install the package with pip install ., the package is correctly installed in site-packages/commonmark and import commonmark will work OK. Apparently this bug only happens if you install from pypi.

@nikolas
Copy link
Collaborator

nikolas commented Sep 6, 2018

Thanks for the report, and apologies for the problem.

The intention here was to maintain some backwards compatibility by making the CommonMark directory a symlink to commonmark. It looks like this isn't necessary, isn't working correctly with python packaging mechanisms, and also breaks compatibility with case-insensitive filesystems (biolab/orange3#3234, though.... what filesystem is case-insensitive? :P)

I will just remove the CommonMark symlink for good - we should only be relying on commonmark for simplicity.

nikolas added a commit that referenced this issue Sep 6, 2018
nikolas added a commit that referenced this issue Sep 6, 2018
remove CommonMark symlink (#134)
@nikolas
Copy link
Collaborator

nikolas commented Sep 6, 2018

I've released a new 0.8.1 version of this package. I'm using Linux so I can't tell whether the problem with macOS is fixed.

One thing I still need to fix: on PyPI's website, the package name is still CommonMark, even if you try to visit the lower-cased version here: https://pypi.org/project/commonmark/

However, the version that I just uploaded on test.pypi.org is correct: https://test.pypi.org/project/commonmark/

I don't see a setting for this in PyPI's interface, so I'm going to contact them and see how I can change this. I've created an issue for this problem here: pypi/warehouse#4679

@edmorley
Copy link

edmorley commented Sep 7, 2018

Our Read the docs builds have just started failing (using commonmark 0.8.1), with:

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/treeherder/envs/latest/local/lib/python2.7/site-packages/sphinx/config.py", line 161, in __init__
    execfile_(filename, config)
  File "/home/docs/checkouts/readthedocs.org/user_builds/treeherder/envs/latest/local/lib/python2.7/site-packages/sphinx/util/pycompat.py", line 150, in execfile_
    exec_(code, _globals)
  File "/home/docs/checkouts/readthedocs.org/user_builds/treeherder/envs/latest/local/lib/python2.7/site-packages/six.py", line 709, in exec_
    exec("""exec _code_ in _globs_, _locs_""")
  File "<string>", line 1, in <module>
  File "conf.py", line 2, in <module>
    from recommonmark.parser import CommonMarkParser
  File "/home/docs/checkouts/readthedocs.org/user_builds/treeherder/envs/latest/src/recommonmark/recommonmark/parser.py", line 9, in <module>
    from CommonMark import Parser
ImportError: No module named CommonMark

Please can this breaking change be reverted and released in a point release, then the change relanded for a new major version bump?

@abulte
Copy link
Author

abulte commented Sep 7, 2018

Thanks 🙇 for taking care of this @nikolas. I can confirm that 0.8.1is 👌 on my environment.

I also agree with @edmorley that this change (alias removal) should be introduced in a new major.

edmorley added a commit to mozilla/treeherder that referenced this issue Sep 7, 2018
CommonMark has made a breaking package change in a minor version
release (0.8.0), so we need to pin to the last working release. See:
readthedocs/commonmark.py#134

Fixes:

```
  ...
  File ".../recommonmark/recommonmark/parser.py", line 9, in <module>
    from CommonMark import Parser
ImportError: No module named CommonMark
```

The Travis docs setup step has also been modified to always upgrade
dependencies, which will mean we will catch cases like this on Travis
next time.
edmorley added a commit to mozilla/treeherder that referenced this issue Sep 7, 2018
…3998)

CommonMark has made a breaking package change in a minor version
release (0.8.0), so we need to pin to the last working release. See:
readthedocs/commonmark.py#134

Fixes:

```
  ...
  File ".../recommonmark/recommonmark/parser.py", line 9, in <module>
    from CommonMark import Parser
ImportError: No module named CommonMark
```

The Travis docs setup step has also been modified to always upgrade
dependencies, which will mean we will catch cases like this on Travis
next time.
@anthraxx
Copy link

anthraxx commented Sep 7, 2018

@nikolas the symlink removal broke every Linux distro packaging and the PEP complience change actually ruined compatibility with anything that depends on the modules (like python-recommonmark)

What is your plan for the situation, commit to PEP complience without symlinks and force all depending packages to be fixed? This would be important to know before pinging such libraries to fix the import and at the end you maybe revert to cammel case module and depending packages need to revert as well.
Please clarify the situation with the module names so we can either start pinging people or just chill out until its back to the old one... either way a final decision is what i'm looking for.

@eli-schwartz
Copy link

eli-schwartz commented Sep 7, 2018

Thanks for the report, and apologies for the problem.

But... you're not fixing the problem?

The intention here was to maintain some backwards compatibility by making the CommonMark directory a symlink to commonmark. It looks like this isn't necessary, isn't working correctly with python packaging mechanisms, and also breaks compatibility with case-insensitive filesystems (biolab/orange3#3234, though.... what filesystem is case-insensitive? :P)

What is a symlink supposed to do in the first place? How is it not necessary to provide compatibility anyway?

I will just remove the CommonMark symlink for good - we should only be relying on commonmark for simplicity.

But then you completely break API in a point release which is not fixing things.

It's not exactly challenging to provide a module under two names. Since I guess Arch Linux won't be foolish enough to update to new versions of commonmark any time soon, I can trivially get the intention of your whole change, simply by creating a new, toplevel module called "commonmark.py" containing the following import:

from CommonMark import *

And unlike CommonMark 0.8.1 it works.

Please unbreak your everything.

Also, camelcase is not really offensive to me and I don't understand the original feature request in the first place... but it's unquestionably failed to do anything it intended at all. Instead you're seeing projects pin the old version or simply remove commonmark support at all due to intolerably working nowhere.

@nikolas
Copy link
Collaborator

nikolas commented Sep 10, 2018

@anthraxx yeah - the idea is if you're depending on the old CommonMark namespace then just stay at version 0.7.x.

@nikolas
Copy link
Collaborator

nikolas commented Sep 10, 2018

@eli-schwartz I have fixed the problem reported by @abulte: version 0.8.0 doesn't work on Mac OS.

Sorry everyone about the namespace confusion, I was holding off on pushing out this change because I was afraid of breaking everything, but basically here's where we're at:

  • Use commonmark 0.7.x if you rely on CommonMark package name.
  • Use commonmark >=0.8.1 if you're relying on the commonmark package name, which will be the norm from here on out.

Open specific tickets if you have any suggestions, but I'm closing this one because it looks like the package is working on Mac OS again, as of 0.8.1.

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

No branches or pull requests

6 participants