-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Error in updating deck by importing from Github URL (Anki 2.1.46) #138
Comments
Thanks for the bug report and I'm sorry that you're encountering the bug! This looks like a windows file permission or file locking problem, but I'm not 100% sure. I'll try to reproduce (and hopefully fix), but probably not in the next several days. (I don't usually use Windows, so I'll need to reboot etc.) (This might be a dulwich issue — perhaps we're not "closing" the repo always when we should? — I'll investigate. ) |
P.S. I reproduced the
Hopefully it might help you in fixing the error when you have the time! Thanks again for your patience. EDIT :
The issue listed at jelmer/dulwich#584 is the exact same error I'm facing. |
Otherwise, especially on Windows, we might end up with locked files. Fix Stvad#138. dulwich's pull can operate either on paths or on dulwich repo objects. Since it's using `open_repo_closing` internally, if it (and hence `open_repo_closing`) are passed a path, the associated repo object will be automatically closed after use, so we don't have to worry about anything. (If they're passed an already existing "repo object", as we had been doing previously, then, internally, they use `_noop_context_manager()`, instead of `closing()` and the repo object isn't closed.) (The above is true for current dulwich master (`80bffaca1dfe59a5ba57a045f3ce7122227a7647`) — see `dulwich/porcelain.py`.)
Thank you for your patience! :) (You're the one being affected by the bug!) Thanks also for the very detailed bug report! I could, indeed, reproduce the error on Windows. Like we both noted, the issue seems to be caused by the fact that the dulwich repo objects weren't being closed. I've fixed that in the linked PR (#139), and with that fix the error seems to have disappeared, at least for me! Unfortunately, there are, as you can see, quite a few open PRs, so it might take a while for an updated version of CrowdAnki to be released to AnkiWeb. In the mean-time, if you want to fix the bug on your system (and it'd be great if you tested whether the bug is indeed fixed — I might have missed something), you can manually patch try:
repo_local_path = self.get_repo_local_path(repo_url)
- porcelain.pull(porcelain.open_repo(str(repo_local_path)), repo_url)
+ porcelain.pull(str(repo_local_path), repo_url)
except ValueError: def clone_repository(self, repo_url, repo_path):
repo_path.mkdir(parents=True, exist_ok=True)
- porcelain.clone(repo_url, target=str(repo_path), bare=False, checkout=True, errstream=porcelain.NoneStream(),
- outstream=porcelain.NoneStream())
+ repo_object = porcelain.clone(repo_url, target=str(repo_path), bare=False, checkout=True, errstream=porcelain.NoneStream(),
+ outstream=porcelain.NoneStream())
+ repo_object.close()
(The third change is an unimportant trailing newline one.) (If something is unclear, or doesn't make sense, please ask.) (For clarity: I'm not merging most of the open PRs, yet, despite most of them being by me, because:
) |
I made the changes, but unfortunately it still throws an error. github_importer.py.txt
To recreate : import via GitHub URL, and do it again after the first import is successful. |
After a larger number of tests, I can also, again reproduce the error. I'm slightly confused, as I've now tried quite a few permutations of dulwich repo closing, and none has worked. (I've also realised that I had neglected to ensure that the repository was closed when archiving/snapshotting, but I can reproduce the issue without snapshotting enabled, so it's not the problem.) I'll try the following (at some point, in some order (1. will be tried first, but I'm not sure about the rest)):
Thanks for your patience! :) Edit: 1 seems to work: Updating dulwich together with the patch seems to resolve the issue (I now did more tests than the last time I declared victory, but I may still have missed something :)). Unfortunately, manually editing the source of the add-on isn't practicable in this case — you could try building |
I've tested the version with both the mentioned changes on our side and an updated If you want to check (I might have still missed something and behaviour might be different on different PCs) you could either build it yourself (using |
Diagnose #138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).)
Otherwise, especially on Windows, we might end up with locked files. Fix Stvad#138. dulwich's pull can operate either on paths or on dulwich repo objects. Since it's using `open_repo_closing` internally, if it (and hence `open_repo_closing`) are passed a path, the associated repo object will be automatically closed after use, so we don't have to worry about anything. (If they're passed an already existing "repo object", as we had been doing previously, then, internally, they use `_noop_context_manager()`, instead of `closing()` and the repo object isn't closed.) (The above is true for current dulwich master (`80bffaca1dfe59a5ba57a045f3ce7122227a7647`) — see `dulwich/porcelain.py`.)
Otherwise, especially on Windows, we might end up with locked files. Fix Stvad#138. dulwich's pull can operate either on paths or on dulwich repo objects. Since it's using `open_repo_closing` internally, if it (and hence `open_repo_closing`) are passed a path, the associated repo object will be automatically closed after use, so we don't have to worry about anything. (If they're passed an already existing "repo object", as we had been doing previously, then, internally, they use `_noop_context_manager()`, instead of `closing()` and the repo object isn't closed.) (The above is true for current dulwich master (`80bffaca1dfe59a5ba57a045f3ce7122227a7647`) — see `dulwich/porcelain.py`.)
Diagnose Stvad#138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).)
Diagnose Stvad#138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).)
Diagnose Stvad#138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).)
Diagnose Stvad#138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).) The monkey-patching of `get_repo_local_path` is ugly, but far less complex than partially mocking ConfigSettings.
Diagnose Stvad#138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (`PermissionError`).) The monkey-patching of `get_repo_local_path` is ugly, but far less complex than partially mocking ConfigSettings. Alternatively, we could have modified `get_repo_local_path` to have an extra, optional argument (say `full_snapshot_path` that'd override the config call), but modifying the code to fit the test is also ugly.
Otherwise, especially on Windows, we might end up with locked files. Fix Stvad#138. dulwich's pull can operate either on paths or on dulwich repo objects. Since it's using `open_repo_closing` internally, if it (and hence `open_repo_closing`) are passed a path, the associated repo object will be automatically closed after use, so we don't have to worry about anything. (If they're passed an already existing "repo object", as we had been doing previously, then, internally, they use `_noop_context_manager()`, instead of `closing()` and the repo object isn't closed.) (The above is true for current dulwich master (`80bffaca1dfe59a5ba57a045f3ce7122227a7647`) — see `dulwich/porcelain.py`.)
I've been trying to get my deck uploaded with CrowdAnki in my attempt to open community contributions for v1.0.0 of my deck which can be found here. While testing out the "Import git repository" feature, I am encountering a weird error again and again. Apologies if this has been explained earlier, but I've wracked my brain trying to understand the fixes suggested in past similar issues and I still can't resolve this myself.
Steps to reproduce the error
https://github.com/Raagaception/raagaception-12STD-CBSE-deck
- this works flawlessly for the first time, every time. Regardless of whether I first import an older version (v0.9.3) of my deck and update that, or I start out in a blank profile, the first import is always perfect.The resulting error(s)
The error
PermissionError: [WinError x]
wherex
can be5
,32
, or something else always shows up. And whenWinError 32
occurs (haven't found a reliable way to reproduce it), Anki doesn't allow to delete the demo profile which I created for testing purposes. The.git
folder mentioned in the error cannot be deleted, as Windows says it's "in use by another process".A system restart allows me to delete the demo profile, but the CrowdAnki import issue still remains.
Any ideas as to what I might be doing wrong? Thanks for your help and patience in advance!
The text was updated successfully, but these errors were encountered: