Rewrite version_win.pl in Python

NEW
Unassigned

Status

()

Core
Build Config
3 years ago
2 months ago

People

(Reporter: ted, Unassigned, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=python])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
It lives here:
http://dxr.mozilla.org/mozilla-central/source/config/version_win.pl

And it gets called from here:
http://dxr.mozilla.org/mozilla-central/source/config/version.mk

We should rewrite it in Python, it's pretty simple and it would get rid of one more Perl script. version.mk is the only place it's called, so any commandline arguments that it supports that aren't used there aren't necessary (whoever takes this doesn't have to do a full port of all supported options).

Fundamentally what this script does is pretty simple:
1) Parse some commandline options.
2) Read milestone.txt and the buildid file for some input data (version number and build id)
3) Read $srcdir/module.ver if it exists for some override values
4) Calculate a few trivial values based on the inputs from the commandline and the files in steps 2 and 3
5) Write to $objdir/module.rc:
a) A static header
b) The contents of the RCINCLUDE file from the commandline options (with a s/\@MOZ_APP_DISPLAYNAME\@/$displayname/)
c) A template string including some values from the inputs or the calculations in step 4

Comment 1

3 years ago
So i am a beginer as you might have guessed... I've started working on a peice of Python code...
You may wish to build on to of the patch in bug 739601; it already provides some of the code you'll need.

Comment 3

3 years ago
Created attachment 8543759 [details]
Though it is not complete yet... This is what i propose...

Here is what i've worked on so far... Criticisms are welcome (Dont be too harsh on me...)
(Reporter)

Comment 4

3 years ago
Comment on attachment 8543759 [details]
Though it is not complete yet... This is what i propose...

This actually looks like a good start! I'm going to set the feedback flag on this so that I remember to give it a more thorough look, but I think you're soundly on the right track.

Do you have a local Firefox build environment set up? To contribute effectively you'll want to do that, so at the very least you can provide patches against our current source code. The documentation here should help:
https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

If you need more help you can join our IRC server at irc.mozilla.org and join the #introduction channel.

Thanks for contributing!
Attachment #8543759 - Flags: feedback?(ted)
(Reporter)

Comment 5

3 years ago
I'm going to assign this bug to you, which will let other people know you're actively working on it.
Assignee: nobody → talinpaul

Comment 6

3 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> I'm going to assign this bug to you, which will let other people know you're
> actively working on it.

Thanks for the feedback...
(Reporter)

Comment 7

3 years ago
Comment on attachment 8543759 [details]
Though it is not complete yet... This is what i propose...

This is a great start, thanks! I'm going to give you a lot of feedback here but don't panic! Most of it is pretty straightforward. Did you get a local copy of the Firefox repository to work with? Submitting patches that way will make everyone's life easier, and if you have a Windows machine you can actually build the source and test that your patch does what you want.

You'll want to put a license header at the top of this file, you can just copy one from another Python file in the source tree. Also: a minor note but we use single quotes (') for strings in all our new Python.

>import sys
>import os
>import argparse
>
>#1) Parse some commandline options.
>
>#Creates version resource file
>
>#Paramaters are passed on the command line:
>
>#Example: -MODNAME nsToolkitCompsModule -DEBUG=1
>
># DEBUG - Mozilla's global debug variable - tells if its debug version
># OFFICIAL - tells Mozilla is building a milestone or nightly
># MSTONE - tells which milestone is being built;
># OBJDIR - Holds the object directory;
># MODNAME - tells what the name of the module is like nsBMPModule
># DEPTH - Holds the path to the root obj dir
># TOPSRCDIR - Holds the path to the root mozilla dir
># SRCDIR - Holds module.ver and source
># BINARY - Holds the name of the binary file
># DISPNAME - Holds the display name of the built application
># APPVERSION - Holds the version string of the built application
># RCINCLUDE - Holds the name of the RC File to include or ""
># QUIET - Turns off output
>
>#Description and Comment come from module.ver
>#Bug 23560
>#http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/rc_7x2d.asp
>
>################################################################################
># ToDo: Code in progress
>################################################################################
>def getDaysFromBuild():
>    # Add Later...
>    return days_from_build
>def getOfficialMilestone(milestone_file):
>    with milestone_file as file:
>      for line in file:
>          if bool(re.match("^\d",line)):
>             milestone = line
>             return milestone
>
>module = None
>
>parser = argparse.ArgumentParser(description='Argument parser...')
># Adding arguments.
>parser.add_argument('--quiet',      metavar='QUIET',        help='Turns off output')
>parser.add_argument('--debug',      metavar='DEBUG',        help='Mozilla\'s global debug variable - tells if its debug version')
>parser.add_argument('--official',   metavar='OFFICIAL',     help='tells Mozilla is building a milestone or nightly')

>parser.add_argument('--mstone',     metavar='MSTONE',       help='tells which milestone is being built')
>parser.add_argument('--modname',    metavar='MODNAME',      help='tells what the name of the module is like nsBMPModule')

We don't seem to use these two arguments in version.mk, so you can leave them out.

I noticed two things:
1) The original script uses UPPERCASE for its options.
2) The original script passes arguments with a single leading dash, like -QUIET.

I'm not even sure you can support #2 with argparse, so it's probably not worthwhile. This means you'll have to change version.mk as well:
https://hg.mozilla.org/mozilla-central/annotate/29b05d283b00/config/version.mk

So that things like '-QUIET 1' become '--quiet 1'. Since you're making that change you could also change any of these arguments that take a 1 as the option value to just be action='store_true' in the parser.add_argument call, so that you could simply pass '--quiet'.


># Parsing the args...
>args = vars(parser.parse_args())
>
>#Adding default arguments...

You can set these defaults up above in the calls to add_argument, it takes a 'default=' keyword argument.

>
>#I dont understand what these are.... So {{Voodoo}} Do not touch unless you understand...
>mfversion = "Personal"
>mpversion = "Personal"

These don't seem to ever be used in the original script, they're always overridden by reading milestone.txt here:
https://hg.mozilla.org/mozilla-central/annotate/29b05d283b00/config/version_win.pl#l192

>comment = None
>description = None
>fileflags = ["0"]
>
>#still not used
>bufferstr = ""
>
>#2) Read milestone.txt and the buildid file for some input data (version number and build id)
># Figuring out what to do with this....
># Figured out that we are supposed to read some values and voodoo dance with it!!!
>MILESTONE_FILE = open( os.path.join( args['topsrcdir'] + "/config/milestone.txt"), 'r')

Lucky for you in the interim here Ms2ger already wrote this in Python, so you can just do:

from mozbuild import milestone
m = milestone.get_official_milestone(os.path.join(args['topsrcdir'], 'config/milestone.txt')


>
>BUILDID_FILE = open( os.path.join( args['depth'] + "/config/buildid"), 'r')

note my example above, you want a comma there in your call to os.path.join, not a +.

>buildid = BUILDID_FILE.readline()
>BUILDID_FILE.close()

it's more conventional Python to write:
with open(os.path.join(args['depth'], 'config/buildid'), 'r') as f:
  buildid = f.readline()

>
>#3) Read $srcdir/module.ver if it exists for some override values
>if os.path.isfile( os.path.join( args['srcdir'] + '/module.ver')): # Check module.ver file exists...
>    # It exists!!! YAAAAAYYYYYY......
>    # Read the Module.ver file
>    VER_FILE = open( os.path.join( args['srcdir'] + '/module.ver'), 'r')
>    # Complicated reading...Involes RE.... Hence Do it later...
>
>
>else:
>    # It does'nt exist... Take that suckers....
>    # Use default values.
>    # Where am i gonna find these values???
>    # Got It!!!
>    if args['quiet'] is not None:
>        # Shout it out soapbox!!!
>        print "WARNING: No module.ver file included ($module, $binary). Default values used\n"

As funny as your stream-of-consciousness comments are here, make sure you remove them when you submit a patch that's closer to being ready to land. :)

>
>#4) Calculate a few trivial values based on the inputs from the commandline and the files in steps 2 and 3
>day_count = getDaysFromBuild()
>
>if milestone is None:
>    milestone = getOfficialMilestone(MILESTONE_FILE);
>
>
>#5) Write to $objdir/module.rc:
>RC_FILE = open( os.path.join( args['objdir'] + '/module.rc'), 'w')
>
>RC_FILE.write(  """
>// This Source Code Form is subject to the terms of the Mozilla Public
>// License, v. 2.0. If a copy of the MPL was not distributed with this
>// file, You can obtain one at http://mozilla.org/MPL/2.0/.
>
>#include<winver.h>
>
>// Note: if you contain versioning information in an included
>// RC script, it will be discarded
>// Use module.ver to explicitly set these values
>
>// Do not edit this file. Changes won\'t affect the build.
>""")
>versionlevel    = 0
>insideversion   = 0
>
>RC_FILE.write(  """
>
>/////////////////////////////////////////////////////////////////////////////
>//
>// Version
>//
>
>1 VERSIONINFO
> FILEVERSION    $fileversion
> PRODUCTVERSION $productversion
> FILEFLAGSMASK 0x3fL
> FILEFLAGS $fileflags
> FILEOS VOS__WINDOWS32
> FILETYPE VFT_DLL
> FILESUBTYPE 0x0L
>BEGIN
>    BLOCK "StringFileInfo"
>    BEGIN
>        BLOCK "000004b0"
>        BEGIN
>            VALUE "Comments", "$comment"
>            VALUE "LegalCopyright", "$copyright"
>            VALUE "CompanyName", "$company"
>            VALUE "FileDescription", "$description"
>            VALUE "FileVersion", "$mfversion"
>            VALUE "ProductVersion", "$mpversion"
>            VALUE "InternalName", "$module"
>            VALUE "LegalTrademarks", "$trademarks"
>            VALUE "OriginalFilename", "$binary"
>            VALUE "ProductName", "$productname"
>            VALUE "BuildID", "$buildid"
>        END
>    END
>    BLOCK "VarFileInfo"
>    BEGIN
>        VALUE "Translation", 0x0, 1200
>    END
>END
>""")

You're going to want to use Python string interpolation here to get the variables into that string you're writing, so it'd look something like:
RC_FILE.write(  """
...
 FILEVERSION    {fileversion}
...
""".format(fileversion=version))

>
>RC_FILE.close()

You should also use a with statement here like I suggested above.

Overall you're off to a good start here, I'd like to see you make more progress. Thanks for working on this!
Attachment #8543759 - Flags: feedback?(ted) → feedback+
Hey, are you planning to finish this bug or should I?
Flags: needinfo?(talinpaul)

Comment 9

3 years ago
Since there no one seems to work on this bug for four months, and the author of the original patch haven't responded on commends for two months, Шэв like to work on it.
Flags: needinfo?(talinpaul)

Comment 10

3 years ago
Created attachment 8608078 [details]
version_win.py replacement for version_win.pl - reviison 1

I tried to address everything in comment #7
Assignee: talinpaul → tim_tim2000
Attachment #8543759 - Attachment is obsolete: true
Attachment #8608078 - Flags: review?(ted)
Attachment #8608078 - Flags: feedback?(Ms2ger)

Updated

3 years ago
Attachment #8608078 - Flags: review?(ted) → review?(gps)
Created attachment 8620440 [details] [diff] [review]
Rewrite version_win in Python

I converted your code to a patch file, which is Mozilla's standard for
reviewing code.

I also made some minor changes to styling and did things like add line
breaks to make things slightly more readable.

The changes were invasive enough that I don't feel comfortable reviewing
my own code. Over to ted for review.
Attachment #8620440 - Flags: review?(ted)

Updated

3 years ago
Assignee: tim_tim2000 → gps
Status: NEW → ASSIGNED

Updated

3 years ago
Attachment #8608078 - Attachment is obsolete: true
Attachment #8608078 - Flags: review?(gps)
Attachment #8608078 - Flags: feedback?(Ms2ger)

Comment 12

3 years ago
>I also made some minor changes to styling and did things like add line
>breaks to make things slightly more readable.

I do not undersatnd difference between importing os.path and os.
Second, what's reasoning behind re-ordering imports?
Flags: needinfo?(gps)
`import os` will pull in `os.path` automatically, so there is no need to do `import os.path`.

I like ordered imports for readability. Makes it clear where to insert new things.
Flags: needinfo?(gps)
ted: review ping?
ted: review ping?

Comment 16

a year ago
Hey, I'd be interested to take this up if noone else is still working on it. Will wait for a response before working :)
Comment on attachment 8620440 [details] [diff] [review]
Rewrite version_win in Python

Review of attachment 8620440 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/version_win.py
@@ +12,5 @@
> +from mozbuild import milestone
> +
> +
> +def days_from_build_id(build):
> +    init = datetime.datetime(2000, 1, 1, 0, 0, 0)

You can drop the last three arguments.
Attachment #8620440 - Flags: review?(ted) → review+

Comment 18

8 months ago
I'm not actively working on this.
Assignee: gps → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.