PR#25: Module name and stream in project settings - copr/copr

copr / copr

#25 Module name and stream in project settings

Closed 8 years ago by frostyx. Opened 8 years ago by frostyx.

https://github.com/FrostyX/copr.git modularity-settings into master

[frontend] add placeholders for module name and stream

Jakub Kadlčík • 8 years ago

8bde236

[frontend] add info about module stream

Jakub Kadlčík • 8 years ago

26c6b5d

[frontend] move module name and stream to project settings

Jakub Kadlčík • 8 years ago

647a136

[frontend] module name and stream should not be empty

Jakub Kadlčík • 8 years ago

3c77b04

frontend/coprs_frontend/alembic/versions/178a3754a9b9_add_modularity_properites_to_copr.py

file added

+28

		`@@ -0,0 +1,28 @@`
		`+ """Add modularity properites to copr`
		`+`
		`+ Revision ID: 178a3754a9b9`
		`+ Revises: 3b67c52f5277`
		`+ Create Date: 2017-01-09 20:32:16.195039`
		`+`
		`+ """`
		`+`
		`+ # revision identifiers, used by Alembic.`
		`+ revision = '178a3754a9b9'`
		`+ down_revision = '3b67c52f5277'`
		`+`
		`+ from alembic import op`
		`+ import sqlalchemy as sa`
		`+`
		`+`
		`+ def upgrade():`
		`+ ### commands auto generated by Alembic - please adjust! ###`
		`+ op.add_column('copr', sa.Column('module_name', sa.String(length=100), nullable=True))`
		`+ op.add_column('copr', sa.Column('module_stream', sa.String(length=100), nullable=True))`
		`+ ### end Alembic commands ###`
		`+`
		`+`
		`+ def downgrade():`
		`+ ### commands auto generated by Alembic - please adjust! ###`
		`+ op.drop_column('copr', 'module_stream')`
		`+ op.drop_column('copr', 'module_name')`
		`+ ### end Alembic commands ###`

frontend/coprs_frontend/coprs/forms.py

file modified

+27 -2

		`@@ -677,6 +677,31 @@`
		`auto_prune = wtforms.BooleanField(validators=[wtforms.validators.Optional()])`


		`+ class CoprModularityFormFactory(object):`
		`+ @staticmethod`
		`+ def create_form_cls(copr, _changeable=None):`
		`+ """`
		`+ :param _changeable: Function to determine, whether module attribute can be changed.`
		+ See ``CoprsLogic.changeable``
		`+ """`
		`+ class F(wtf.Form):`
		`+ name = wtforms.StringField('Name', validators=[NameNotNumberValidator()])`
		`+ stream = wtforms.StringField('Stream', validators=[NameNotNumberValidator()])`
		`+ changeable = _changeable`
		`+`
		`+ def validate(self):`
		`+ if not wtf.Form.validate(self):`
		`+ return False`
		`+`
		`+ for copr_attr, form_attr in [("module_name", "name"), ("module_stream", "stream")]:`
		`+ if getattr(self, form_attr).data and not self.changeable(copr, copr_attr):`
		`+ self.errors[form_attr] = ["Attribute {} cannot be changed".format(form_attr)]`
		`+ return False`
		`+`
		`+ return True`
		`+ return F`
		`+`
		`+`
		`class CoprForkFormFactory(object):`
		`@staticmethod`
		`def create_form_cls(copr, user, groups):`
		`@@ -746,8 +771,8 @@`


		`class CreateModuleForm(wtf.Form):`
		`- name = wtforms.StringField("Name")`
		`- stream = wtforms.StringField("Stream")`
		`+ name = wtforms.StringField("Name", validators=[wtforms.validators.DataRequired("Please set module name in project settings")])`
		`+ stream = wtforms.StringField("Stream", validators=[wtforms.validators.DataRequired("Please set module stream in project settings")])`
		`version = wtforms.IntegerField("Version")`
		`filter = wtforms.FieldList(wtforms.StringField("Package Filter"))`
		`api = wtforms.FieldList(wtforms.StringField("Module API"))`

frontend/coprs_frontend/coprs/logic/coprs_logic.py

file modified

		`@@ -350,6 +350,12 @@`
		`raise exceptions.InsufficientRightsException(`
		`"Only owners may delete their projects.")`

		`+ @classmethod`
		`+ def changeable(cls, copr, modularity_property):`
		`+ if not getattr(copr, modularity_property):`
		`+ return True`
		`+ return not copr.modules`
		`+`

		`class CoprPermissionsLogic(object):`
		`@classmethod`

frontend/coprs_frontend/coprs/models.py

file modified

		`@@ -225,6 +225,10 @@`
		`# if backend deletion script should be run for the project's builds`
		`auto_prune = db.Column(db.Boolean, default=True, nullable=False, server_default="1")`

		`+ # modularity properties`
		`+ module_name = db.Column(db.String(100))`
		`+ module_stream = db.Column(db.String(100))`
		`+`
		`__mapper_args__ = {`
		`"order_by": created_on.desc()`
		`}`

frontend/coprs_frontend/coprs/templates/_helpers.html

file modified

+1 -1

		`@@ -217,7 +217,7 @@`
		`{{- url_for(view, username=copr.user.name, coprname=copr.name, **kwargs) }}`
		`{%- else %}`
		`{#- Only the listed views have merged functions for regular and group projects. Once all views have it, @TODO remove the workaround #}`
		`- {%- if view in ["coprs_ns.copr_add_package", "coprs_ns.copr_new_package", "coprs_ns.copr_rebuild_package", "coprs_ns.copr_packages", "coprs_ns.copr_package", "coprs_ns.copr_edit_package", "coprs_ns.copr_fork", "coprs_ns.copr_delete_package", "coprs_ns.copr_create_module", "coprs_ns.copr_module_raw", "coprs_ns.generate_module_repo_file", "coprs_ns.copr_modules", "coprs_ns.copr_module", "coprs_ns.copr_package_icon"] %}`
		`+ {%- if view in ["coprs_ns.copr_add_package", "coprs_ns.copr_new_package", "coprs_ns.copr_rebuild_package", "coprs_ns.copr_packages", "coprs_ns.copr_package", "coprs_ns.copr_edit_package", "coprs_ns.copr_fork", "coprs_ns.copr_delete_package", "coprs_ns.copr_create_module", "coprs_ns.copr_module_raw", "coprs_ns.generate_module_repo_file", "coprs_ns.copr_modules", "coprs_ns.copr_module", "coprs_ns.copr_package_icon", "coprs_ns.copr_modularity"] %}`
		`{%- set group_view = view %}`
		`{%- else %}`
		`{%- set group_view = view.split(".")[0] + ".group_" + view.split(".")[-1] %}`

frontend/coprs_frontend/coprs/templates/coprs/_coprs_forms.html

file modified

+21

		`@@ -186,6 +186,27 @@`
		`</form>`
		`{% endmacro %}`

		`+ {% macro copr_modularity_form(form, copr) %}`
		`+ <form method="post" action="">`
		`+ {{ render_form_errors(form) }}`
		`+ <div class="panel panel-default">`
		`+ <div class="panel-heading">`
		`+ <h3 class="panel-title">1. Module naming</h3>`
		`+ </div>`
		`+ <div class="panel-body">`
		`+`
		`+ {% set warning = 'This property will not be possible to change after building first module from the project' %}`
		`+ {{ render_field(form.name, label='Module Name', disabled=(not form.changeable(copr, 'module_name')), placeholder='May be same as copr name', info=warning) }}`
		`+ {{ render_field(form.stream, label='Module Stream', disabled=(not form.changeable(copr, 'module_stream')), placeholder='e.g. master', info=[warning,`
		`+ 'Each version of a module has its own independent update stream associated with it. Module updates within the stream should be backwards compatible and should not change ABI. See <a href="https://fedoraproject.org/wiki/Modularity/Architecture/Module_versioning_and_branching/Basic_branching_terminology">naming and versioning wiki</a>.']) }}`
		`+`
		`+ {{ form.csrf_token }}`
		`+ <input class="btn btn-primary pull-right" type="submit" value="Save">`
		`+ </div>`
		`+ </div>`
		`+ </form>`
		`+ {% endmacro %}`
		`+`
		`{% macro copr_permissions_form(form, copr, permissions) %}`
		`{% if permissions %}`
		`<form action="{{ url_for('coprs_ns.copr_update_permissions', username=copr.user.name, coprname=copr.name) }}" method=post>`

frontend/coprs_frontend/coprs/templates/coprs/create_module.html

file modified

+11 -7

		`@@ -30,6 +30,13 @@`
		`<strong>Building modules is experimental and in early stage.</strong> At the moment, the UI will not show you the modules you have built.`
		`</div>`

		`+ {% if not copr.module_name or not copr.module_stream %}`
		`+ <div class="alert alert-info">`
		`+ <span class="pficon pficon-info"></span>`
		`+ Please, set <code>name</code> and <code>stream</code> properties in the <a href="{{ copr_url('coprs_ns.copr_modularity', copr) }}">project settings</a>, first.`
		`+ </div>`
		`+ {% endif %}`
		`+`
		`<h2>Build Module</h2>`
		`<p> Build a module from your copr repository. Copr will generate a modulemd yaml for your module automatically. <a href="https://fedoraproject.org/wiki/Modularity" class="alert-link" target="_blank">Learn more about Modularity</a>.`

		`@@ -44,16 +51,13 @@`
		`<div class="row">`
		`<div class="form-group col-sm-4">`
		`<label for="moduleName">Name:</label>`
		`- <input type="text" class="form-control" id="moduleName" placeholder="{{ copr.name }}" disabled>`
		`- <input type="hidden" value="{{ copr.name }}" name="name">`
		`+ {{ render_field(form.name, id="moduleName", disabled=True, value=(copr.module_name or '')) }}`
		`+ <input type="hidden" name="name" value="{{ copr.module_name or '' }}">`
		`</div>`
		`<div class="form-group col-sm-4">`
		`<label for="moduleRelease">Stream:</label>`
		`- {{ render_field(form.stream, id="moduleStream") }}`
		`- </div>`
		`- <div class="form-group col-sm-4">`
		`- <label for="moduleVersion">Version:</label>`
		`- {{ render_field(form.version, id="moduleVersion") }}`
		`+ {{ render_field(form.stream, id="moduleStream", disabled=True, value=(copr.module_stream or '')) }}`
		`+ <input type="hidden" name="stream" value="{{ copr.module_stream or '' }}">`
		`</div>`
		`</div>`
		`</div>`

frontend/coprs_frontend/coprs/templates/coprs/detail/settings.html

file modified

		`@@ -36,6 +36,10 @@`
		`{{ tab("edit", "Project Details", copr_url('coprs_ns.copr_edit', copr)) }}`
		`{% endif %}`

		`+ {% if g.user.can_edit(copr) %}`
		`+ {{ tab("modularity", "Modularity", copr_url('coprs_ns.copr_modularity', copr)) }}`
		`+ {% endif %}`
		`+`
		`{% if not copr.is_a_group_project %}`
		`{{ tab("permissions", "Permissions", copr_url('coprs_ns.copr_permissions', copr)) }}`
		`{% endif %}`

frontend/coprs_frontend/coprs/templates/coprs/detail/settings/modularity.html

file added

+13

		`@@ -0,0 +1,13 @@`
		`+ {% extends "coprs/detail/settings.html" %}`
		`+ {% from "coprs/_coprs_forms.html" import copr_modularity_form %}`
		`+`
		`+ {% set selected_monitor_tab = "modularity" %}`
		`+ {%block settings_breadcrumb %}Modularity Details{% endblock %}`
		`+`
		`+ {% block tab_content %}`
		`+`
		`+ <h2> Modularity Details </h2>`
		`+ <p> Edit modularity settings for this project. Please be aware that some of this settings might not be possible to change after building a first module from this project. </p>`
		`+ {{ copr_modularity_form(form, copr) }}`
		`+`
		`+ {% endblock %}`

frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py

file modified

+43 -3

		`@@ -442,6 +442,46 @@`
		`return render_copr_edit(copr, form, 'coprs_ns.copr_update')`


		`+`
		`+ @coprs_ns.route("/<username>/<coprname>/modularity/")`
		`+ @coprs_ns.route("/g/<group_name>/<coprname>/modularity/")`
		`+ @login_required`
		`+ @req_with_copr`
		`+ def copr_modularity(copr, form=None):`
		`+ return render_copr_modularity(copr, form)`
		`+`
		`+`
		`+ def render_copr_modularity(copr, form):`
		`+ if not form:`
		`+ form = forms.CoprModularityFormFactory.create_form_cls(copr, CoprsLogic.changeable)()`
		`+`
		`+ for key in ["name", "stream"]:`
		`+ getattr(form, key).data = getattr(form, key).data or getattr(copr, "module_" + key)`
		`+`
		`+ return flask.render_template(`
		`+ "coprs/detail/settings/modularity.html",`
		`+ copr=copr, form=form, view='coprs_ns.copr_modularity')`
		`+`
		`+`
		`+ @coprs_ns.route("/<username>/<coprname>/modularity/", methods=["POST"])`
		`+ @coprs_ns.route("/g/<group_name>/<coprname>/modularity/", methods=["POST"])`
		`+ @login_required`
		`+ @req_with_copr`
		`+ def copr_modularity_post(copr):`
		`+ form = forms.CoprModularityFormFactory.create_form_cls(copr, CoprsLogic.changeable)()`
		`+ if form.validate_on_submit():`
		`+ prefix = "module_"`
		`+ for key in ["name", "stream"]:`
		`+ if CoprsLogic.changeable(copr, prefix + key):`
		`+ setattr(copr, prefix + key, getattr(form, key).data)`
		`+`
		`+ db.session.add(copr)`
		`+ db.session.commit()`
		`+ flask.flash("Modularity settings updated sucessfully", "success")`
		`+ return flask.redirect(url_for_copr_details(copr))`
		`+ return render_copr_modularity(copr, form)`
		`+`
		`+`
		`def _check_rpmfusion(repos):`
		`if "rpmfusion" in repos:`
		`message = flask.Markup('Using rpmfusion as dependency is nearly always wrong. Please see <a href="https://fedorahosted.org/copr/wiki/UserDocs#WhatIcanbuildinCopr">What I can build in Copr</a>.')`
		`@@ -967,9 +1007,9 @@`
		`return render_create_module(copr, form, profiles=len(form.profile_names))`

		`mmd = modulemd.ModuleMetadata()`
		`- mmd.name = str(copr.name)`
		`- mmd.stream = str(form.stream.data)`
		`- mmd.version = form.version.data`
		`+ mmd.name = str(copr.module_name)`
		`+ mmd.stream = str(copr.module_stream)`
		`+ mmd.version = int(time.time())`
		`mmd.summary = "Module from Copr repository: {}".format(copr.full_name)`

		`for package in form.filter.data:`

no initial comment

frostyx commented 8 years ago

This PR moves module name and stream from module creation page to project settings. That means all modules in one particular project will have same name and stream. Version will be unique each time, because it is current timestamp.

This change was requested by @asamalik weeks ago, so now it is finally done.

clime commented 8 years ago

I would suggest module_stream to be set to a project name automatically and let users set module names at module creation time.

7 new commits added

[frontend] show a quick guide how to install 'dnf module' command
[frontend] add info what to do with modulemd
[frontend] add placeholders for module name and stream
[frontend] add info about module stream
[frontend] allow to have multiple info lines per field
[frontend] print info when there are no packages
[frontend] suggest dnf to enable module

8 years ago

frostyx commented 8 years ago

All changes discussed on meeting done ^^

clime commented 8 years ago

These last commits have mostly nothing to do with the PR title: "Module name and stream in project settings". Could you please split it into one other "doc"-like PR?

4 new commits added

[frontend] add placeholders for module name and stream
[frontend] add info about module stream
[frontend] move module name and stream to project settings
[frontend] module name and stream should not be empty

8 years ago

frostyx commented 8 years ago

Sure, I moved some commits to PR #33

clime commented 8 years ago

I still think, automatic derivation should be possible. E.g. stream = username (@groupname) and module_name = project_name. Limitation of type "This property cannot be changed after first build" is not very convenient for users, nor common from my point of view. Also, requiring user to set something in project settings before building his or her first module can be a little bit off-putting.

frostyx commented 8 years ago

Let's just throw this PR and also the idea away.

There might have been some cool idea in the modularity group in the past, that Adam requested this feature, but I think that it is no longer valid

Let's take https://pagure.io/modulemd/blob/master/f/spec.yaml as an evidence

# Module name, optional
# Typically filled in by the buildsystem, using the VCS repository
# name as the name of the module.
name: foo
# Module update stream, optional
# Typically filled in by the buildsystem, using the VCS branch name
# as the name of the stream.
stream: stream-name
# Module version, integer, optional, cannot be negative
# Typically filled in by the buildsystem, using the VCS commit
# timestamp.  Module version defines upgrade path for the particular
# update stream.
version: 20160927144203

All the fields explicitly says it "Typically filled in by the buildsystem". So it would be an awesome idea to just fill it in by our buildsystem - as you have suggested months ago.

Pull-Request has been closed by frostyx

8 years ago