Skip to content
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

Add build_distinfo command to packaging #56488

Closed
merwok opened this issue Jun 7, 2011 · 16 comments
Closed

Add build_distinfo command to packaging #56488

merwok opened this issue Jun 7, 2011 · 16 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@merwok
Copy link
Member

merwok commented Jun 7, 2011

BPO 12279
Nosy @carljm, @merwok

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/merwok'
closed_at = <Date 2011-07-22.00:58:12.473>
created_at = <Date 2011-06-07.15:48:52.901>
labels = ['type-feature']
title = 'Add build_distinfo command to packaging'
updated_at = <Date 2011-07-22.00:58:12.472>
user = 'https://github.com/merwok'

bugs.python.org fields:

activity = <Date 2011-07-22.00:58:12.472>
actor = 'eric.araujo'
assignee = 'eric.araujo'
closed = True
closed_date = <Date 2011-07-22.00:58:12.473>
closer = 'eric.araujo'
components = ['Distutils2']
creation = <Date 2011-06-07.15:48:52.901>
creator = 'eric.araujo'
dependencies = []
files = []
hgrepos = []
issue_num = 12279
keywords = ['gsoc']
message_count = 16.0
messages = ['137842', '137910', '137968', '138179', '138196', '140022', '140132', '140133', '140134', '140145', '140169', '140188', '140196', '140204', '140749', '140851']
nosy_count = 5.0
nosy_names = ['carljm', 'eric.araujo', 'alexis', 'michael.mulich', 'higery']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12279'
versions = ['Python 3.3']

@merwok
Copy link
Member Author

merwok commented Jun 7, 2011

In the current packaging module, the PEP-376 .dist-info directory is generated at install time. It should be split into two phases, build_distinfo and install_distinfo, to support at least two use cases:

  • the develop command, which needs access to .dist-info
  • the resources API (packaging.database.get_file), which is intended to work from an uninstalled codebase too.

The normal directory to put the generated files would be the build directory; however, setuptools’ egg_info command generates the file in the same directory as the setup.py file. I’d prefer we avoid it in packaging, for consistency with other build_* commands, and to avoid that every project has to put “*.dist-info” in their VCS ignore file, but I don’t know if there is a technical reason that constrains setuptools to do so.

@merwok merwok self-assigned this Jun 7, 2011
@merwok merwok added the type-feature A feature request or enhancement label Jun 7, 2011
@higery
Copy link
Mannequin

higery mannequin commented Jun 8, 2011

Now, the workaround of my code is just setting the 'distinfo-dir' option with os.curdir value through calling a 'reinitialize_command(self, command, reinit_subcommands=False, **kw)' function , which is added in packaging's Command class for my own purpose, but I think it should be taken as a standard function in Command - it's useful and setuptools' Command class does have this function.

@merwok
Copy link
Member Author

merwok commented Jun 9, 2011

packaging.command.cmd already has this method: it was renamed to get_reinitialized_function, to better match other method names.

the workaround of my code is just setting the 'distinfo-dir' option
with os.curdir value
Yes, that’s a workaround :) Have you tested writing to build.build_lib (see my previous message)? The test command for example runs the tests in the build dir, to test what will really get packaged, and to allow build-time 2to3 conversion.

One part of properly implementing a build_distinfo command that can be tricky is the .dist-info/RECORD file. If you just move the code from install_distinfo, the locations in the RECORD file will not be the actual locations of the files. Possibilities:

  1. don’t generate RECORD at all → invalid PEP-376
  2. generate RECORD with paths to the files in the build dir
  3. other?

@merwok
Copy link
Member Author

merwok commented Jun 11, 2011

I forgot one thing: setuptools’ egg_info command does write a list of paths, so we can look at how it solved the problem with the RECORD and RESOURCES files.

Higery: Michael is willing to work with you on this bug.

@higery
Copy link
Mannequin

higery mannequin commented Jun 12, 2011

Higery: Michael is willing to work with you on this bug.

OK. :)

@merwok
Copy link
Member Author

merwok commented Jul 8, 2011

I forgot one thing: setuptools’ egg_info command does write a list of
paths, so we can look at how it solved the problem with the RECORD and
RESOURCES files.

I was wrong: I just checked the output of egg_info, and it does not generate files with paths.

So, what do we do?

  1. don’t generate RECORD at all → invalid PEP-376
  2. generate RECORD with paths to the files in the build dir
  3. other?
  4. don’t add a build_distinfo command, just run install_distinfo to the build dir from the test and develop commands

@michaelmulich2
Copy link
Mannequin

michaelmulich2 mannequin commented Jul 11, 2011

We have to generate a RECORD, otherwise resource lookups in
development and testing modes will fail or at least should fail.

Yes, but that's not all.

  1. don’t add a build_distinfo command, just run install_distinfo to the build dir from the test and develop commands

The action needs to be called with an install, develop or test
context, so I think item four is our best option.

@michaelmulich2
Copy link
Mannequin

michaelmulich2 mannequin commented Jul 11, 2011

Gmail decided to strip the quotes... Grr...

> So, what do we do?
> 1) don’t generate RECORD at all → invalid PEP-376
We have to generate a RECORD, otherwise resource lookups in
development and testing modes will fail or at least should fail.

> 2) generate RECORD with paths to the files in the build dir
Yes, but that's not all.

> 4) don’t add a build_distinfo command, just run install_distinfo
to the build dir from the test and develop commands

The action needs to be called with an install, develop or test
context, so I think item four is our best option.

@merwok
Copy link
Member Author

merwok commented Jul 11, 2011

(It was probably the Roundup quotes bug.)

The action needs to be called with an install, develop or test
context, so I think item four is our best option.

What do you mean with context? Wouldn’t all three commands just make install_distinfo generate files in the build dir?

I think that option 4 is the most inelegant, and would be sad to see it win.

@michaelmulich
Copy link
Mannequin

michaelmulich mannequin commented Jul 11, 2011

On Mon, Jul 11, 2011 at 11:43 AM, Éric Araujo <report@bugs.python.org> wrote:
> What do you mean with context?  Wouldn’t all three commands just
make install_distinfo generate files in the build dir?

Right, my context comment is invalid.

> I think that option 4 is the most inelegant, and would be sad to see it win.
Ok... I forgot that we now have this RESOURCES file, which makes
leaving out the RECORD file less important, but still invalid based on
PEP-376.

So if we include the RECORD file (point number 2) without the checksum
and size (two columns in the RECORD csv format), we will still be
PEP-376 valid (maybe?), but the file verification information will be
missing. And we don't really want this information because if we edit
a file, the checksum and size will be incorrect anyhow. This missing
information is not important when using the develop or test commands,
because we are running the commands on a trusted local copy. What are
the consequences of not writing the checksum or size to the RECORD
file? And does that solve the issue?

@carljm
Copy link
Member

carljm commented Jul 12, 2011

You guys are more familiar with the codebase than I am, but it seems to me that the RECORD file should clearly either be not present or empty when metadata has been built but not yet installed. I don't really think the "invalid PEP-376" issue is a problem: PEP-376 describes the metadata for installed distributions; it has nothing to say about built metadata for a distribution which has not yet been installed.

For purposes of the develop command, if a pth file is used to implement develop, then ideally when develop is run a RECORD file would be added containing only the path to that pth file, as thats the only file that has actually been installed (and the only one that should be removed if the develop-installed package is uninstalled).

@merwok
Copy link
Member Author

merwok commented Jul 12, 2011

So if we include the RECORD file (point number 2) without the checksum
and size (two columns in the RECORD csv format),
Well, three columns, the last one being empty.

we will still be PEP-376 valid (maybe?), but the file verification
information will be missing. And we don't really want this
information because if we edit a file, the checksum and size will be
incorrect anyhow. This missing information is not important when
using the develop or test commands, because we are running the
commands on a trusted local copy.
Good thinking.

What are the consequences of not writing the checksum or size to the
RECORD file? And does that solve the issue?
I think checksum was intended for use by uninstallers, so we’re good. I don’t know why the size is included.

I don't really think the "invalid PEP-376" issue is a problem: PEP
376 describes the metadata for installed distributions; it has
nothing to say about built metadata for a distribution which has not
yet been installed.
The problem is that develop is a kind of install.

For purposes of the develop command, if a pth file is used to
implement develop, then ideally when develop is run a RECORD file
would be added containing only the path to that pth file, as thats
the only file that has actually been installed
Yeah!

(and the only one that should be removed if the develop-installed
package is uninstalled).
Are you saying that such a RECORD file would allow any installer compatible with PEP-376 to undo a develop install? Clever!

@merwok
Copy link
Member Author

merwok commented Jul 12, 2011

About writing dist-info files into the build dir or the project root dir, see msg140195

@carljm
Copy link
Member

carljm commented Jul 12, 2011

> I don't really think the "invalid PEP-376" issue is a problem: PEP
> 376 describes the metadata for installed distributions; it has
> nothing to say about built metadata for a distribution which has not
> yet been installed.
The problem is that develop is a kind of install.

Right, I was simply referring to "build_distinfo" leaving it
empty/missing; I'd want "develop" to add a (very short) RECORD file as
specified below.

> For purposes of the develop command, if a pth file is used to
> implement develop, then ideally when develop is run a RECORD file
> would be added containing only the path to that pth file, as thats
> the only file that has actually been installed
Yeah!

> (and the only one that should be removed if the develop-installed
> package is uninstalled).
Are you saying that such a RECORD file would allow any installer compatible with PEP-376 to undo a develop install? Clever!

Yeah, that's the idea. I don't see any actual use case for having all of
the Python modules etc included in the RECORD file for a
develop-install, because they haven't been installed anywhere: what we
really want to know is "what has been placed in the installation
location that we need to keep track of."?

@merwok
Copy link
Member Author

merwok commented Jul 20, 2011

I temporarily retract my request for addition of build_distinfo.

Other build_spam/install_spam commands have clear responsibilities: build creates files, install moves them. This is the classic make/make install division of work, where you may want to run build frequently when updating C code, and you may want to run install as root.

For the dist-info files, we’ve seen that the RECORD file cannot be generated at build time, only install time, so splitting a build_distinfo command out of install_distinfo does not make sense.

The develop command can continue to call install_distinfo in the build dir, writing a RECORD file containing only the path to the pth file. (Let’s continue that discussion on the develop bug.)

For the test command, we could either require people to run develop, or run install_distinfo in the build dir. I think the latter is nicer. Who wants to make a patch for that?

For the resources API, which should work even in an unbuilt checkout or unarchived tarball, that’s another bug.

@merwok
Copy link
Member Author

merwok commented Jul 22, 2011

I kept this report open to address “test command needs dist-info”, but there’s already a report about that (bpo-12302) and I think it’s clearer to keep this one closed as a discussion archive linked from the develop bug.

@merwok merwok closed this as completed Jul 22, 2011
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants