Skip to content
This repository was archived by the owner on Jan 10, 2023. It is now read-only.

Conversation

@Ocramius
Copy link
Contributor

@Ocramius Ocramius commented Feb 3, 2013

Fixes problem with executable in #53

To test it out:

  1. Create a new directory and add following composer.json file to it

    {
        "minimum-stability": "dev",
        "require": {
            "sebastian/phpcpd": "dev-feature/composer-support@dev"
        },
        "repositories": [
            {
                "type": "vcs",
                "url": "https://github.com/Ocramius/phpcpd.git"
            }
        ]
    }
  2. Run following:

    curl -s https://getcomposer.org/installer | php
    php composer.phar install
  3. Run the cli tools:

     ./vendor/bin/phpcpd vendor/sebastian/phpcpd/src

    should produce

     phpcpd 1.4.0-11-g458de68 by Sebastian Bergmann.
    
     0.00% duplicated lines out of 1597 total lines of code.
    
     Time: 0 seconds, Memory: 2.25Mb
    

Same works without defining the composer.json file and using the cloned repository instead.

Thomas Ploch and others added 2 commits February 5, 2013 11:54
@omares
Copy link

omares commented Feb 6, 2013

Hey there are various issues regarding composer but it seems there is still no composer support.
Out of curiosity: Whats the issue?

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 8, 2013

News here?

@omares see #53

@mikedfunk
Copy link

One of the few things in the PHP QA Toolchain not on Packagist yet. I look forward to it's arrival!

@Ocramius
Copy link
Contributor Author

Ocramius commented Mar 5, 2013

@sebastianbergmann any chance to get this merged? There were no changes to the repo other than this one

(obviously, as @mikedfunk said, the packagist hook is a must too :) )

@robocoder
Copy link

BTW the PR is missing a doc update to README.markdown.

@Ocramius
Copy link
Contributor Author

Ocramius commented Mar 5, 2013

@robocoder done

@cordoval
Copy link

cordoval commented Mar 6, 2013

👍

@JoshuaEstes
Copy link

ping @sebastianbergmann Status on getting this merged in?

@Ocramius
Copy link
Contributor Author

@robocoder that code block is ok :)

@robocoder
Copy link

@Ocramius just trying to avoid an erroneous include

btw can you follow this phploc convention of having a separate bin script for composer? https://github.com/sebastianbergmann/phploc/blob/master/composer/bin/phploc -- I think that would alleivate sebastian's concern as this would allow you to revert the change to phpcpd.php, and modify composer.json so composer users would have a vendor/bin/phpcpd (instead of vendor/bin/phpcpd.php).

@Ocramius
Copy link
Contributor Author

@robocoder if that helps bringing this in, well of course I will do it :)

@Ocramius
Copy link
Contributor Author

@sebastianbergmann now that the PR does not introduce changes to existing code, can we get this in? :)

@omares
Copy link

omares commented Apr 2, 2013

Does anyone have/know a maintained fork containing composer support which is available via packagist?

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 2, 2013

That would just be wrong. Don't even think of doing it. @sebastianbergmann
ping
On 2 Apr 2013 15:40, "Ota Mares" [email protected] wrote:

Does anyone have/know a maintained fork containing composer support which
is available via packagist?


Reply to this email directly or view it on GitHubhttps://github.com//pull/58#issuecomment-15775404
.

@omares
Copy link

omares commented Apr 2, 2013

I can accept and understand that sebastian has a lot todo but this is not a state we should keep, the issue is two months old and i dont see any reaction from sebastian side.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 2, 2013

@omares if you cannot wait for @sebastianbergmann then please use the "vcs" repository type as described in http://getcomposer.org/doc/04-schema.md#repositories and point it at git://github.com/Ocramius/phpcpd.git for now. Also, as far as I know he'll be on vacation for 5 weeks now.

@omares
Copy link

omares commented Apr 2, 2013

Thank you Ocramius.

@sebastianbergmann
Copy link
Owner

For PHPUnit it makes sense to have different versions of PHPUnit on a per-project basis but I still fail to see the point of installing tools such as PHPCPD via Composer. I will eventually merge this pull request once I had the time to test it. However, this is of very low priority to me.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 2, 2013

It's just like "pushing a button"? I actually personally need this for a CI
environment where I don't have access to PEAR

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 2 April 2013 16:18, Sebastian Bergmann [email protected] wrote:

For PHPUnit it makes sense to have different versions of PHPUnit on a
per-project basis but I still fail to see the point of installing tools
such as PHPCPD via Composer. I will eventually merge this pull request once
I had the time to test it. However, this is of very low priority to me.


Reply to this email directly or view it on GitHubhttps://github.com//pull/58#issuecomment-15777646
.

@sebastianbergmann
Copy link
Owner

Why not use the PHAR then? Just curious.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 2, 2013

Because I'd have to commit the phar to my repo (eww) or rely on the current
compiled version being downloaded during the build process.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 2 April 2013 16:23, Sebastian Bergmann [email protected] wrote:

Why not use the PHAR then? Just curious.


Reply to this email directly or view it on GitHubhttps://github.com//pull/58#issuecomment-15777999
.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 2, 2013

@sebastianbergmann yes, but there's better infrastructure to handle the downloads/dependencies in there.

  • if something conflicts with phpcpd, I get a failing build or a compatible set of tools (or a failure with some output that makes sense)
  • I don't have a hardcoded URI to a phpcpd.phar to be downloaded - if someone migrates the repo, the download location or if one of the download methods is unavailable, composer will attempt to fallback to other URIs/protocols
  • I don't need to re-download the phar every time, since the workspace is aware of the currently installed version, and can eventually use git to even speed up upgrades

As also @omares points out, committing a binary is kinda useless, and doesn't explain what is going on, while a diff does indeed show a change in the dependency constraints.

And yes, it's the current standard way of doing things: I already have problems explaining my co-workers how to use composer. I don't want to mix up other ways of doing things when there's one "good-ish" one that makes things simpler :)

@micha149
Copy link

micha149 commented Apr 4, 2013

👍 to get this on packagist. PHPCPD is a dependency to our development process. And dependencies has to be installable through the package manager. By the way: Its the last quality tool, which isn't on packagist yet. So, raise the priority @sebastianbergmann.

@sebastianbergmann
Copy link
Owner

I do not need stranger telling me how I should prioritize spending my free time, thank you very much.

How about those who are interested in getting this pull request merged try it out and post here "yes, works for me" or "no, does not work for me"? That would speed up the process as with enough positive feedback I would not need to try it out myself.

@RWOverdijk
Copy link

@sebastianbergmann Yes, it works for me. :) I've also verified the composer.json file and it is correct.

@GeeH
Copy link

GeeH commented Apr 4, 2013

👍 Works perfectly for me :)

@sebastianbergmann sebastianbergmann merged commit c9c1dbb into sebastianbergmann:master Apr 4, 2013
@Ocramius Ocramius deleted the feature/composer-support branch April 4, 2013 08:09
@micha149
Copy link

micha149 commented Apr 4, 2013

Wow, that was fast. THANK YOU!

@Ocramius Ocramius restored the feature/composer-support branch April 4, 2013 08:20
@sebastianbergmann
Copy link
Owner

Can you please test again after my recent changes? If all is well I'll roll a 1.4.1 release today or tomorrow. Thanks!

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 4, 2013

@sebastianbergmann will do asap

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 4, 2013

@sebastianbergmann looks good, nice improvement!

$ ./composer/bin/phpcpd ./src/
phpcpd 1.4.0-20-g5d83afa by Sebastian Bergmann.

0.00% duplicated lines out of 1518 total lines of code.

Time: 0 seconds, Memory: 2.75Mb

Can you register the package on packagist and setup the hook? https://packagist.org/packages/submit

@micha149
Copy link

micha149 commented Apr 4, 2013

I got an error caused by a static usage of \PHP_Timer::resourceUsage() in ResultPrinter Line 117. But PHP_Timer uses a reference on $this in the called method:
https://github.com/sebastianbergmann/php-timer/blob/master/PHP/Timer.php#L150

I've got this error on @Ocramius's branch, too.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 4, 2013

@micha149 are you on 5.4? I just tried it on a 5.4 box and cannot reproduce with either phpcpd as requirement or as base repo.

@micha149
Copy link

micha149 commented Apr 4, 2013

Oh, yes. I was. But after a switch to 5.3, I've got the same errors…

