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

Support using the auto-detected name #355

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bwarden
Copy link
Member

@bwarden bwarden commented Mar 27, 2019

Rework to absorb a lot of the work we were doing in makefiles so that we
can avoid setting the package name manually in 'make autospecnew'.

@bwarden
Copy link
Member Author

bwarden commented Mar 27, 2019

You'll also need to modify Makefile.toplevel to:

  1. Remove the check for $(NAME)
  2. Remove $(MAKE) clone_$(NAME) and the mkdir/git block
  3. Don't write the Makefile (autospec will do that instead)
  4. Remove the --target and --name parameters from autospec
  5. Add the --package-dir packages/ param to autospec
  6. Remove the else block for $(NAME) already exists...

...and add git_pull to autospec.conf if you use a different git URI for pull than push.

@bwarden
Copy link
Member Author

bwarden commented Mar 27, 2019

The link-new-rpms and checkblacklist actions will also need to be modified/replaced at some point.

@bwarden bwarden force-pushed the use-detected-package-name branch from 00a7a22 to 28a8644 Compare March 29, 2019 15:56
@bwarden
Copy link
Member Author

bwarden commented Apr 1, 2019

@phmccarty, @bryteise Could I have your review, please?

autospec/config.py Outdated Show resolved Hide resolved
autospec/git.py Outdated Show resolved Hide resolved
autospec/tarball.py Show resolved Hide resolved
@bwarden bwarden force-pushed the use-detected-package-name branch 2 times, most recently from b6fc152 to 114e815 Compare April 9, 2019 16:10
@bwarden bwarden force-pushed the use-detected-package-name branch from 114e815 to 716b2fe Compare April 9, 2019 23:00
@bwarden bwarden force-pushed the use-detected-package-name branch from 716b2fe to e4719d0 Compare April 18, 2019 22:52
@bwarden bwarden force-pushed the use-detected-package-name branch from 473718d to 5cbdfdb Compare May 9, 2019 16:07
bwarden added 4 commits May 20, 2019 09:00
Rework to absorb a lot of the work we were doing in makefiles so that we
can avoid setting the package name manually in 'make autospecnew'.
When using github urls, the code tries to test the detected name against
the previously provided name. If we didn't provide a name, though, this
would throw an exception.
@bwarden bwarden force-pushed the use-detected-package-name branch from 5cbdfdb to 35665e2 Compare May 20, 2019 16:01
@bwarden
Copy link
Member Author

bwarden commented Jun 10, 2019

Remaining work, in case someone else has time to pick up the torch:

  • refactor config to read autospec.conf first (so we know the git URL patterns), then go back and read package configuration files after deciding what the package name and path are.
  • tackle creation of rpms subdir and hard-linking resulting RPMs. This is the last ${NAME} dependency in the common makefile.

@phmccarty
Copy link
Contributor

@bryteise Is this feature still in scope for autospec?

I think it would be nice to autodetect the package name, but due to the extra complexity introduced with the interaction with the common tooling, we were somewhat undecided about what needed to change in that tooling, and autospec, or both, to accommodate...

@bryteise
Copy link
Member

bryteise commented Mar 2, 2022

Hrm I'm a little unsure about all the bits that need to be reworked.

Because make autospecnew is probably the way an autodetected name would be made use of, getting the common tooling as much out of the way as possible of autospec doing what needs to be would be sensible. We could also we add a --get-package-name type option for autospec and then common tooling gets the name from that and reruns autospec with the full options like now under the hood.

@phmccarty
Copy link
Contributor

I agree that all the name detection logic should be handled by autospec. The idea of adding a --get-package-name flag to autospec to replace the NAME value for make autospecnew also sounds reasonable...

@phmccarty
Copy link
Contributor

phmccarty commented Mar 4, 2022

There was also some discussion at the time about autospec creating the package Makefile (instead of the common tooling) if it doesn't exist, so that consideration could be revisited as well.

Edit: One second look, this PR adds the makefile write routine, so disregard.

Edit 2: IIRC, at the time, I wasn't convinced that autospec writing the makefile was a good idea, since the primary user of it is the common tooling, not autospec. Perhaps autospec could write a separate makefile (say, Makefile.autospec) that would be fully owned by autospec, and the common tooling could source it -- a similar approach to how it handles Makefile.custom. For non-autospec packages, Makefile would define PKG_NAME, and for autospec packages, Makefile.autospec would define it, replacing the value from Makefile if it exists there. And if autospec cannot detect, or misdetects, the package name, the common tooling could fall back to use PKG_NAME defined in Makefile (or Makefile.custom) if that's easier / makes more sense, and propagate that value to autospec like it does now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants