Ticket #1934 (closed cage: fixed)

Opened 4 years ago

Last modified 4 years ago

Some Python programs added to tools/dev

Reported by: mikehh Owned by: jkeenan
Priority: normal Milestone:
Component: coding_standards Version: 2.11.0
Severity: medium Keywords:
Cc: Language: python
Patch status: applied Platform:

Description

Two Python programs have been added to tools/dev:

gdb-pp.py and gdb-pp-load.py

Now I am not objecting to Python programs per se, but we need to establish appropriate coding standards, if we are going to do this.

What about copyright, editing coda and so forth.

Another point, does this add to the dependencies needed to run, install parrot etc.

Cheers, Michael (mikehh)

Attachments

python.copyright.diff Download (5.3 KB) - added by jkeenan 4 years ago.
Have Parrot::Distribution report Python files; insert copyright notice into all .py files

Change History

in reply to: ↑ description ; follow-up: ↓ 2   Changed 4 years ago by jkeenan

  • owner set to jkeenan
  • component changed from none to coding_standards

Replying to mikehh:

Two Python programs have been added to tools/dev: gdb-pp.py and gdb-pp-load.py Now I am not objecting to Python programs per se,

Well, I amobjecting to these programs. Two years ago, we rewrote the remaining Python program in the distribution precisely so that we could reduce the number of requirements necessary to build Parrot. If we add Python programs to our repository, we have to add Python to the list of requirements needed to build/develop Parrot. That's a discussion that has to begin with a Trac ticket or a post to parrot-dev.

Mike, can you create a branch which stores these files, then back them out from master (making sure nothing breaks)?

Thank you very much.

kid51

in reply to: ↑ 1   Changed 4 years ago by plobsing

Replying to jkeenan:

Replying to mikehh:

Two Python programs have been added to tools/dev: gdb-pp.py and gdb-pp-load.py Now I am not objecting to Python programs per se,

Well, I amobjecting to these programs. Two years ago, we rewrote the remaining Python program in the distribution precisely so that we could reduce the number of requirements necessary to build Parrot. If we add Python programs to our repository, we have to add Python to the list of requirements needed to build/develop Parrot. That's a discussion that has to begin with a Trac ticket or a post to parrot-dev.

These are pretty-printers for gdb. They are by no means required to build, install, or make use of Parrot. They simply make debugging slightly nicer for developers with appropriately configured, recent versions of gdb.

Mike, can you create a branch which stores these files, then back them out from master (making sure nothing breaks)?

Please do not do this. It is not possible to provide this without python (short of convincing gdb to embed something else).

Thank you very much. kid51

follow-ups: ↓ 6 ↓ 7   Changed 4 years ago by jkeenan

Peter, this is a long-standing policy of the Parrot project. I can remember when we did have a configuration step, auto::python, which detected Python on a user's system. However, at that point we were down to exactly one instance of a Python program in the distribution -- the ops-renumbering program, IIRC -- and Coke had me rewrite that in Perl 5.

If you would like us to change that policy and once again require Python in order to build Parrot, then I suggest you post to parrot-dev. In the meantime, these files have to come out, particularly since we're facing our 3.0 release in less than two weeks.

Thank you very much.

kid51

follow-up: ↓ 8   Changed 4 years ago by coke

  • lang python deleted

kid51 - I think you missed comment #2 - These are not required to build parrot. They are merely to help when trying to use gdb, and are not required even then.

There's no problem with these being in tools/dev, though i'd love to see them commented to describe their function & use. (and passing codetest, if they're not already.)

  Changed 4 years ago by coke

(that is, codetest needs to keep passing - if it's doing so by not checking these files, that is fine.)

in reply to: ↑ 3   Changed 4 years ago by darbelo

Replying to jkeenan:

If you would like us to change that policy and once again require Python in order to build Parrot, then I suggest you post to parrot-dev.

The files in question are 100% unrelated to the parrot build process. They are extensions to GDB (written in GDB's extension language, python) and have no effect for people with no python or for people with a GDB that doesn't support this type of extensions.

They are the debugging equivalent of the emacs mode we ship in the distribution. This doesn't introduce a dependency on python any more than parrot.el introduces a dependency on emacs.

in reply to: ↑ 3   Changed 4 years ago by plobsing

Replying to jkeenan:

Peter, this is a long-standing policy of the Parrot project. I can remember when we did have a configuration step, auto::python, which detected Python on a user's system. However, at that point we were down to exactly one instance of a Python program in the distribution -- the ops-renumbering program, IIRC -- and Coke had me rewrite that in Perl 5.

What exactly is the policy? There is precedent for python code in the repo. examples/ contains no fewer than 11 python scripts.

If you would like us to change that policy and once again require Python in order to build Parrot, then I suggest you post to parrot-dev. In the meantime, these files have to come out, particularly since we're facing our 3.0 release in less than two weeks. Thank you very much. kid51

in reply to: ↑ 4 ; follow-up: ↓ 9   Changed 4 years ago by jkeenan

  • status changed from new to assigned

Replying to coke:

These are not required to build parrot. They are merely to help when trying to use gdb, and are not required even then. There's no problem with these being in tools/dev, though i'd love to see them commented to describe their function & use. (and passing codetest, if they're not already.)

Okay, I can live with that.

And, clearly, I spoke to soon, especially with not knowing what's in examples/, as plobsing++ pointed out in another comment.

So that brings us back to mikehh's original concern: What coding standards should be applied to such files? My guesses:

1. Copyright.

2. Coda -- though I'm not sure what the correct coda would be.

3. Linelength -- probably the same as applied to Perl 5 files.

Clearly, the tabs thing won't work for Python ;-)

Which others?

Thank you very much.

kid51

in reply to: ↑ 8   Changed 4 years ago by plobsing

Replying to jkeenan:

Replying to coke:

These are not required to build parrot. They are merely to help when trying to use gdb, and are not required even then. There's no problem with these being in tools/dev, though i'd love to see them commented to describe their function & use. (and passing codetest, if they're not already.)

Okay, I can live with that. And, clearly, I spoke to soon, especially with not knowing what's in examples/, as plobsing++ pointed out in another comment. So that brings us back to mikehh's original concern: What coding standards should be applied to such files? My guesses: 1. Copyright. 2. Coda -- though I'm not sure what the correct coda would be. 3. Linelength -- probably the same as applied to Perl 5 files. Clearly, the tabs thing won't work for Python ;-) Which others?

The lazy thing to do would be to declare our python codingstd to be PEP8 and put the onus of testing on the python contributors (e.i.: don't bother rolling our own conformance tests). It is well known, and already has tools for testing conformance.

I am hardly an experienced python developer, so take my words with the appropriate amount of salt.

Thank you very much. kid51

follow-up: ↓ 11   Changed 4 years ago by cotto

For any non-core language programs included with Parrot as optional components, we should require that they be licensed under the same terms as Parrot, but not to make any restrictions on their coding standards. The most I'd require would be that they contain instructions as to what the tool is, how it's used and what it needs to be run. I'd recommend that scripts follow whichever guidelines are most prevalent in that language's community (e.g. PEP8), but we're not getting into the business of enforcing coding standards on non-core languages. (By non-core language, I mean anything that's not one of the major languages used by Parrot, essentially PIR, PASM, nqp-rx, C, Perl 5, etc.)

Non-core language tools must never be a required part of the build. If they're in git at all there needs to be a good reason why they can't be implemented in a core language. In this case gdb only includes Python bindings, so that's the only option for nicer debugging. We should consider these kinds of tools an optional and unsupported part of Parrot. If they work it's nice, but if they're broken they won't mess anything up.

short answer: keep them for now with no guarantees, let them be maintained as the committer sees fit and if they bitrot and nobody fixes them up, out they go.

I'll close this ticket after a while if there are no objections.

in reply to: ↑ 10   Changed 4 years ago by jkeenan

Replying to cotto:

short answer: keep them for now with no guarantees, let them be maintained as the committer sees fit and if they bitrot and nobody fixes them up, out they go.

I presume we have to enforce the copyright requirement. Are there any others which do need specific enforcement? What, if anything needs to be added to docs/pdds/pdd07_codingstd.pod?

Changed 4 years ago by jkeenan

Have Parrot::Distribution report Python files; insert copyright notice into all .py files

  Changed 4 years ago by jkeenan

I have attached a patch, python.copyright.diff, that does two things:

1. Adds functionality to lib/Parrot/Distribution.pm to locate Python files. As a first, rudimentary step, we confine our search to files ending in .py.

2. Adds the Parrot Foundation copyright notice to all .py files currently found in the distribution.

Please review. Please tell me if the addition of the copyright line in a comment beginning with a # mark will interfere with any of the examples or with the functioning of the two files added by Nolan Lum to facilitate pretty-printing gdb output.

Thank you very much.

kid51

follow-up: ↓ 14   Changed 4 years ago by cotto

Thanks, kid51. We can test and apply the patch to trunk after the 3.0 release. It's getting close to the release at this point and I don't want to change anything that doesn't need to be changed.

in reply to: ↑ 13   Changed 4 years ago by jkeenan

  • lang set to python
  • patch set to applied

Replying to cotto:

Thanks, kid51. We can test and apply the patch to trunk after the 3.0 release.

This is the kind of patch where, if there are bugs, we won't know until we get reports from the wild. So I applied the patch tonight.

[master de841c2] [codingstd] Add functionality to identify Python files. Test those files for copyright.

I will keep the ticket open for 2-3 days more for comments and complaints.

Thank you very much.

kid51

  Changed 4 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Yuki`N++ has confirmed that the addition of the copyright comment does no harm to the two .py files included to work with gdb pretty-printing. So I am closing this ticket.

Thank you very much. kid51

Note: See TracTickets for help on using tickets.