- 
                Notifications
    
You must be signed in to change notification settings  - Fork 81
 
DOC: minor tweak to docs on invoking pypa/build #707
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -228,7 +228,8 @@ Building the project | |
| 
     | 
||
| Before continuing, ensure you have committed the three files we created so far | ||
| to your Git repository - ``meson-python`` will only take into account the files | ||
| that Git knows about. | ||
| that Git knows about, and an sdist is created from the most recent commit - | ||
| uncommitted changes are ignored. | ||
| 
     | 
||
| To generate the distribution artifacts we will use the `pypa/build`_ tool. It | ||
| will create a temporary virtual environment, install all the required build | ||
| 
        
          
        
         | 
    @@ -240,6 +241,9 @@ dependencies, and ask ``meson-python`` to build the artifacts. | |
| $ python -m build | ||
| 
     | 
||
| If the build succeeded, you'll have the binary artifacts in the ``dist`` folder. | ||
| Note that by default, ``python -m build`` builds an sdist first, and then a | ||
| wheel from the sdist. If you only want one artifact, add ``--sdist`` or | ||
| ``--wheel`` to the invocation. | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this misses the main point. The issue in not the extra work for building an sdist that will not be used, but that doing so any uncommitted change in the repository is not reflected in the wheel. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good point, will add that as the first point since it's indeed a footgun. I think both can be relevant; sdist creation for a large project can be really slow (a quick unscientific measurement says 40 seconds for  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a note on uncommitted changes two paragraphs up, where it already talks about Git.  | 
||
| 
     | 
||
| .. admonition:: Building wheels for multiple platforms | ||
| :class: tip | ||
| 
          
            
          
           | 
    ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is very confusing. What is committed only affects what gets into an sdist, but this section is actually about building a wheel. I think this paragraph needs to be removed and the note about uncommitted changes moved below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about both sdist and wheels. It's part of the "Creating your first release" section, and clearly does talk about both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to change it to an admonition while we're at it, but I really just want to get this PR in - it was about adding the
--wheelflag and not related to the commit status of files at all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not make this paragraph worse, so we can merge this PR and fix this part of the documentation later.