$ php -v                                
PHP 5.3.17 (cli) (built: Oct 30 2012 15:30:03) 
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
    with Xdebug v2.2.1, Copyright (c) 2002-2012, by Derick Rethans

The phpcpd script uses the right php executable.

@sebastianbergmann
Copy link
Owner

I'll look into it.

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 4, 2013

@micha149 can you please provide your current composer.json?

@micha149
Copy link

micha149 commented Apr 4, 2013

Here is the composer json, which should not differ from sebatians repository:

{
    "name": "sebastian/phpcpd",
    "description": "Copy/Paste Detector (CPD) for PHP code.",
    "homepage": "https://github.com/sebastianbergmann/phpcpd",
    "license": "BSD-3-Clause",
    "authors": [
        {
            "name": "Sebastian Bergmann",
            "email": "[email protected]",
            "role": "lead"
        }
    ],
    "support": {
        "issues": "https://github.com/sebastianbergmann/phpcpd/issues"
    },
    "require": {
        "php": ">=5.3.3",
        "zetacomponents/base": ">=1.8",
        "zetacomponents/console-tools": ">=1.6",
        "sebastian/finder-facade": "dev-master",
        "phpunit/php-timer": ">=1.0.4",
        "symfony/finder": ">=2.1.2",
        "theseer/fdomdocument": "dev-master",
        "sebastian/version": ">=1.0.0"
    },
    "minimum-stability": "dev",
    "autoload": {
        "classmap": [
            "src/"
        ]
    },
    "bin": [
        "composer/bin/phpcpd"
    ]
}

And here is the complete error:

PHP Fatal error:  Call to undefined method SebastianBergmann\PHPCPD\TextUI\ResultPrinter::timeSinceStartOfRequest() in /Users/mvengelshoven/Sites/phpcpd/vendor/phpunit/php-timer/PHP/Timer.php on line 150
PHP Stack trace:
PHP   1. {main}() /Users/mvengelshoven/Sites/phpcpd/composer/bin/phpcpd:0
PHP   2. SebastianBergmann\PHPCPD\TextUI\Command->main() /Users/mvengelshoven/Sites/phpcpd/composer/bin/phpcpd:72
PHP   3. SebastianBergmann\PHPCPD\TextUI\ResultPrinter->printResult() /Users/mvengelshoven/Sites/phpcpd/src/TextUI/Command.php:243
PHP   4. PHP_Timer->resourceUsage() /Users/mvengelshoven/Sites/phpcpd/src/TextUI/ResultPrinter.php:117

@sebastianbergmann
Copy link
Owner

Please try with my latest patch.

@micha149
Copy link

micha149 commented Apr 4, 2013

Yep. Now, it works great. Tested on 5.3.17 and 5.4.9

$ ./composer/bin/phpcpd ./src/
phpcpd 1.4.0-21-gee9900c by Sebastian Bergmann.

0.00% duplicated lines out of 1518 total lines of code.

Time: 0 seconds, Memory: 4.25Mb

@tommygnr
Copy link
Contributor

tommygnr commented Apr 4, 2013

@sebastianbergmann Thanks for merging this PR. I can't find the project on packagist. Have you registered it and set up a hook yet?

@hakre
Copy link

hakre commented Apr 4, 2013

@tommygnr: Can you create a more "kept fresh one" than https://packagist.org/packages/shrink/phpcpd and test if it works? Also it might be worth to keep shrink in the loop.

@tommygnr
Copy link
Contributor

tommygnr commented Apr 4, 2013

@hakre This isn't really something I can do. Once sebastian has registered phpcpd on packagist I do intend to advise @shrink to update or remove his fork

@tommygnr
Copy link
Contributor

tommygnr commented Apr 4, 2013

@hakre See what ocramius wrote in his description of this PR if you want to use a branch alias.

@hakre
Copy link

hakre commented Apr 5, 2013

@tommygnr: sorry to bother you, I did thought you were interested. I'm okay with the status-quo. comment deleted.

@tommygnr
Copy link
Contributor

tommygnr commented Apr 7, 2013

For anybody following this thread phpcpd is now available on packagist. Thanks Sebastian

@Ocramius
Copy link
Contributor Author

Ocramius commented Apr 7, 2013

Awesome! 👍

@vjc vjc mentioned this pull request Jul 19, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.