Skip to content

Use our own cpython clone.#1675

Merged
JulienPalard merged 1 commit intopython:3.10from
JulienPalard:mdk/makefile
Sep 25, 2021
Merged

Use our own cpython clone.#1675
JulienPalard merged 1 commit intopython:3.10from
JulienPalard:mdk/makefile

Conversation

@JulienPalard
Copy link
Copy Markdown
Member

Alternative to #1630

@JulienPalard JulienPalard force-pushed the mdk/makefile branch 3 times, most recently from e2bbabd to d979ffe Compare September 24, 2021 19:38
@jeanas
Copy link
Copy Markdown
Collaborator

jeanas commented Sep 24, 2021

J'aime beaucoup l'idée. Je pense juste qu'il faudrait mettre à jour le fichier CONTRIBUTING-CORE.rst.

@JulienPalard
Copy link
Copy Markdown
Member Author

J'aime beaucoup l'idée. Je pense juste qu'il faudrait mettre à jour le fichier CONTRIBUTING-CORE.rst.

Houla oui :)

C'est fait.

echo "You're missing dependencies please install:"; \
echo ""; \
echo " python -m pip install -r requirements.txt -r $(CPYTHON_PATH)/Doc/requirements.txt"; \
echo " python -m pip install -r requirements.txt -r venv/cpython/Doc/requirements.txt"; \
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Il y a deux approches différentes avec ce commit :

  • CPython est cloné automatiquement,
  • C'est l'utilisateur qui doit manuellement installer le reste des dépendances dans l'environnement virtuel.

Que penseriez-vous de tout migrer vers l'automatique ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

IIRC on le faisait à une époque mais ça pose deux-trois soucis :

  • Ça nécessite de préjuger de la commande à utiliser pour invoquer Python (python, python3, py)
  • Ça peut échouer, typiquement sur Debian et dérivés qui n'a pas venv dans le paquet Python (il l'ont découpé et mis dans le paquet python3-venv).
  • C'est compliqué de savoir si les dépendances sont à jour, ou s'il faut relancer le pip install, et c'est pas sympa de relancer le pip install à chaque fois.

Donc on s'est dit qu'on allait juste dire aux gens d'installer les choses avec un message suffisament précis pour tout dire mais suffisament vague pour qu'ils se débrouillent en fonction de leur environnement, préférences, ...

You're missing dependencies please install:
python -m pip install -r requirements.txt -r venv/cpython/Doc/requirements.txt

J'ai même retiré le conseil de le faire dans un venv, les gens sont libres, s'ils veulent utiliser autre chose, ou installer ça dans ~/.local, libre à eux, ça fonctionnera.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Le but de la PR n'est pas tant d'automatiser le clone de Python, mais plutôt d'utiliser notre propre clone et pas le clone de l'utilisateur, qui pourrait l'utiliser pour autre chose (comme, contribuer à Python), et qui pourrait être embêté par les git checkout en detached-head qu'on fait dessus. Ou qui pourrait être géné s'il a un travail en cours sur cpython, de ne pas pouvoir build la doc (le git checkout refusera de se faire si le dossier n'est pas propre).

Copy link
Copy Markdown
Collaborator

@jeanas jeanas Sep 25, 2021

Choose a reason for hiding this comment

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

IIRC on le faisait à une époque mais ça pose deux-trois soucis :

  • Ça nécessite de préjuger de la commande à utiliser pour invoquer Python (python, python3, py)
  • Ça peut échouer, typiquement sur Debian et dérivés qui n'a pas venv dans le paquet Python (il l'ont découpé et mis dans le paquet python3-venv).
  • C'est compliqué de savoir si les dépendances sont à jour, ou s'il faut relancer le pip install, et c'est pas sympa de relancer le pip install à chaque fois.

Houlà, je retire ce que j'ai dit.

Le but de la PR n'est pas tant d'automatiser le clone de Python, mais plutôt d'utiliser notre propre clone et pas le clone de l'utilisateur, qui pourrait l'utiliser pour autre chose (comme, contribuer à Python), et qui pourrait être embêté par les git checkout en detached-head qu'on fait dessus. Ou qui pourrait être géné s'il a un travail en cours sur cpython, de ne pas pouvoir build la doc (le git checkout refusera de se faire si le dossier n'est pas propre).

Oui, c'est sûr, je me demandais juste si on ne pouvait pas harmoniser, mais c'était une mauvaise idée.

@PyDocTeur
Copy link
Copy Markdown

Hello @JulienPalard ! Désolé, mais ton titre de pull request me semble invalide par rapport à ce que je suis programmé d'accepter.
Merci de le corriger ou d'ajouter le label meta si c'est une PR spéciale. Un exemple de titre valide serait : « Traduction de dossier/fichier.po ».


Disclaimer

Je suis un robot fait par l'équipe de l'AFPy et de Traduction
sur leur temps libre. Je risque de dire des bétises. Ne me blâmez pas, blamez les développeurs.

Code source

I'm a bot made by the Translation and AFPy teams on their free
time. I might say or do dumb things sometimes. Don't blame me, blame the developer !

Source code

(state: incorrect_title)
PyDocTeur v1.12.0

@jeanas
Copy link
Copy Markdown
Collaborator

jeanas commented Sep 25, 2021

(Je ne sais pas si le fait que je clique "Approve" change quoi que ce soit, mais cela correspond à mon sentiment.)

@PyDocTeur
Copy link
Copy Markdown

Hello @JulienPalard ! Désolé, mais ton titre de pull request me semble invalide par rapport à ce que je suis programmé d'accepter.
Merci de le corriger ou d'ajouter le label meta si c'est une PR spéciale. Un exemple de titre valide serait : « Traduction de dossier/fichier.po ».


Disclaimer

Je suis un robot fait par l'équipe de l'AFPy et de Traduction
sur leur temps libre. Je risque de dire des bétises. Ne me blâmez pas, blamez les développeurs.

Code source

I'm a bot made by the Translation and AFPy teams on their free
time. I might say or do dumb things sometimes. Don't blame me, blame the developer !

Source code

(state: incorrect_title)
PyDocTeur v1.12.0

@PyDocTeur
Copy link
Copy Markdown

C'est bien parti ! Ça manque d'un label automerge ?


Disclaimer

Je suis un robot fait par l'équipe de l'AFPy et de Traduction
sur leur temps libre. Je risque de dire des bétises. Ne me blâmez pas, blamez les développeurs.

Code source

I'm a bot made by the Translation and AFPy teams on their free
time. I might say or do dumb things sometimes. Don't blame me, blame the developer !

Source code

(state: approved)
PyDocTeur v1.12.0

@JulienPalard
Copy link
Copy Markdown
Member Author

JulienPalard commented Sep 25, 2021

(Je ne sais pas si le fait que je clique "Approve" change quoi que ce soit, mais cela correspond à mon sentiment.)

Du point de vue de Github, çàd de la possibilité de merger la PR ça ne change rien, seuls les "Approve" des commiters sont pris en compte, et on a pas le doit d'"Approve" nos propre PR non plus.

D'un point de vue humain tous les "Approve" sont pris en compte, donc c'est pas innutile du tout.

@JulienPalard JulienPalard merged commit 3392eba into python:3.10 Sep 25, 2021
@JulienPalard JulienPalard deleted the mdk/makefile branch September 25, 2021 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants