This updates WORKSPACE to the latest subpar, and ./update_tools.sh. #40
Conversation
|
@duggelz This makes I'd like to reach a point where we have this done, so that CI makes sure the above passes (since LMK if you want to dig into this at all, I'm just trying to get HEAD into a good state. |
WORKSPACE
Outdated
| name = "subpar", | ||
| remote = "https://github.com/google/subpar", | ||
| tag = "1.0.0", | ||
| commit = "b1b6bd13ff80ede09f57539031cfb20459c28237", |
There was a problem hiding this comment.
I tagged 1.1.0 if you want to use that instead (You'll miss the CI changes which are irrelevant here).
rules_python/piptool.py
Outdated
| os.environ['PYTHONPATH'] = ':'.join(dirs_to_add + existing_pythonpath) | ||
| existing_pythonpath = os.environ.get('PYTHONPATH') | ||
| if existing_pythonpath: | ||
| dirs_to_add.append(existing_pythonpath.split(':')) |
There was a problem hiding this comment.
append will do the wrong thing here (adds the whole list as a single element of the first list), you want extend
There was a problem hiding this comment.
thanks, good catch.
| cert_path = os.path.join(tempfile.mkdtemp(), "cacert.pem") | ||
| with open(cert_path, "wb") as cert: | ||
| cert.write(pkgutil.get_data("pip._vendor.requests", "cacert.pem")) | ||
| argv = ["--disable-pip-version-check", "--cert", cert_path] + argv |
There was a problem hiding this comment.
Yes, this shouldn't be necessary, but ok to add them back in while I investigate more.
|
Oops, can I unapprove? See my comment about |
This also fixes a couple bugs I hit after regenerating piptool.par: - os.environ['PYTHONPATH'] may result in a KeyError - The cert expansion logic was removed, but things fail without it (after I ./update_tools.sh). I thought this was intentional (should the PAR changes have fixes this?), but I'm restoring for now to keep HEAD in a good state.
|
Updated, thanks. |
|
@duggelz To be clear, I'll hold off until you re-approve :) |
|
LGTM to me, submit. (I still can't find any button to unapprove or re-approve). |
3 participants
This also fixes a couple bugs I hit after regenerating
piptool.par:os.environ['PYTHONPATH']may result in aKeyError./update_tools.sh). I thought this was intentional (should the PAR changes have fixes this?), but I'm restoring for now to keepHEADin a good state.